Eliminate memory leak when fetching objects that are null

(oracle/python-cx_Oracle#298).
oracle · Apr 25, 2019 · 2ea2c1d · 2ea2c1d
1 parent b96b11b
commit 2ea2c1d
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 20 deletions.
diff --git a/src/dpiImpl.h b/src/dpiImpl.h
@@ -1547,7 +1547,8 @@ int dpiOci__numberToInt(void *number, void *value, unsigned int valueLength,
 int dpiOci__numberToReal(double *value, void *number, dpiError *error);
 int dpiOci__objectCopy(dpiObject *obj, void *sourceInstance,
         void *sourceIndicator, dpiError *error);
-int dpiOci__objectFree(dpiObject *obj, int checkError, dpiError *error);
+int dpiOci__objectFree(void *envHandle, void *data, int checkError,
+        dpiError *error);
 int dpiOci__objectGetAttr(dpiObject *obj, dpiObjectAttr *attr,
         int16_t *scalarValueIndicator, void **valueIndicator, void **value,
         void **tdo, dpiError *error);

diff --git a/src/dpiObject.c b/src/dpiObject.c
@@ -16,6 +16,10 @@
 
 #include "dpiImpl.h"
 
+// forward declarations of internal functions only used in this file
+int dpiObject__closeHelper(dpiObject *obj, int checkError, dpiError *error);
+
+
 //-----------------------------------------------------------------------------
 // dpiObject__allocate() [INTERNAL]
 //   Allocate and initialize an object structure.
@@ -171,25 +175,41 @@ int dpiObject__close(dpiObject *obj, int checkError, dpiError *error)
     // flag; again, this must be done while holding the lock (if in threaded
     // mode) in order to avoid race conditions!
     if (obj->instance && !obj->dependsOnObj) {
-        if (dpiOci__objectFree(obj, checkError, error) < 0) {
+        if (dpiObject__closeHelper(obj, checkError, error) < 0) {
             if (obj->env->threaded)
                 dpiMutex__acquire(obj->env->mutex);
             obj->closing = 0;
             if (obj->env->threaded)
                 dpiMutex__release(obj->env->mutex);
             return DPI_FAILURE;
         }
-        if (!obj->type->conn->closing)
-            dpiHandleList__removeHandle(obj->type->conn->objects,
-                    obj->openSlotNum);
-        obj->instance = NULL;
-        obj->indicator = NULL;
     }
 
     return DPI_SUCCESS;
 }
 
 
+//-----------------------------------------------------------------------------
+// dpiObject__closeHelper() [INTERNAL]
+//   Helper function for closing an object.
+//-----------------------------------------------------------------------------
+int dpiObject__closeHelper(dpiObject *obj, int checkError, dpiError *error)
+{
+    if (dpiOci__objectFree(obj->env->handle, obj->instance, checkError,
+            error) < 0)
+        return DPI_FAILURE;
+    obj->instance = NULL;
+    if (obj->freeIndicator && dpiOci__objectFree(obj->env->handle,
+            obj->indicator, checkError, error) < 0)
+        return DPI_FAILURE;
+    obj->indicator = NULL;
+    if (!obj->type->conn->closing)
+        dpiHandleList__removeHandle(obj->type->conn->objects,
+                obj->openSlotNum);
+    return DPI_SUCCESS;
+}
+
+
 //-----------------------------------------------------------------------------
 // dpiObject__free() [INTERNAL]
 //   Free the memory for an object.

diff --git a/src/dpiOci.c b/src/dpiOci.c
@@ -2276,15 +2276,16 @@ int dpiOci__objectCopy(dpiObject *obj, void *sourceInstance,
 // dpiOci__objectFree() [INTERNAL]
 //   Wrapper for OCIObjectFree().
 //-----------------------------------------------------------------------------
-int dpiOci__objectFree(dpiObject *obj, int checkError, dpiError *error)
+int dpiOci__objectFree(void *envHandle, void *data, int checkError,
+        dpiError *error)
 {
     int status;
 
     DPI_OCI_LOAD_SYMBOL("OCIObjectFree", dpiOciSymbols.fnObjectFree)
-    status = (*dpiOciSymbols.fnObjectFree)(obj->env->handle, error->handle,
-            obj->instance, DPI_OCI_DEFAULT);
+    status = (*dpiOciSymbols.fnObjectFree)(envHandle, error->handle, data,
+            DPI_OCI_DEFAULT);
     if (checkError && DPI_OCI_ERROR_OCCURRED(status)) {
-        dpiError__setFromOCI(error, status, obj->type->conn, "free instance");
+        dpiError__setFromOCI(error, status, NULL, "free instance");
 
         // during the attempt to free, PL/SQL records fail with error
         // "ORA-21602: operation does not support the specified typecode", but
@@ -2295,13 +2296,6 @@ int dpiOci__objectFree(dpiObject *obj, int checkError, dpiError *error)
             return DPI_SUCCESS;
         return DPI_FAILURE;
     }
-    if (obj->freeIndicator) {
-        status = (*dpiOciSymbols.fnObjectFree)(obj->env->handle, error->handle,
-                obj->indicator, DPI_OCI_DEFAULT);
-        if (checkError && DPI_OCI_ERROR_OCCURRED(status))
-            return dpiError__setFromOCI(error, status, obj->type->conn,
-                    "free indicator");
-    }
     return DPI_SUCCESS;
 }
 

diff --git a/src/dpiVar.c b/src/dpiVar.c
@@ -629,7 +629,6 @@ int dpiVar__getValue(dpiVar *var, dpiVarBuffer *buffer, uint32_t pos,
         return DPI_SUCCESS;
     }
 
-
     // check for a NULL value; for objects the indicator is elsewhere
     data = &buffer->externalData[pos];
     if (!buffer->objectIndicator)
@@ -638,8 +637,19 @@ int dpiVar__getValue(dpiVar *var, dpiVarBuffer *buffer, uint32_t pos,
         data->isNull = (*((int16_t*) buffer->objectIndicator[pos]) ==
                 DPI_OCI_IND_NULL);
     else data->isNull = 1;
-    if (data->isNull)
+    if (data->isNull) {
+        if (var->objectType) {
+            if (dpiOci__objectFree(var->env->handle,
+                    buffer->data.asObject[pos], 1, error) < 0)
+                return DPI_FAILURE;
+            if (inFetch && var->objectType->isCollection) {
+                if (dpiOci__objectFree(var->env->handle,
+                        buffer->objectIndicator[pos], 1, error) < 0)
+                    return DPI_FAILURE;
+            }
+        }
         return DPI_SUCCESS;
+    }
 
     // check return code for variable length data
     if (buffer->returnCode) {