Skip to content

Commit

Permalink
Fix memory leak by replacing free() with destroy
Browse files Browse the repository at this point in the history
  • Loading branch information
justinfx committed May 19, 2020
1 parent 8ff3621 commit 968bf5f
Show file tree
Hide file tree
Showing 13 changed files with 90 additions and 31 deletions.
2 changes: 1 addition & 1 deletion colorspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func deleteColorspace(c *ColorSpace) {
}
if c.ptr != nil {
runtime.SetFinalizer(c, nil)
C.free(c.ptr)
C.deleteColorSpace(c.ptr)
c.ptr = nil
}
runtime.KeepAlive(c)
Expand Down
10 changes: 5 additions & 5 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func deleteConfig(c *Config) {
}
if c.ptr != nil {
runtime.SetFinalizer(c, nil)
C.freeContext((*C._Context)(c.ptr))
C.deleteConfig(c.ptr)
c.ptr = nil
}
runtime.KeepAlive(c)
Expand All @@ -52,7 +52,7 @@ func NewConfig() *Config {
func CurrentConfig() (*Config, error) {
c, err := C.GetCurrentConfig()
if err != nil {
return nil, getLastError((*C._Context)(c))
return nil, getLastError((*C._HandleContext)(c))
}
return newConfig(c), err
}
Expand All @@ -68,7 +68,7 @@ func SetCurrentConfig(config *Config) error {
func ConfigCreateFromEnv() (*Config, error) {
c, err := C.Config_CreateFromEnv()
if err != nil {
return nil, getLastError((*C._Context)(c))
return nil, getLastError((*C._HandleContext)(c))
}
return newConfig(c), err
}
Expand All @@ -80,7 +80,7 @@ func ConfigCreateFromFile(filename string) (*Config, error) {

c, err := C.Config_CreateFromFile(c_str)
if err != nil {
return nil, getLastError((*C._Context)(c))
return nil, getLastError((*C._HandleContext)(c))
}
return newConfig(c), err
}
Expand All @@ -92,7 +92,7 @@ func ConfigCreateFromData(data string) (*Config, error) {

c, err := C.Config_CreateFromData(c_str)
if err != nil {
return nil, getLastError((*C._Context)(c))
return nil, getLastError((*C._HandleContext)(c))
}
return newConfig(c), err
}
Expand Down
2 changes: 1 addition & 1 deletion context.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func deleteContext(c *Context) {
}
if c.ptr != nil {
runtime.SetFinalizer(c, nil)
C.free(c.ptr)
C.deleteContext(c.ptr)
c.ptr = nil
}
runtime.KeepAlive(c)
Expand Down
6 changes: 6 additions & 0 deletions cpp/colorspace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ extern "C" {

namespace OCIO = OCIO_NAMESPACE;

void deleteColorSpace(ColorSpace *p) {
if (p != NULL) {
delete static_cast<OCIO::ColorSpaceRcPtr*>(p);
}
}

ColorSpace* ColorSpace_Create() {
OCIO::ColorSpaceRcPtr ptr;
BEGIN_CATCH_ERR
Expand Down
25 changes: 18 additions & 7 deletions cpp/config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,24 @@ extern "C" {

namespace OCIO = OCIO_NAMESPACE;

void deleteConfig(Config *p) {
if (p != NULL) {
_HandleContext* ctx = (_HandleContext*)p;
if (ctx->handle != NULL) {
delete (OCIO::ConfigRcPtr*)(ctx->handle);
ctx->handle = NULL;
}
freeHandleContext(ctx);
}
}

// Config Init
Config* Config_Create() {
return (Config*) NEW_CONTEXT(new OCIO::ConfigRcPtr(OCIO::Config::Create()));
return (Config*) NEW_HANDLE_CONTEXT(new OCIO::ConfigRcPtr(OCIO::Config::Create()));
}

const Config* GetCurrentConfig() {
Config* c = NEW_CONTEXT();
Config* c = NEW_HANDLE_CONTEXT();
BEGIN_CATCH_ERR
c->handle = new OCIO::ConstConfigRcPtr(OCIO::GetCurrentConfig());
END_CATCH_CTX_ERR(c)
Expand All @@ -32,15 +43,15 @@ extern "C" {
}

const Config* Config_CreateFromEnv() {
Config* c = NEW_CONTEXT();
Config* c = NEW_HANDLE_CONTEXT();
BEGIN_CATCH_ERR
c->handle = new OCIO::ConstConfigRcPtr(OCIO::Config::CreateFromEnv());
END_CATCH_CTX_ERR(c)
return c;
}

const Config* Config_CreateFromFile(const char* filename) {
Config* c = NEW_CONTEXT();
Config* c = NEW_HANDLE_CONTEXT();
BEGIN_CATCH_ERR
c->handle = new OCIO::ConstConfigRcPtr(OCIO::Config::CreateFromFile(filename));
END_CATCH_CTX_ERR(c)
Expand All @@ -52,7 +63,7 @@ extern "C" {
s << data;
s.seekp(0);

Config* c = NEW_CONTEXT();
Config* c = NEW_HANDLE_CONTEXT();

BEGIN_CATCH_ERR
c->handle = new OCIO::ConstConfigRcPtr(OCIO::Config::CreateFromStream(s));
Expand All @@ -62,7 +73,7 @@ extern "C" {
}

Config* Config_createEditableCopy(Config *p) {
Config* cpy = NEW_CONTEXT();
Config* cpy = NEW_HANDLE_CONTEXT();
BEGIN_CATCH_ERR
cpy->handle = new OCIO::ConfigRcPtr(static_cast<OCIO::ConstConfigRcPtr*>(p->handle)->get()->createEditableCopy());
END_CATCH_CTX_ERR(p)
Expand Down Expand Up @@ -140,7 +151,7 @@ extern "C" {
ptr = static_cast<OCIO::ConstConfigRcPtr*>(p->handle)->get()->getProcessor(ct_ptr, srcCS_ptr, dstCS_ptr);
END_CATCH_CTX_ERR(p)

return (Processor*) NEW_CONTEXT(new OCIO::ConstProcessorRcPtr(ptr));
return (Processor*) new OCIO::ConstProcessorRcPtr(ptr);
}

Processor* Config_getProcessor_CS_CS(Config *p, ColorSpace* srcCS, ColorSpace* dstCS) {
Expand Down
6 changes: 6 additions & 0 deletions cpp/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ extern "C" {

namespace OCIO = OCIO_NAMESPACE;

void deleteContext(Context *p) {
if (p != NULL) {
delete (OCIO::ContextRcPtr*)p;
}
}

Context* Context_Create() {
OCIO::ContextRcPtr ptr;
BEGIN_CATCH_ERR
Expand Down
17 changes: 11 additions & 6 deletions cpp/ocio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
static char* NO_ERROR = (char*)"";


_Context* NEW_CONTEXT(void* handle=NULL) {
_Context *ctx = new _Context;
_HandleContext* NEW_HANDLE_CONTEXT(void* handle=NULL) {
_HandleContext *ctx = new _HandleContext;
ctx->handle = handle;
ctx->last_error = NULL;
return ctx;
Expand All @@ -55,19 +55,24 @@ extern "C" {
const char* ROLE_TEXTURE_PAINT = OCIO::ROLE_TEXTURE_PAINT;
const char* ROLE_MATTE_PAINT = OCIO::ROLE_MATTE_PAINT;

void freeContext(_Context* ctx) {
void freeHandleContext(_HandleContext* ctx) {
if (ctx != NULL) {
// Specific handle type may have already
// deleted this for us
if (ctx->handle != NULL) {
free(ctx->handle);
}
ctx->handle = NULL;
}
if (ctx->last_error != NULL && ctx->last_error != NO_ERROR) {
// from strdup()
free(ctx->last_error);
ctx->last_error = NULL;
}
free(ctx);
delete ctx;
}
}

char* getLastError(_Context* ctx) {
char* getLastError(_HandleContext* ctx) {
return ctx->last_error;
}

Expand Down
19 changes: 13 additions & 6 deletions cpp/ocio.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ typedef enum TransformDirection {
TRANSFORM_DIR_INVERSE
} TransformDirection;

typedef struct _Context {
typedef struct _HandleContext {
void* handle;
char* last_error;
} _Context;
} _HandleContext;

// typedef void Config;
typedef _Context Config;
typedef _HandleContext Config;
typedef void ColorSpace;
typedef void Context;
typedef void Processor;
Expand All @@ -82,8 +82,8 @@ typedef void PackedImageDesc;
typedef void Transform;
typedef void DisplayTransform;

void freeContext(_Context* ctx);
char* getLastError(_Context* ctx);
void freeHandleContext(_HandleContext* ctx);
char* getLastError(_HandleContext* ctx);

// Global
void ClearAllCaches();
Expand All @@ -93,6 +93,7 @@ LoggingLevel GetLoggingLevel();
void SetLoggingLevel(LoggingLevel level);

// Config Init
void deleteConfig(Config *p);
Config* Config_Create();
const Config* GetCurrentConfig();
void SetCurrentConfig(Config *p);
Expand Down Expand Up @@ -122,6 +123,7 @@ Processor* Config_getProcessor_TX_D(Config *p, Transform* tx, TransformDirection
Processor* Config_getProcessor_CT_TX_D(Config *p, Context* ct, Transform* tx, TransformDirection direction);

// Config ColorSpaces
void deleteColorSpace(ColorSpace *p);
int Config_getNumColorSpaces(Config *p);
const char* Config_getColorSpaceNameByIndex(Config *p, int index);
const ColorSpace* Config_getColorSpace(Config *p, const char* name);
Expand Down Expand Up @@ -168,7 +170,8 @@ void ColorSpace_setDescription(ColorSpace *p, const char* description);
BitDepth ColorSpace_getBitDepth(ColorSpace *p);
void ColorSpace_setBitDepth(ColorSpace *p, BitDepth bitDepth);

// Context
// Context
void deleteContext(Context *p);
Context* Context_Create();
Context* Context_createEditableCopy(Context *p);
const char* Context_getCacheID(Context *p);
Expand All @@ -185,6 +188,7 @@ const char* Context_resolveStringVar(Context *p, const char* val);
const char* Context_resolveFileLocation(Context *p, const char* filename);

// Processor
void deleteProcessor(Processor* p);
Processor* Processor_Create();
bool Processor_isNoOp(Processor *p);
bool Processor_hasChannelCrosstalk(Processor *p);
Expand All @@ -194,6 +198,7 @@ ProcessorMetadata* Processor_getMetadata(Processor *p);
void Processor_apply(Processor *p, ImageDesc *i);
const char* Processor_getCpuCacheID(Processor *p);

void deleteProcessorMetadata(ProcessorMetadata* p);
ProcessorMetadata* ProcessorMetadata_Create();
int ProcessorMetadata_getNumFiles(ProcessorMetadata *p);
const char* ProcessorMetadata_getFile(ProcessorMetadata *p, int index);
Expand All @@ -203,13 +208,15 @@ void ProcessorMetadata_addFile(ProcessorMetadata *p, const char* fname);
void ProcessorMetadata_addLook(ProcessorMetadata *p, const char* look);

// ImageDesc
void deletePackedImageDesc(PackedImageDesc* p);
PackedImageDesc* PackedImageDesc_Create(float* data, long width, long height, long numChannels);
float* PackedImageDesc_getData(PackedImageDesc *p);
long PackedImageDesc_getWidth(PackedImageDesc *p);
long PackedImageDesc_getHeight(PackedImageDesc *p);
long PackedImageDesc_getNumChannels(PackedImageDesc *p);

// DisplayTransform
void deleteDisplayTransform(DisplayTransform* d);
DisplayTransform* DisplayTransform_Create();
DisplayTransform* DisplayTransform_createEditableCopy(DisplayTransform *p);
TransformDirection DisplayTransform_getDirection(DisplayTransform *p);
Expand Down
18 changes: 18 additions & 0 deletions cpp/processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ extern "C" {

namespace OCIO = OCIO_NAMESPACE;

void deleteProcessor(Processor* p) {
if (p != NULL) {
delete (OCIO::ProcessorRcPtr*)p;
}
}

Processor* Processor_Create() {
OCIO::ProcessorRcPtr ptr;
BEGIN_CATCH_ERR
Expand Down Expand Up @@ -42,6 +48,12 @@ extern "C" {
}

// ProcessorMetadata
void deleteProcessorMetadata(ProcessorMetadata* p) {
if (p != NULL) {
delete (OCIO::ProcessorMetadataRcPtr*)p;
}
}

ProcessorMetadata* Processor_getMetadata(Processor *p) {
OCIO::ConstProcessorMetadataRcPtr ptr;
BEGIN_CATCH_ERR
Expand Down Expand Up @@ -95,6 +107,12 @@ extern "C" {
}

// ImageDesc
void deletePackedImageDesc(PackedImageDesc* p) {
if (p != NULL) {
delete (OCIO::PackedImageDesc*)p;
}
}

PackedImageDesc* PackedImageDesc_Create(float* data, long width, long height, long numChannels) {
BEGIN_CATCH_ERR
return (PackedImageDesc*) new OCIO::PackedImageDesc(data, width, height, numChannels);
Expand Down
6 changes: 6 additions & 0 deletions cpp/transform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ extern "C" {

namespace OCIO = OCIO_NAMESPACE;

void deleteDisplayTransform(DisplayTransform* d) {
if (d != NULL) {
delete (OCIO::DisplayTransformRcPtr*)d;
}
}

DisplayTransform* DisplayTransform_Create() {
OCIO::DisplayTransformRcPtr ptr;
BEGIN_CATCH_ERR
Expand Down
2 changes: 1 addition & 1 deletion ocio.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ var (
Errors
*/

func getLastError(ptr *C._Context) error {
func getLastError(ptr *C._HandleContext) error {
return errors.New(C.GoString(ptr.last_error))
}

Expand Down
6 changes: 3 additions & 3 deletions processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func deleteProcessor(p *Processor) {
}
if p.ptr != nil {
runtime.SetFinalizer(p, nil)
C.free(p.ptr)
C.deleteProcessor(p.ptr)
p.ptr = nil
}
runtime.KeepAlive(p)
Expand Down Expand Up @@ -102,7 +102,7 @@ func deleteProcessorMetadata(c *ProcessorMetadata) {
}
if c.ptr != nil {
runtime.SetFinalizer(c, nil)
C.free(c.ptr)
C.deleteProcessorMetadata(c.ptr)
c.ptr = nil
}
runtime.KeepAlive(c)
Expand Down Expand Up @@ -200,7 +200,7 @@ func deletePackedImageDesc(p *PackedImageDesc) {
}
if p.ptr != nil {
runtime.SetFinalizer(p, nil)
C.free(p.ptr)
C.deletePackedImageDesc(p.ptr)
p.ptr = nil
}
runtime.KeepAlive(p)
Expand Down
2 changes: 1 addition & 1 deletion transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func deleteDisplayTransform(tx *DisplayTransform) {
}
if tx.ptr != nil {
runtime.SetFinalizer(tx, nil)
C.free(tx.ptr)
C.deleteDisplayTransform(tx.ptr)
tx.ptr = nil
}
runtime.KeepAlive(tx)
Expand Down

0 comments on commit 968bf5f

Please sign in to comment.