Skip to content

Commit 8dadfbd

Browse files
committed
Fix potential cursor issues while using DRCP
1 parent 586e71c commit 8dadfbd

File tree

5 files changed

+181
-87
lines changed

5 files changed

+181
-87
lines changed

Diff for: doc/src/release_notes.rst

+2
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ Common Changes
3333
Thin Mode Changes
3434
++++++++++++++++++
3535

36+
#) Fixed potential cursor issues when using DRCP.
37+
3638
#) Fixed bug in reading PLS_INTEGER type when used in PL/SQL records.
3739

3840
#) Error ``NJS-141: errors in array DML exceed 65535`` is now raised

Diff for: lib/thin/connection.js

+8-79
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ const process = require('process');
4141
const types = require('../types.js');
4242
const errors = require("../errors.js");
4343
const messages = require('./protocol/messages');
44+
const StatementCache = require('./statementCache.js');
4445

4546
const finalizationRegistry = new global.FinalizationRegistry((heldValue) => {
4647
heldValue.disconnect();
@@ -626,11 +627,8 @@ class ThinConnectionImpl extends ConnectionImpl {
626627
this.statementCache = null;
627628
this.currentSchema = "";
628629
this.invokeSessionCallback = true;
629-
this.statementCache = new Map();
630630
this.statementCacheSize = params.stmtCacheSize;
631-
this._numCursorsToClose = 0;
632631
this._currentSchemaModified = false;
633-
this._cursorsToClose = new Set();
634632
this._tempLobsToClose = [];
635633
this._tempLobsTotalSize = 0;
636634
this._drcpEstablishSession = false;
@@ -734,6 +732,7 @@ class ThinConnectionImpl extends ConnectionImpl {
734732
throw err;
735733
}
736734

735+
this.statementCache = new StatementCache(this.statementCacheSize);
737736
// maintain a list of partially populated database object types
738737
this._partialDbObjectTypes = [];
739738

@@ -746,51 +745,10 @@ class ThinConnectionImpl extends ConnectionImpl {
746745
}
747746

748747
//---------------------------------------------------------------------------
749-
// Sets that a statement is no longer in use
748+
// Return the statement to the statement cache, if applicable
750749
//---------------------------------------------------------------------------
751750
_returnStatement(statement) {
752-
if (statement.bindInfoList) {
753-
statement.bindInfoList.forEach(bindInfo => {
754-
bindInfo.bindVar = null;
755-
});
756-
}
757-
if (statement.queryVars) {
758-
statement.queryVars.forEach(queryVar => {
759-
queryVar.values.fill(null);
760-
});
761-
}
762-
if (statement.returnToCache) {
763-
statement.inUse = false;
764-
} else if (statement.cursorId !== 0) {
765-
this._addCursorToClose(statement.cursorId);
766-
}
767-
}
768-
769-
//---------------------------------------------------------------------------
770-
// Adds the cursors that needs to be closed to the _cursorsToClose set
771-
//---------------------------------------------------------------------------
772-
_addCursorToClose(cursorId) {
773-
if (this._cursorsToClose.has(cursorId)) {
774-
const reason = `attempt to close cursor ${cursorId} twice`;
775-
errors.throwErr(errors.ERR_INTERNAL, reason);
776-
}
777-
this._cursorsToClose.add(cursorId);
778-
}
779-
780-
//---------------------------------------------------------------------------
781-
// Adjusts the statement cache to remove least recently used statements
782-
//---------------------------------------------------------------------------
783-
_adjustStatementCache() {
784-
while (this.statementCache.size > this.statementCacheSize) {
785-
const sql = this.statementCache.keys().next().value;
786-
const stmt = this.statementCache.get(sql);
787-
this.statementCache.delete(sql);
788-
if (stmt.inUse) {
789-
stmt.returnToCache = false;
790-
} else if (stmt.cursorId !== 0) {
791-
this._addCursorToClose(stmt.cursorId);
792-
}
793-
}
751+
this.statementCache.returnStatement(statement);
794752
}
795753

796754
//---------------------------------------------------------------------------
@@ -900,14 +858,6 @@ class ThinConnectionImpl extends ConnectionImpl {
900858
return resultSet;
901859
}
902860

903-
//---------------------------------------------------------------------------
904-
// Clears the statement cache for the connection
905-
//---------------------------------------------------------------------------
906-
resetStatementCache() {
907-
this.statementCache.clear();
908-
this._cursorsToClose.clear();
909-
}
910-
911861
//---------------------------------------------------------------------------
912862
// getDbObjectClass()
913863
//
@@ -972,31 +922,10 @@ class ThinConnectionImpl extends ConnectionImpl {
972922
// new session is going to be used.
973923
//---------------------------------------------------------------------------
974924
_getStatement(sql, cacheStatement = false) {
975-
let statement = this.statementCache.get(sql);
976-
if (!statement) {
977-
statement = new Statement();
978-
statement._prepare(sql);
979-
if (cacheStatement && !this._drcpEstablishSession && !statement.isDdl &&
980-
this.statementCacheSize > 0) {
981-
statement.returnToCache = true;
982-
this.statementCache.set(sql, statement);
983-
this._adjustStatementCache();
984-
}
985-
} else if (statement.inUse || !cacheStatement ||
986-
this._drcpEstablishSession) {
987-
if (!cacheStatement) {
988-
this.statementCache.delete(sql);
989-
statement.returnToCache = false;
990-
}
991-
if (statement.inUse || this._drcpEstablishSession) {
992-
statement = statement._copy();
993-
}
994-
} else {
995-
this.statementCache.delete(sql);
996-
this.statementCache.set(sql, statement);
925+
if (this._drcpEstablishSession) {
926+
cacheStatement = false;
997927
}
998-
statement.inUse = true;
999-
return statement;
928+
return this.statementCache.getStatement(sql, cacheStatement);
1000929
}
1001930

1002931
//---------------------------------------------------------------------------
@@ -1091,7 +1020,7 @@ class ThinConnectionImpl extends ConnectionImpl {
10911020
// the connection object
10921021
//---------------------------------------------------------------------------
10931022
getStmtCacheSize() {
1094-
return this.statementCacheSize;
1023+
return this.statementCache._maxSize;
10951024
}
10961025

10971026
setCallTimeout(timeout) {

Diff for: lib/thin/protocol/messages/base.js

+3-7
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ class Message {
282282
const flags = buf.readUB4(); // session flags
283283
if (flags & constants.TNS_SESSGET_SESSION_CHANGED) {
284284
if (this.connection._drcpEstablishSession) {
285-
this.connection.resetStatementCache();
285+
this.connection.statementCache.clearOpenCursors();
286286
}
287287
}
288288
this.connection._drcpEstablishSession = false;
@@ -297,7 +297,7 @@ class Message {
297297
if (this.connection._currentSchemaModified) {
298298
this._writeCurrentSchemaPiggyback(buf);
299299
}
300-
if (this.connection._cursorsToClose.size > 0 && !this.connection._drcpEstablishSession) {
300+
if (this.connection.statementCache._cursorsToClose.size > 0 && !this.connection._drcpEstablishSession) {
301301
this.writeCloseCursorsPiggyBack(buf);
302302
}
303303
if (
@@ -326,11 +326,7 @@ class Message {
326326
writeCloseCursorsPiggyBack(buf) {
327327
this.writePiggybackHeader(buf, constants.TNS_FUNC_CLOSE_CURSORS);
328328
buf.writeUInt8(1);
329-
buf.writeUB4(this.connection._cursorsToClose.size);
330-
for (const cursorNum of this.connection._cursorsToClose.keys()) {
331-
buf.writeUB4(cursorNum);
332-
}
333-
this.connection._cursorsToClose.clear();
329+
this.connection.statementCache.writeCursorsToClose(buf);
334330
}
335331

336332
writeCloseTempLobsPiggyback(buf) {

Diff for: lib/thin/protocol/messages/withData.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ class MessageWithData extends Message {
117117
this.errorOccurred = false;
118118
this.statement.moreRowsToFetch = false;
119119
} else if (this.errorInfo.num !== 0 && this.errorInfo.cursorId !== 0) {
120-
this.connection.statementCache.delete(this.statement.sql);
120+
this.connection.statementCache._cachedStatements.delete(this.statement.sql);
121121
this.statement.returnToCache = false;
122122
}
123123
if (this.errorInfo.batchErrors) {

Diff for: lib/thin/statementCache.js

+167
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
// Copyright (c) 2024, Oracle and/or its affiliates.
2+
3+
//-----------------------------------------------------------------------------
4+
//
5+
// This software is dual-licensed to you under the Universal Permissive License
6+
// (UPL) 1.0 as shown at https://oss.oracle.com/licenses/upl and Apache License
7+
// 2.0 as shown at http://www.apache.org/licenses/LICENSE-2.0. You may choose
8+
// either license.
9+
//
10+
// If you elect to accept the software under the Apache License, Version 2.0,
11+
// the following applies:
12+
//
13+
// Licensed under the Apache License, Version 2.0 (the "License");
14+
// you may not use this file except in compliance with the License.
15+
// You may obtain a copy of the License at
16+
//
17+
// https://www.apache.org/licenses/LICENSE-2.0
18+
//
19+
// Unless required by applicable law or agreed to in writing, software
20+
// distributed under the License is distributed on an "AS IS" BASIS,
21+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
22+
// See the License for the specific language governing permissions and
23+
// limitations under the License.
24+
//
25+
// Node file defining the StatementCache class used to manage cached statements
26+
//-----------------------------------------------------------------------------
27+
28+
const errors = require("../errors.js");
29+
const { Statement } = require("./statement");
30+
31+
class StatementCache {
32+
constructor(maxSize) {
33+
this._cachedStatements = new Map();
34+
this._maxSize = maxSize;
35+
this._cursorsToClose = new Set();
36+
this._openCursors = new Set();
37+
}
38+
39+
//---------------------------------------------------------------------------
40+
// _addCursorToClose()
41+
//
42+
// Add the statement's cursor to the list of cursors that need to be closed.
43+
//---------------------------------------------------------------------------
44+
_addCursorToClose(statement) {
45+
if (this._cursorsToClose.has(statement.cursorId)) {
46+
const reason = `attempt to close cursor ${statement.cursorId} twice`;
47+
errors.throwErr(errors.ERR_INTERNAL, reason);
48+
}
49+
if (statement.cursorId != 0) {
50+
this._cursorsToClose.add(statement.cursorId);
51+
}
52+
this._openCursors.delete(statement);
53+
}
54+
55+
//---------------------------------------------------------------------------
56+
// _adjustCache()
57+
// Adjust the cache so that no more than the maximum number of statements
58+
// are cached by removing least recently used statements
59+
//---------------------------------------------------------------------------
60+
_adjustCache() {
61+
while (this._cachedStatements.size > this._maxSize) {
62+
const sql = this._cachedStatements.keys().next().value;
63+
const stmt = this._cachedStatements.get(sql);
64+
this._cachedStatements.delete(sql);
65+
if (stmt.inUse) {
66+
stmt.returnToCache = false;
67+
} else if (stmt.cursorId !== 0) {
68+
this._addCursorToClose(stmt);
69+
}
70+
}
71+
}
72+
73+
//---------------------------------------------------------------------------
74+
//clearOpenCursors() {
75+
// Clears the list of open cursors and removes the list of cursors that
76+
// need to be closed. This is required when a DRCP session change has
77+
// taken place as the cursor ID values are invalidated.
78+
//---------------------------------------------------------------------------
79+
clearOpenCursors() {
80+
const newOpenCursors = new Set();
81+
for (const stmt of this._openCursors) {
82+
stmt.cursorId = 0;
83+
if (stmt.inUse) {
84+
newOpenCursors.add(stmt);
85+
}
86+
}
87+
this._openCursors = newOpenCursors;
88+
}
89+
90+
//---------------------------------------------------------------------------
91+
// get_statement()
92+
// Get a statement from the statement cache, or prepare a new statement
93+
// for use. If a statement is already in use or the statement is not
94+
// supposed to be cached, a copy will be made (and not returned to the
95+
// cache).
96+
//---------------------------------------------------------------------------
97+
getStatement(sql, cacheStatement = false) {
98+
let stmt = null;
99+
if (sql) {
100+
stmt = this._cachedStatements.get(sql);
101+
if (!cacheStatement) {
102+
this._openCursors.add(stmt);
103+
this._cachedStatements.delete(sql);
104+
}
105+
}
106+
if (!stmt) {
107+
stmt = new Statement();
108+
if (sql) {
109+
stmt._prepare(sql);
110+
}
111+
if (cacheStatement && !stmt.isDdl && this._maxSize > 0) {
112+
stmt.returnToCache = true;
113+
this._cachedStatements.set(sql, stmt);
114+
this._adjustCache();
115+
}
116+
this._openCursors.add(stmt);
117+
} else if (stmt.inUse || !cacheStatement) {
118+
stmt = stmt._copy();
119+
this._openCursors.add(stmt);
120+
} else {
121+
this._cachedStatements.delete(sql);
122+
this._cachedStatements.set(sql, stmt);
123+
}
124+
stmt.inUse = true;
125+
return stmt;
126+
}
127+
128+
//---------------------------------------------------------------------------
129+
// returnStatement()
130+
// Return the statement to the statement cache, if applicable. If the
131+
// statement must not be returned to the statement cache, add the cursor
132+
// id to the list of cursor ids to close on the next round trip to the
133+
// database. Clear all bind variables and fetch variables in order to
134+
// ensure that unnecessary references are not retained.
135+
//---------------------------------------------------------------------------
136+
returnStatement(statement) {
137+
if (statement.bindInfoList) {
138+
statement.bindInfoList.forEach(bindInfo => {
139+
bindInfo.bindVar = null;
140+
});
141+
}
142+
if (statement.queryVars) {
143+
statement.queryVars.forEach(queryVar => {
144+
queryVar.values.fill(null);
145+
});
146+
}
147+
if (statement.returnToCache) {
148+
statement.inUse = false;
149+
} else if (statement.cursorId !== 0) {
150+
this._addCursorToClose(statement);
151+
}
152+
}
153+
154+
//---------------------------------------------------------------------------
155+
// writeCursorsToClose()
156+
// Write the list of cursors to close to the buffer.
157+
//---------------------------------------------------------------------------
158+
writeCursorsToClose(buf) {
159+
buf.writeUB4(this._cursorsToClose.size);
160+
for (const cursorNum of this._cursorsToClose.keys()) {
161+
buf.writeUB4(cursorNum);
162+
}
163+
this._cursorsToClose.clear();
164+
}
165+
}
166+
167+
module.exports = StatementCache;

0 commit comments

Comments
 (0)