Close cursors for repeated SQL queries that return NJS errors

Author: sharadraju

doc/src/release_notes.rst

@@ -19,6 +19,9 @@ Common Changes
 Thin Mode Changes
 ++++++++++++++++++
 
+#)  Fixed bug that throws an 'ORA-01000' error when a query is executed multiple
+    times even though query returned errors like 'NJS-119', 'NJS-016'.
+
 #)  Internal performance optimizations for network buffer and packet handling
 
 #)  Ensure that the database port is passed as a number to the network connection.

lib/thin/protocol/messages/base.js

@@ -448,6 +448,13 @@ class Message {
     }
     return writtenBytes;
   }
+
+  // Called when an error is encountered during decode of RPC
+  saveDeferredErr() {
+    if (!this.deferredErr) {
+      this.deferredErr = errors.getErr(...arguments);
+    }
+  }
 }
 
 module.exports = Message;

lib/thin/protocol/messages/withData.js

@@ -57,6 +57,7 @@ class MessageWithData extends Message {
     this.inFetch = false;
     this.parseOnly = false;
     this.resultSetsToSetup = [];
+    this.deferredErr = null;
   }
 
   /**
@@ -472,7 +473,7 @@ class MessageWithData extends Message {
       if (actualNumBytes < 0 && oraTypeNum === constants.TNS_DATA_TYPE_BOOLEAN) {
         colValue = null;
       } else if (actualNumBytes !== 0 && colValue !== null) {
-        errors.throwErr(errors.ERR_INSUFFICIENT_BUFFER_FOR_BINDS);
+        this.saveDeferredErr(errors.ERR_INSUFFICIENT_BUFFER_FOR_BINDS);
       }
     } else if (oraTypeNum === constants.TNS_DATA_TYPE_LONG || oraTypeNum === constants.TNS_DATA_TYPE_LONG_RAW || variable.maxSize > buf.caps.maxStringSize) {
       buf.skipSB4();                            // null indicator
@@ -525,11 +526,15 @@ class MessageWithData extends Message {
   }
 
   async postProcess() {
+    if (this.deferredErr) {
+      throw this.deferredErr;
+    }
+
     if (this.outVariables) {
       for (const variable of this.outVariables) {
         if (variable.isArray) {
-          for (let pos = 0; pos < variable.numElementsInArray; pos++) {
-            if (variable.outConverter) {
+          if (variable.outConverter) {
+            for (let pos = 0; pos < variable.numElementsInArray; pos++) {
               variable.values[0][pos] = await variable.outConverter(variable.values[0][pos]);
             }
           }

test/clobPlsqlBindAsString_bindout.js

@@ -36,6 +36,7 @@ const assert   = require('assert');
 const fs       = require('fs');
 const dbConfig = require('./dbconfig.js');
 const random   = require('./random.js');
+const testsUtil = require('./testsUtil.js');
 
 describe('75. clobPlsqlBindAsString_bindout.js', function() {
 
@@ -162,13 +163,17 @@ describe('75. clobPlsqlBindAsString_bindout.js', function() {
                "END nodb_clobs_out_741; ";
     const sqlRun = "BEGIN nodb_clobs_out_741 (:i, :c); END;";
     const proc_drop = "DROP PROCEDURE nodb_clobs_out_741";
+    let sysDBAConn;
 
     before(async function() {
       await connection.execute(proc);
     });
 
     after(async function() {
       await connection.execute(proc_drop);
+      if (sysDBAConn) {
+        await sysDBAConn.close();
+      }
     });
 
     it('75.1.1 works with EMPTY_LOB', async function() {
@@ -546,6 +551,51 @@ describe('75. clobPlsqlBindAsString_bindout.js', function() {
       );
     }); // 75.1.28
 
+    it('75.1.29 Verify Cursor leak is not there for Bind Error', async function() {
+      if (!dbConfig.test.DBA_PRIVILEGE) {
+        this.skip();
+      }
+      const len = 32769;
+      const sequence = insertID++;
+      const specialStr = "75.1.24.1";
+      const clobStr = random.getRandomString(len, specialStr);
+      const bindVar = {
+        i: { val: sequence, type: oracledb.NUMBER, dir: oracledb.BIND_IN },
+        c: { type: oracledb.STRING, dir: oracledb.BIND_OUT, maxSize: len - 1 }
+      };
+      await insertClobWithString(sequence, clobStr);
+
+      // Remove the sqlRun statement from cache before checking leak.
+      // It will also let server open any internal cursors for this statement
+      // and hence the actual cursor count for this statement can be verified
+      // after running in loop below.
+      await assert.rejects(
+        async () => await connection.execute(sqlRun, bindVar, { keepInStmtCache: false}),
+        /NJS-016:/
+      );
+
+      const sid = await testsUtil.getSid(connection);
+      const dbaConfig = {
+        user : dbConfig.test.DBA_user,
+        password: dbConfig.test.DBA_password,
+        connectionString: dbConfig.connectString,
+        privilege: oracledb.SYSDBA
+      };
+      sysDBAConn = await oracledb.getConnection(dbaConfig);
+      const openCount = await testsUtil.getOpenCursorCount(sysDBAConn, sid);
+
+      // Running in loop should not result in opening more than 1 cursor
+      // additionally on server.
+      for (let i = 0; i < 15; i++) {
+        await assert.rejects(
+          async () => await connection.execute(sqlRun, bindVar),
+          /NJS-016:/
+        );
+      }
+      const newOpenCount = await testsUtil.getOpenCursorCount(sysDBAConn, sid);
+      assert.strictEqual(newOpenCount - openCount, 1);
+    }); // 75.1.29
+
   }); // 75.1
 
   describe('75.2 CLOB, PLSQL, BIND_OUT to VARCHAR2', function() {

test/testsUtil.js

@@ -280,6 +280,22 @@ testsUtil.getRoundTripCount = async function(sid) {
   }
 };
 
+testsUtil.getMaxCursorsConfigured = async function(conn) {
+  const sql = `SELECT value FROM v$parameter WHERE name = 'open_cursors'`;
+  const result = await conn.execute(sql);
+  return Number(result.rows[0][0]);// Max Cursors config
+};
+
+testsUtil.getOpenCursorCount = async function(systemconn, sid) {
+  const sql = `
+     select ss.value
+     from v$sesstat ss, v$statname sn
+     where ss.sid = :sid
+       and ss.statistic# = sn.statistic#
+       and sn.name = 'opened cursors current'`;
+  const result = await systemconn.execute(sql, [sid]);
+  return result.rows[0][0];  // open cursor count so far in the session
+};
 
 testsUtil.getParseCount = async function(systemconn, sid) {
   const sql = `