Connection Pool code improvements

Author: sharadraju

lib/errors.js

@@ -177,7 +177,6 @@ adjustErrorXref.set("OCI-22165", [ERR_INVALID_COLL_INDEX_SET, /index \[([0-9]+)\
 adjustErrorXref.set("ORA-00028", ERR_CONNECTION_CLOSED);
 adjustErrorXref.set("ORA-00600", ERR_CONNECTION_CLOSED);
 adjustErrorXref.set("ORA-24338", ERR_INVALID_REF_CURSOR);
-adjustErrorXref.set("ORA-24422", ERR_POOL_HAS_BUSY_CONNECTIONS);
 adjustErrorXref.set("ORA-25708", ERR_TOKEN_HAS_EXPIRED);
 
 // define mapping for error messages

lib/pool.js

@@ -279,8 +279,14 @@ class Pool extends EventEmitter {
       });
     }
 
+    // if any connections are still out and the pool is not being force closed,
+    // throw an exception
+    if (!forceClose && this._connectionsOut > 0) {
+      errors.throwErr(errors.ERR_POOL_HAS_BUSY_CONNECTIONS);
+    }
+
     // close the pool
-    await this._impl.close({forceClose: forceClose});
+    await this._impl.close();
     this._status = constants.POOL_STATUS_CLOSED;
     this.emit('_afterPoolClose');
 

lib/thin/pool.js

@@ -228,13 +228,25 @@ class ThinPoolImpl extends PoolImpl {
   //---------------------------------------------------------------------------
   // close pool by destroying available connections
   //---------------------------------------------------------------------------
-  async close(obj) {
+  async close() {
+
     // clear scheduled job
     if (this._schedulerJob) {
       clearTimeout(this._schedulerJob);
       this._schedulerJob = null;
     }
-    return await this.destroyAllNow(obj);
+
+    // destroy all free connections
+    for (const conn of this._freeConnectionList) {
+      await this._destroy(conn);
+    }
+
+    // destroy all used connections
+    for (const conn of this._usedConnectionList) {
+      await this._destroy(conn);
+    }
+
+    this.eventEmitter.removeAllListeners();
   }
 
   //---------------------------------------------------------------------------
@@ -438,33 +450,6 @@ class ThinPoolImpl extends PoolImpl {
     this._setScheduler();
   }
 
-  //---------------------------------------------------------------------------
-  // destroyAllNow()
-  //
-  // called during pool close operation and close any available connection
-  // available in connection pool
-  //---------------------------------------------------------------------------
-  async destroyAllNow(obj) {
-    let conn;
-    while (this._freeConnectionList.length !== 0) {
-      conn = this._freeConnectionList.pop();
-      await this._destroy(conn);
-    }
-
-    if (obj.forceClose) {
-      for (const item of this._usedConnectionList) {
-        await this._destroy(item);
-        this._usedConnectionList.delete(item);
-      }
-    }
-
-    if (this._usedConnectionList.size !== 0) {
-      errors.throwErr(errors.ERR_POOL_HAS_BUSY_CONNECTIONS);
-    }
-
-    this.eventEmitter.removeAllListeners();
-  }
-
   //---------------------------------------------------------------------------
   // _generateConnectionClass()
   //

src/njsBaton.c

@@ -425,6 +425,7 @@ bool njsBaton_getJsonNodeValue(njsBaton *baton, dpiJsonNode *node,
     napi_value key, temp;
     dpiJsonArray *array;
     dpiJsonObject *obj;
+    double temp_double;
     uint32_t i;
 
     // null is a special case
@@ -469,8 +470,9 @@ bool njsBaton_getJsonNodeValue(njsBaton *baton, dpiJsonNode *node,
                     NULL, value))
             return true;
         case DPI_ORACLE_TYPE_NUMBER:
-            NJS_CHECK_NAPI(env, napi_create_double(env, node->value->asDouble,
-                    value))
+            temp_double = (node->nativeTypeNum == DPI_NATIVE_TYPE_DOUBLE) ?
+                    node->value->asDouble : node->value->asFloat;
+            NJS_CHECK_NAPI(env, napi_create_double(env, temp_double, value))
             return true;
         case DPI_ORACLE_TYPE_DATE:
         case DPI_ORACLE_TYPE_TIMESTAMP:

src/njsPool.c

@@ -1,4 +1,4 @@
-// Copyright (c) 2015, 2022, Oracle and/or its affiliates.
+// Copyright (c) 2015, 2023, Oracle and/or its affiliates.
 
 //-----------------------------------------------------------------------------
 //
@@ -108,17 +108,11 @@ const njsClassDef njsClassDefPool = {
 //-----------------------------------------------------------------------------
 // njsPool_close()
 //   Close the pool.
-//
-// PARAMETERS
-//   - options
 //-----------------------------------------------------------------------------
-NJS_NAPI_METHOD_IMPL_ASYNC(njsPool_close, 1, NULL)
+NJS_NAPI_METHOD_IMPL_ASYNC(njsPool_close, 0, NULL)
 {
     njsPool *pool = (njsPool*) baton->callingInstance;
 
-    if (!njsUtils_getNamedPropertyBool(env, args[0], "forceClose",
-            &baton->force))
-        return false;
     baton->accessTokenCallback = pool->accessTokenCallback;
     pool->accessTokenCallback = NULL;
     baton->dpiPoolHandle = pool->handle;
@@ -134,16 +128,14 @@ NJS_NAPI_METHOD_IMPL_ASYNC(njsPool_close, 1, NULL)
 //-----------------------------------------------------------------------------
 static bool njsPool_closeAsync(njsBaton *baton)
 {
-    dpiPoolCloseMode mode = (baton->force) ? DPI_MODE_POOL_CLOSE_FORCE :
-            DPI_MODE_POOL_CLOSE_DEFAULT;
     njsPool *pool = (njsPool*) baton->callingInstance;
 
     pool->accessTokenCallback = baton->accessTokenCallback;
     if (baton->accessTokenCallback) {
         njsTokenCallback_stopNotifications(baton->accessTokenCallback);
         baton->accessTokenCallback = NULL;
     }
-    if (dpiPool_close(baton->dpiPoolHandle, mode) < 0) {
+    if (dpiPool_close(baton->dpiPoolHandle, DPI_MODE_POOL_CLOSE_FORCE) < 0) {
         njsBaton_setErrorDPI(baton);
         pool->handle = baton->dpiPoolHandle;
         baton->dpiPoolHandle = NULL;