Skip to content

Commit 29baa93

Browse files
cjbjcjbj
committed
Eliminate a JavaScript memory leak
1 parent fb52b59 commit 29baa93

File tree

8 files changed

+171
-173
lines changed

8 files changed

+171
-173
lines changed

‎CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
# Change Log
22

3+
## node-oracledb v4.0.2 (DD Mon YYYY)
4+
5+
**This release is under development**
6+
7+
- Fixed a JavaScript memory leak when getting Oracle Database named type
8+
information, such as with `getDbObjectClass()`.
9+
310
## node-oracledb v4.0.1 (19 Aug 2019)
411

512
- Fixed a regression causing a segfault when setting `oracledb.connectionClass`

‎lib/connection.js

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,10 @@ function close(a1, a2) {
291291

292292
self._close(options, function(err) {
293293
if (!err) {
294+
for (const cls of Object.values(this._dbObjectClasses)) {
295+
cls.prototype.constructor = Object;
296+
cls.prototype = null;
297+
}
294298
self.emit('_after_close');
295299
}
296300

@@ -376,6 +380,31 @@ function unsubscribe(name, cb) {
376380
self._unsubscribe.call(self, name, cb);
377381
}
378382

383+
// build a database object class
384+
function buildDbObjectClass(schema, name, fqn) {
385+
const DbObject = function(initialValue) {
386+
if (this.isCollection) {
387+
const proxy = new Proxy(this, BaseDbObject._collectionProxyHandler);
388+
if (initialValue !== undefined) {
389+
for (let i = 0; i < initialValue.length; i++) {
390+
this.append(initialValue[i]);
391+
}
392+
}
393+
return proxy;
394+
} else if (initialValue !== undefined) {
395+
Object.assign(this, initialValue)
396+
}
397+
}
398+
DbObject.prototype = Object.create(BaseDbObject.prototype);
399+
DbObject.prototype.constructor = DbObject;
400+
DbObject.prototype.schema = schema;
401+
DbObject.prototype.name = name;
402+
DbObject.prototype.fqn = fqn;
403+
DbObject.toString = function() {
404+
return 'DbObjectClass [' + fqn + ']';
405+
};
406+
return DbObject;
407+
}
379408

380409
// define class
381410
class Connection extends EventEmitter {
@@ -409,18 +438,8 @@ class Connection extends EventEmitter {
409438
const fqn = `${schema}.${name}`;
410439
let cls = this._dbObjectClasses[fqn];
411440
if (!cls) {
412-
class DbObject extends BaseDbObject {
413-
get [Symbol.toStringTag]() {
414-
return fqn;
415-
}
416-
}
417-
this._dbObjectClasses[fqn] = cls = DbObject;
418-
cls.prototype.schema = schema;
419-
cls.prototype.name = name;
420-
cls.prototype.fqn = fqn;
421-
cls.toString = function() {
422-
return 'DbObjectClass [' + fqn + ']';
423-
};
441+
cls = buildDbObjectClass(schema, name, fqn);
442+
this._dbObjectClasses[fqn] = cls;
424443
}
425444
return cls;
426445
}

‎lib/dbObject.js

Lines changed: 64 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -21,79 +21,27 @@
2121

2222
const util = require('util');
2323

24-
// define proxy handler used for collections
25-
const collectionProxyHandler = {
26-
27-
deleteProperty(target, prop) {
28-
if (typeof prop === 'string') {
29-
const index = +prop;
30-
if (!isNaN(index)) {
31-
return target.deleteElement(index);
32-
}
33-
}
34-
return delete target[prop];
35-
},
36-
37-
get(target, prop) {
38-
if (typeof prop === 'string') {
39-
40-
// when binding collections, we must be consistent in getting the target
41-
// of the proxy, since napi_wrap() called on the proxy will not be
42-
// available when calling napi_unwrap() on the target; this property
43-
// forces the target to get returned
44-
if (prop === '_target') {
45-
return target;
46-
}
47-
const index = +prop;
48-
if (!isNaN(index)) {
49-
return target.getElement(index);
50-
}
51-
}
52-
const value = target[prop];
53-
if (typeof value === 'function') {
54-
return value.bind(target);
55-
}
56-
return value;
57-
},
58-
59-
set(target, prop, value) {
60-
if (typeof prop === 'string') {
61-
const index = +prop;
62-
if (!isNaN(index)) {
63-
target.setElement(index, value);
64-
return true;
65-
}
66-
}
67-
target[prop] = value;
68-
return true;
69-
}
70-
71-
};
72-
7324
// define base database object class; instances of this class are never
7425
// instantiated; instead, classes subclassed from this one will be
7526
// instantiated; a cache of these classes are maintained on each connection
7627
class BaseDbObject {
7728

78-
constructor(initialValue) {
29+
// extend class with promisified functions
30+
_extend(oracledb) {
31+
this._oracledb = oracledb;
32+
}
33+
34+
// initialize object with value
35+
_initialize(initialValue) {
7936
if (this.isCollection) {
80-
const proxy = new Proxy(this, collectionProxyHandler);
81-
if (initialValue) {
82-
for (let i = 0; i < initialValue.length; i++) {
83-
this.append(initialValue[i]);
84-
}
37+
for (let i = 0; i < initialValue.length; i++) {
38+
this.append(initialValue[i]);
8539
}
86-
return proxy;
87-
} else if (initialValue) {
40+
} else {
8841
Object.assign(this, initialValue);
8942
}
9043
}
9144

92-
// extend class with promisified functions
93-
_extend(oracledb) {
94-
this._oracledb = oracledb;
95-
}
96-
9745
// return as a plain object
9846
_toPojo() {
9947
if (this.isCollection) {
@@ -132,11 +80,65 @@ class BaseDbObject {
13280
}
13381
}
13482

83+
get [Symbol.toStringTag]() {
84+
return this.fqn;
85+
}
86+
13587
toJSON() {
13688
return this._toPojo();
13789
}
13890

13991
}
14092

14193

94+
// define proxy handler used for collections
95+
BaseDbObject._collectionProxyHandler = {
96+
97+
deleteProperty(target, prop) {
98+
if (typeof prop === 'string') {
99+
const index = +prop;
100+
if (!isNaN(index)) {
101+
return target.deleteElement(index);
102+
}
103+
}
104+
return delete target[prop];
105+
},
106+
107+
get(target, prop) {
108+
if (typeof prop === 'string') {
109+
110+
// when binding collections, we must be consistent in getting the target
111+
// of the proxy, since napi_wrap() called on the proxy will not be
112+
// available when calling napi_unwrap() on the target; this property
113+
// forces the target to get returned
114+
if (prop === '_target') {
115+
return target;
116+
}
117+
const index = +prop;
118+
if (!isNaN(index)) {
119+
return target.getElement(index);
120+
}
121+
}
122+
const value = target[prop];
123+
if (typeof value === 'function') {
124+
return value.bind(target);
125+
}
126+
return value;
127+
},
128+
129+
set(target, prop, value) {
130+
if (typeof prop === 'string') {
131+
const index = +prop;
132+
if (!isNaN(index)) {
133+
target.setElement(index, value);
134+
return true;
135+
}
136+
}
137+
target[prop] = value;
138+
return true;
139+
}
140+
141+
};
142+
143+
142144
module.exports = BaseDbObject;

‎src/njsConnection.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2350,8 +2350,9 @@ static bool njsConnection_scanExecuteBindUnit(njsBaton *baton,
23502350
NJS_CHECK_NAPI(env, napi_get_named_property(env, args[1], "type", &value));
23512351
NJS_CHECK_NAPI(env, napi_typeof(env, value, &valueType));
23522352
if (valueType == napi_function) {
2353-
NJS_CHECK_NAPI(env, napi_get_prototype(env, value, &prototype))
2354-
NJS_CHECK_NAPI(env, napi_strict_equals(env, prototype,
2353+
NJS_CHECK_NAPI(env, napi_get_named_property(env, value, "prototype",
2354+
&prototype))
2355+
NJS_CHECK_NAPI(env, napi_instanceof(env, prototype,
23552356
baton->jsBaseDbObjectConstructor, &check))
23562357
if (!check)
23572358
return njsBaton_setError(baton, errInvalidBindDataType, 2);

0 commit comments

Comments
 (0)