During close/terminate/release methods for objects, remove all references in order to reduce memory consumption as much as possible; ensure that failure to close/terminate/release is handled in the same way by all objects (even though for some this is an extremely unlikely situation).

Author: cjbj

src/njs/src/njsCommon.cpp

@@ -212,18 +212,6 @@ njsBaton::~njsBaton()
 }
 
 
-//-----------------------------------------------------------------------------
-// njsBaton::GetCallingObj()
-//   Return the C++ object stored in the baton as the JS calling object.
-//-----------------------------------------------------------------------------
-njsCommon *njsBaton::GetCallingObj()
-{
-    Nan::HandleScope scope;
-    Local<Object> obj = Nan::New(jsCallingObj);
-    return Nan::ObjectWrap::Unwrap<njsCommon>(obj);
-}
-
-
 //-----------------------------------------------------------------------------
 // njsBaton::GetOracledb()
 //   Return the Oracledb object stored in the baton as a JS object.
@@ -293,9 +281,8 @@ void njsBaton::AsyncAfterWorkCallback(uv_work_t *req, int status)
         Local<Function> callback = Nan::New<Function>(baton->jsCallback);
 
         // if this baton is considered the active baton, clear it
-        njsCommon *callingObj = baton->GetCallingObj();
-        if (callingObj && baton == callingObj->activeBaton)
-            callingObj->activeBaton = NULL;
+        if (baton->callingObj && baton == baton->callingObj->activeBaton)
+            baton->callingObj->activeBaton = NULL;
 
         // delete the baton before the callback is made so any unnecessary
         // ODPI-C handles are released as soon as possible

src/njs/src/njsCommon.h

@@ -271,6 +271,7 @@ class njsBaton {
     char *bufferPtr;
     uint64_t lobOffset;
     uint64_t lobAmount;
+    njsCommon *callingObj;
     Nan::Persistent<Object> jsCallingObj;
     Nan::Persistent<Object> jsOracledb;
     Nan::Persistent<Object> jsBuffer;

src/njs/src/njsConnection.cpp

@@ -1474,7 +1474,9 @@ void njsConnection::Async_AfterExecute(njsBaton *baton, Local<Value> argv[])
 // njsConnection::Release()
 //   Releases the connection from use by JS. This releases the connection back
 // to the pool or closes the standalone connection so further use is not
-// possible.
+// possible. The reference to the DPI handle is transferred to the baton so
+// that it will be cleared automatically upon success and so that the
+// connection is marked as invalid immediately.
 //
 // PARAMETERS
 //   - JS callback which will receive (error)
@@ -1498,13 +1500,30 @@ NAN_METHOD(njsConnection::Release)
 
 //-----------------------------------------------------------------------------
 // njsConnection::Async_Release()
-//   Worker function for njsConnection::Release() method.
+//   Worker function for njsConnection::Release() method. If the attempt to
+// close fails, the reference to the DPI handle is transferred back from the
+// baton to the connection.
 //-----------------------------------------------------------------------------
 void njsConnection::Async_Release(njsBaton *baton)
 {
     if (dpiConn_close(baton->dpiConnHandle, DPI_MODE_CONN_CLOSE_DEFAULT, NULL,
-            0) < 0)
+            0) < 0) {
         baton->GetDPIError();
+        njsConnection *connection = (njsConnection*) baton->callingObj;
+        connection->dpiConnHandle = baton->dpiConnHandle;
+        baton->dpiConnHandle = NULL;
+    }
+}
+
+
+//-----------------------------------------------------------------------------
+// njsConnection::Async_AfterRelease()
+//   Finishes release by cleaning up references.
+//-----------------------------------------------------------------------------
+void njsConnection::Async_AfterRelease(njsBaton *baton, Local<Value> argv[])
+{
+    njsConnection *connection = (njsConnection*) baton->callingObj;
+    connection->jsOracledb.Reset();
 }
 
 

src/njs/src/njsConnection.h

