Skip to content

Commit e64ce4a

Browse files
sharadrajusharadraju
committed
Fixed cursor leak for dbobject binds (Issue #1694)
1 parent 846dc82 commit e64ce4a

File tree

5 files changed

+207
-90
lines changed

5 files changed

+207
-90
lines changed

‎doc/src/release_notes.rst‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ Common Changes
4545
Thin Mode Changes
4646
+++++++++++++++++
4747

48+
#) Fixed cursor leak for dbobject binds using :meth:`connection.getDbObjectClass()`.
49+
See `Issue #1694 <https://github.com/oracle/node-oracledb/issues/
50+
1694>`__.
51+
4852
#) Fixed bug that did not allow connection to Oracle Database 23ai instances
4953
that have fast authentication disabled.
5054

‎lib/thin/connection.js‎

Lines changed: 104 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,79 @@ class ThinConnectionImpl extends ConnectionImpl {
404404
}
405405
}
406406

407+
//---------------------------------------------------------------------------
408+
// _populateRowTypeInfo()
409+
//
410+
// Get column and datatype information in case of %ROWTYPE handling.
411+
//---------------------------------------------------------------------------
412+
async _populateRowTypeInfo(info, options, result) {
413+
const getColumnsSQL = `
414+
SELECT
415+
column_name,
416+
data_type,
417+
data_type_owner,
418+
case
419+
when data_type in
420+
('CHAR', 'NCHAR', 'VARCHAR2', 'NVARCHAR2', 'RAW')
421+
then data_length
422+
else 0
423+
end,
424+
nvl(data_precision, 0),
425+
nvl(data_scale, 0)
426+
from all_tab_cols
427+
where owner = :owner
428+
and table_name = :name
429+
and hidden_column != 'YES'
430+
order by column_id`;
431+
432+
const bindVal = [
433+
{
434+
name: "owner",
435+
type: types.DB_TYPE_VARCHAR,
436+
maxSize: 128,
437+
dir: constants.BIND_IN,
438+
values: [result.outBinds.schema],
439+
},
440+
{
441+
name: "name",
442+
type: types.DB_TYPE_VARCHAR,
443+
maxSize: 128,
444+
dir: constants.BIND_IN,
445+
values: [info.name.substring(0, info.name.length - 8)]
446+
}
447+
];
448+
const val = await this.execute(
449+
getColumnsSQL, 1, bindVal, options, false
450+
);
451+
try {
452+
const attrRows = await val.resultSet._getAllRows();
453+
info.attributes = [];
454+
for (const row of attrRows) {
455+
const metaData = {
456+
name: row[0],
457+
dataType: row[1],
458+
dataTypeOwner: row[2],
459+
maxSize: row[3],
460+
dataPrecision: row[4],
461+
dataScale: row[5],
462+
};
463+
if (!metaData.dataTypeOwner) {
464+
const startPos = row[1].indexOf('(');
465+
const endPos = row[1].indexOf(')');
466+
if (endPos > startPos) {
467+
metaData.dataType = metaData.dataType.substring(0, startPos) +
468+
metaData.dataType.substring(
469+
endPos + 1, metaData.dataType.length
470+
);
471+
}
472+
}
473+
this._addAttr(info.attributes, metaData);
474+
}
475+
} finally {
476+
val.resultSet.close();
477+
}
478+
}
479+
407480
//---------------------------------------------------------------------------
408481
// _populateDbObjectTypeInfo()
409482
//
@@ -449,26 +522,6 @@ class ThinConnectionImpl extends ConnectionImpl {
449522
end if;
450523
end;`;
451524

452-
// get column and datatype information in case of %ROWTYPE handling.
453-
const getColumnsSQL = `
454-
SELECT
455-
column_name,
456-
data_type,
457-
data_type_owner,
458-
case
459-
when data_type in
460-
('CHAR', 'NCHAR', 'VARCHAR2', 'NVARCHAR2', 'RAW')
461-
then data_length
462-
else 0
463-
end,
464-
nvl(data_precision, 0),
465-
nvl(data_scale, 0)
466-
from all_tab_cols
467-
where owner = :owner
468-
and table_name = :name
469-
and hidden_column != 'YES'
470-
order by column_id`;
471-
472525
const binds = [
473526
{
474527
name: "full_name",
@@ -540,78 +593,42 @@ class ThinConnectionImpl extends ConnectionImpl {
540593
errors.throwErr(errors.ERR_INVALID_OBJECT_TYPE_NAME, name);
541594
}
542595

543-
// check cache; if already present, nothing more to do!
544-
const info = this._getDbObjectType(result.outBinds.schema,
545-
result.outBinds.name, result.outBinds.package_name, result.outBinds.oid);
546-
if (!info.partial)
547-
return info;
548-
549-
// process TDS and attributes cursor
550-
if (info.name.endsWith('%ROWTYPE')) {
551-
const bindVal = [
552-
{
553-
name: "owner",
554-
type: types.DB_TYPE_VARCHAR,
555-
maxSize: 128,
556-
dir: constants.BIND_IN,
557-
values: [result.outBinds.schema],
558-
},
559-
{
560-
name: "name",
561-
type: types.DB_TYPE_VARCHAR,
562-
maxSize: 128,
563-
dir: constants.BIND_IN,
564-
values: [info.name.substring(0, info.name.length - 8)]
565-
}
566-
];
567-
const val = await this.execute(
568-
getColumnsSQL, 1, bindVal, options, false
569-
);
570-
const attrRows = await val.resultSet._getAllRows();
571-
info.attributes = [];
572-
for (const row of attrRows) {
573-
const metaData = {
574-
name: row[0],
575-
dataType: row[1],
576-
dataTypeOwner: row[2],
577-
maxSize: row[3],
578-
dataPrecision: row[4],
579-
dataScale: row[5],
580-
};
581-
if (!metaData.dataTypeOwner) {
582-
const startPos = row[1].indexOf('(');
583-
const endPos = row[1].indexOf(')');
584-
if (endPos > startPos) {
585-
metaData.dataType = metaData.dataType.substring(0, startPos) +
586-
metaData.dataType.substring(
587-
endPos + 1, metaData.dataType.length
588-
);
589-
}
590-
}
591-
this._addAttr(info.attributes, metaData);
596+
try {
597+
// check cache; if already present, nothing more to do!
598+
const info = this._getDbObjectType(result.outBinds.schema,
599+
result.outBinds.name, result.outBinds.package_name, result.outBinds.oid);
600+
if (!info.partial) {
601+
return info;
592602
}
593-
} else {
594-
info.version = result.outBinds.version;
595-
const attrRows = await result.outBinds.attrs_rc._getAllRows();
596-
if (attrRows.length > 0) {
603+
604+
// process TDS and attributes cursor
605+
if (info.name.endsWith('%ROWTYPE')) {
606+
await this._populateRowTypeInfo(info, options, result);
607+
} else {
608+
info.version = result.outBinds.version;
609+
const attrRows = await result.outBinds.attrs_rc._getAllRows();
610+
if (attrRows.length > 0) {
597611
// Its an object not a collection.
598-
info.attributes = [];
599-
for (const row of attrRows) {
600-
const metaData = {
601-
name: row[1],
602-
dataType: row[3],
603-
dataTypeOwner: row[4],
604-
packageName: row[5],
605-
oid: row[6]
606-
};
607-
this._addAttr(info.attributes, metaData);
608-
}
612+
info.attributes = [];
613+
for (const row of attrRows) {
614+
const metaData = {
615+
name: row[1],
616+
dataType: row[3],
617+
dataTypeOwner: row[4],
618+
packageName: row[5],
619+
oid: row[6]
620+
};
621+
this._addAttr(info.attributes, metaData);
622+
}
609623

624+
}
625+
await this._parseTDS(result.outBinds.tds, info);
610626
}
611-
await this._parseTDS(result.outBinds.tds, info);
627+
info.partial = false;
628+
return info;
629+
} finally {
630+
result.outBinds.attrs_rc.close();
612631
}
613-
info.partial = false;
614-
return info;
615632

616633
}
617634

‎test/dbObject20.js‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,4 +1269,59 @@ describe('290. dbObject20.js', () => {
12691269
});
12701270
});
12711271

1272+
describe('290.7 Check cursor close behaviour', function() {
1273+
let pool, sysDBAConn, sid;
1274+
const TYPE1 = 'NODB_TYP_NODB_MASTER_ID_ARR';
1275+
1276+
before(async () => {
1277+
if (!dbConfig.test.DBA_PRIVILEGE) this.skip();
1278+
1279+
const dbaConfig = {
1280+
user: dbConfig.test.DBA_user,
1281+
password: dbConfig.test.DBA_password,
1282+
connectionString: dbConfig.connectString,
1283+
privilege: oracledb.SYSDBA
1284+
};
1285+
sysDBAConn = await oracledb.getConnection(dbaConfig);
1286+
pool = await oracledb.createPool({
1287+
user: dbConfig.user,
1288+
password: dbConfig.password,
1289+
connectString: dbConfig.connectString,
1290+
poolMin: 1});
1291+
const conn = await pool.getConnection();
1292+
const sql =
1293+
`CREATE OR REPLACE TYPE ${TYPE1} IS TABLE OF NUMBER`;
1294+
await testsUtil.createType(conn, TYPE1, sql);
1295+
sid = await testsUtil.getSid(conn);
1296+
await conn.close();
1297+
}); // before()
1298+
1299+
after(async () => {
1300+
if (sysDBAConn) {
1301+
await sysDBAConn.close();
1302+
}
1303+
if (pool) {
1304+
const conn = await pool.getConnection();
1305+
await testsUtil.dropType(conn, TYPE1);
1306+
await conn.close();
1307+
await pool.close(0);
1308+
}
1309+
}); // after()
1310+
1311+
// https://github.com/oracle/node-oracledb/issues/1694
1312+
it('290.7.1 create and delete DB object instance in a loop should not cause cursor leak ', async () => {
1313+
const iterations = 100;
1314+
const openCount = await testsUtil.getOpenCursorCount(sysDBAConn, sid);
1315+
for (let i = 0; i < iterations; i++) {
1316+
const connection = await pool.getConnection();
1317+
await connection.getDbObjectClass(TYPE1);
1318+
await connection.close();
1319+
}
1320+
const newOpenCount = await testsUtil.getOpenCursorCount(sysDBAConn, sid);
1321+
1322+
// ensure cursors are not linearly opened as iterations causing leak.
1323+
assert(newOpenCount - openCount < 5);
1324+
});
1325+
});
1326+
12721327
});

‎test/list.txt‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ Overview of node-oracledb functional tests
140140
2.19.1 DBA user with SYSDBA privilege
141141
2.20 settable parameters
142142
2.20.1 Check parameter values after user update
143-
2.20.2 negative - Check parameter value types
143+
2.20.2 negative - Check parameter value types
144144

145145
3. examples.js
146146
3.1 connect.js
@@ -5734,6 +5734,8 @@ oracledb.OUT_FORMAT_OBJECT and resultSet = true
57345734
290.5.1 verify associative array outbinds
57355735
290.6 db Object tests with XML Value type
57365736
290.6.1 Verify XML value and metaData inside object
5737+
290.7 Check cursor close behaviour
5738+
290.7.1 create and delete DB object instance in a loop should not cause cursor leak
57375739

57385740
291. dbSchema.js
57395741
291.1 dbSchema and Annotations
@@ -5913,6 +5915,7 @@ oracledb.OUT_FORMAT_OBJECT and resultSet = true
59135915
304. plsqlRowtype.js
59145916
304.1 %ROWTYPE
59155917
304.2 %ROWTYPE collection
5918+
304.3 %ROWTYPE object create and delete in a loop to check cursor leak
59165919

59175920
305. dataTypeVector5.js
59185921
305.1 tests with vector distance
@@ -5959,4 +5962,4 @@ oracledb.OUT_FORMAT_OBJECT and resultSet = true
59595962
307.1.2 getConnection behaviour with a custom instance enabling trace
59605963
307.1.3 getConnection behaviour with a custom instance disabling trace
59615964
307.1.4 test getConnection fail behaviour with custom instance enabling trace
5962-
307.1.5 test getConnection fail behaviour with custom instance disabling trace
5965+
307.1.5 test getConnection fail behaviour with custom instance disabling trace

‎test/plsqlRowtype.js‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const assert = require('assert');
3737
const testsUtil = require('./testsUtil.js');
3838

3939
describe('304. plSqlRowType.js', function() {
40-
let connection;
40+
let connection, pool, sysDBAConn;
4141
const table = 'NODB_ROWTYPE';
4242
const typeName = `NODB_OBJ`;
4343
const stmts = [
@@ -136,6 +136,11 @@ describe('304. plSqlRowType.js', function() {
136136
};
137137

138138
before(async function() {
139+
pool = await oracledb.createPool({
140+
user: dbConfig.user,
141+
password: dbConfig.password,
142+
connectString: dbConfig.connectString,
143+
poolMin: 1});
139144
connection = await oracledb.getConnection(dbConfig);
140145
await testsUtil.createType(connection, typeName, ObjSql);
141146
await testsUtil.createTable(connection, table, createTableSql);
@@ -156,6 +161,12 @@ describe('304. plSqlRowType.js', function() {
156161
await testsUtil.dropType(connection, typeName);
157162
await connection.execute(dropPackageSql);
158163
await connection.close();
164+
if (sysDBAConn) {
165+
await sysDBAConn.close();
166+
}
167+
if (pool) {
168+
await pool.close(0);
169+
}
159170
});
160171

161172
it('304.1 %ROWTYPE', async function() {
@@ -172,4 +183,31 @@ describe('304. plSqlRowType.js', function() {
172183
assert.deepStrictEqual(expectedTypes, types);
173184
}); // 304.2
174185

186+
it('304.3 %ROWTYPE object create and delete in a loop to check cursor leak', async function() {
187+
if (!dbConfig.test.DBA_PRIVILEGE) this.skip();
188+
189+
const name = "NODB_ROWTYPE%ROWTYPE";
190+
const iterations = 100;
191+
const dbaConfig = {
192+
user: dbConfig.test.DBA_user,
193+
password: dbConfig.test.DBA_password,
194+
connectionString: dbConfig.connectString,
195+
privilege: oracledb.SYSDBA
196+
};
197+
const connection = await pool.getConnection();
198+
const sid = await testsUtil.getSid(connection);
199+
await connection.close();
200+
sysDBAConn = await oracledb.getConnection(dbaConfig);
201+
const openCount = await testsUtil.getOpenCursorCount(sysDBAConn, sid);
202+
for (let i = 0; i < iterations; i++) {
203+
const connection = await pool.getConnection();
204+
await connection.getDbObjectClass(name);
205+
await connection.close();
206+
}
207+
const newOpenCount = await testsUtil.getOpenCursorCount(sysDBAConn, sid);
208+
209+
// ensure cursors are not linearly opened as iterations causing leak.
210+
assert(newOpenCount - openCount < 5);
211+
}); // 304.3
212+
175213
}); // 304

0 commit comments

Comments
 (0)