@@ -99,6 +99,7 @@ class njsConnection: public njsCommon {
     // Release Method on Connection class
     static NAN_METHOD(Release);
     static void Async_Release(njsBaton *baton);
+    static void Async_AfterRelease(njsBaton *baton, Local<Value> argv[]);
 
     // Commit Method on Connection class
     static NAN_METHOD(Commit);

src/njs/src/njsIntLob.cpp

@@ -346,7 +346,7 @@ void njsILob::Async_Read(njsBaton *baton)
 void njsILob::Async_AfterRead(njsBaton *baton, Local<Value> argv[])
 {
     Nan::EscapableHandleScope scope;
-    njsILob *lob = (njsILob*) baton->GetCallingObj();
+    njsILob *lob = (njsILob*) baton->callingObj;
 
     if (!baton->bufferSize)
         argv[1] = scope.Escape(Nan::Null());
@@ -420,7 +420,7 @@ void njsILob::Async_Write(njsBaton *baton)
 //-----------------------------------------------------------------------------
 void njsILob::Async_AfterWrite(njsBaton *baton, Local<Value> argv[])
 {
-    njsILob *lob = (njsILob*) baton->GetCallingObj();
+    njsILob *lob = (njsILob*) baton->callingObj;
     lob->offset += baton->lobAmount;
 }
 

src/njs/src/njsPool.cpp

@@ -371,7 +371,9 @@ void njsPool::Async_AfterGetConnection(njsBaton *baton, Local<Value> argv[])
 
 //-----------------------------------------------------------------------------
 // njsPool::Terminate()
-//   Terminate the pool.
+//   Terminate the pool. The reference to the DPI handle is transferred to the
+// baton so that it will cleared automatically upon success and so that the
+// pool is marked as invalid immediately.
 //
 // PARAMETERS
 //   - JS callback which will receive (error)

src/njs/src/njsPool.h

@@ -84,7 +84,6 @@ class njsPool: public njsCommon {
     // Terminate Methods
     static NAN_METHOD(Terminate);
     static void Async_Terminate(njsBaton *baton);
-    static void Async_AfterTerminate(njsBaton *baton, Local<Value> argv[]);
 
     // Define Getter Accessors to properties
     static NAN_GETTER(GetPoolMax);

src/njs/src/njsResultSet.cpp

@@ -377,7 +377,7 @@ void njsResultSet::Async_AfterGetRows(njsBaton *baton, Local<Value> argv[])
         newBaton->maxRows = baton->maxRows - baton->rowsFetched;
         newBaton->fetchMultipleRows = true;
         newBaton->jsRows.Reset(rows);
-        resultSet = (njsResultSet*) baton->GetCallingObj();
+        resultSet = (njsResultSet*) baton->callingObj;
         resultSet->activeBaton = NULL;
         resultSet->GetRowsCommon(newBaton);
 
@@ -392,7 +392,9 @@ void njsResultSet::Async_AfterGetRows(njsBaton *baton, Local<Value> argv[])
 
 //-----------------------------------------------------------------------------
 // njsResultSet::Close()
-//   Close the result set.
+//   Close the result set. The reference to the DPI handle is transferred to
+// the baton so that it will be cleared automatically upon success and so that
+// the result set is marked as invalid immediately.
 //
 // PARAMETERS
 //   - JS callback which will receive (error)
@@ -414,17 +416,46 @@ NAN_METHOD(njsResultSet::Close)
         baton->dpiStmtHandle = resultSet->dpiStmtHandle;
         resultSet->dpiStmtHandle = NULL;
     }
-    baton->QueueWork("Close", Async_Close, NULL, 1);
+    baton->QueueWork("Close", Async_Close, Async_AfterClose, 1);
 }
 
 
 //-----------------------------------------------------------------------------
 // njsResultSet::Async_Close()
-//   Worker function for njsResultSet::Close() method.
+//   Worker function for njsResultSet::Close() method. If the attempt to close
+// the statement fails, the reference to the DPI handle is transferred back
+// from the baton to the result set.
 //-----------------------------------------------------------------------------
 void njsResultSet::Async_Close(njsBaton *baton)
 {
-    if (dpiStmt_close(baton->dpiStmtHandle, NULL, 0) < 0)
+    if (dpiStmt_close(baton->dpiStmtHandle, NULL, 0) < 0) {
         baton->GetDPIError();
+        njsResultSet *resultSet = (njsResultSet*) baton->callingObj;
+        resultSet->dpiStmtHandle = baton->dpiStmtHandle;
+        baton->dpiStmtHandle = NULL;
+    }
+}
+
+
+//-----------------------------------------------------------------------------
+// njsResultSet::Async_AfterClose()
+//   Finishes close by transferring variables and fetch as types to the baton
+// where they will be freed.
+//-----------------------------------------------------------------------------
+void njsResultSet::Async_AfterClose(njsBaton *baton, Local<Value> argv[])
+{
+    njsResultSet *resultSet = (njsResultSet*) baton->callingObj;
+
+    resultSet->jsConnection.Reset();
+    resultSet->jsOracledb.Reset();
+    baton->keepQueryInfo = false;
+    baton->queryVars = resultSet->queryVars;
+    baton->numQueryVars = resultSet->numQueryVars;
+    resultSet->queryVars = NULL;
+    resultSet->numQueryVars = 0;
+    baton->numFetchAsStringTypes = resultSet->numFetchAsStringTypes;
+    baton->fetchAsStringTypes = resultSet->fetchAsStringTypes;
+    resultSet->fetchAsStringTypes = NULL;
+    resultSet->numFetchAsStringTypes = 0;
 }
 

src/njs/src/njsResultSet.h

@@ -93,6 +93,7 @@ class njsResultSet: public njsCommon {
     // Close Methods
     static NAN_METHOD(Close);
     static void Async_Close(njsBaton *baton);
+    static void Async_AfterClose(njsBaton *baton, Local<Value> argv[]);
 
     // Define Getter Accessors to properties
     static NAN_GETTER(GetMetaData);