Skip to content

Commit 550cc13

Browse files
cjbjcjbj
committed
More LOB tweaks: fix a mem leak with NULL LOBs; consistently return NULL for string/buffer of EMPTY_LOB; Auto-close (where possible) the IN-bit of a LOB bind used for BIND_INOUT
1 parent 58b5475 commit 550cc13

File tree

5 files changed

+150
-47
lines changed

5 files changed

+150
-47
lines changed

‎src/njs/src/njsConnection.cpp

Lines changed: 83 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1960,6 +1960,26 @@ void Connection::Async_Execute (uv_work_t *req)
19601960
executeBaton->dpistmt->release ();
19611961
}
19621962

1963+
// Auto close the IN-LOB used for INOUT bind
1964+
for ( unsigned int index = 0 ;index < executeBaton->binds.size ();
1965+
index++ )
1966+
{
1967+
Bind *bind = executeBaton->binds[index];
1968+
1969+
if ( ( bind->type == DpiClob || bind->type == DpiBlob ) && bind->isInOut )
1970+
{
1971+
if ( executeBaton->extBinds[index] &&
1972+
executeBaton->extBinds[index]->type == NJS_EXTBIND_LOB &&
1973+
!executeBaton->extBinds[index]->fields.extLob.isStringBuffer2LOB )
1974+
{
1975+
ILob *iLob = ( ILob * )
1976+
( executeBaton->extBinds[index]->fields.extLob.value );
1977+
// cleanupNJS() will be called later in Async_AfterExecute()
1978+
iLob->cleanupDPI ();
1979+
}
1980+
}
1981+
}
1982+
19631983
// In case of error, free the allocated resources
19641984
if ( !(executeBaton->error).empty() )
19651985
{
@@ -2103,14 +2123,24 @@ void Connection::LOB2StringOrBuffer ( eBaton* executeBaton, unsigned int index,
21032123
lobLocator, byteAmount, charAmount, offset,
21042124
bind->value, bufLen );
21052125

2106-
// There is more data in the LOB than the maxSize, set the error
2126+
/*
2127+
* byteAmount returns the number of bytes read into the buffer irrespective
2128+
* of charAmount or byteAmount passed to Lob::read()
2129+
* If there is more data in the LOB than the maxSize, set the error
2130+
*/
21072131
if ( byteAmount > ( unsigned long long ) bind->maxSize )
21082132
{
21092133
executeBaton->error = NJSMessages::getErrorMsg(
21102134
errInsufficientBufferForBinds);
21112135
goto exitLOB2StringOrBuffer;
21122136
}
21132137

2138+
// Treat empty LOB case as NULL to be consistent with varchar/buffer columns
2139+
if ( !byteAmount )
2140+
{
2141+
*bind->ind = -1;
2142+
}
2143+
21142144
*bind->len = ( DPI_BUFLEN_TYPE ) byteAmount;
21152145

21162146
exitLOB2StringOrBuffer:
@@ -3390,40 +3420,58 @@ void Connection::Descr2protoILob( eBaton *executeBaton, unsigned int numCols,
33903420
{
33913421
Bind *bind = executeBaton->binds[obndpos];
33923422

3393-
if ((!bind->isOut && !bind->isInOut) ||
3394-
( bind->ind && *(sb2 *)bind->ind == -1))
3395-
continue; // we only need to process the non-null OUT binds
3396-
3397-
if (bind->type == DpiClob || bind->type == DpiBlob)
3423+
if ( ( bind->isOut || bind->isInOut ) &&
3424+
( bind->type == DpiClob || bind->type == DpiBlob ) )
33983425
{
3399-
Descriptor *lobLocator = NULL;
3400-
3401-
if ( executeBaton->stmtIsReturning )
3426+
// Free the allocated descriptors in case of NULL OUTs or DML returning
3427+
if ( *bind->ind == -1 )
34023428
{
3403-
// TODO: Update this loop when support for array binds is added.
3404-
// For UPDATE, loop through all the rows returned for each bind
3405-
// For INSERT, eventually we will consider the iters.
3406-
// For now only 1 row will be inserted.
3407-
for ( unsigned int rowidx = 0; rowidx < bind->rowsReturned; rowidx++ )
3429+
if ( executeBaton->stmtIsReturning )
34083430
{
3409-
lobLocator =
3410-
( ( Descriptor ** ) bind->value )[ rowidx ];
3411-
ProtoILob *protoILob = new ProtoILob ( executeBaton, lobLocator,
3412-
bind->type );
3413-
3414-
( ( Descriptor ** )( bind->value ) )[rowidx] =
3415-
reinterpret_cast<Descriptor *>( protoILob );
3431+
for ( unsigned int rowidx = 0; rowidx < bind->rowsReturned;
3432+
rowidx++ )
3433+
{
3434+
Env::freeDescriptor ( ( ( Descriptor ** ) bind->value )[ rowidx ],
3435+
LobDescriptorType);
3436+
}
3437+
}
3438+
else
3439+
{
3440+
Env::freeDescriptor ( *( ( Descriptor ** ) bind->value ),
3441+
LobDescriptorType);
34163442
}
34173443
}
3418-
else // case for PLSQL INOUT or OUT
3444+
else
34193445
{
3420-
lobLocator = *( ( Descriptor ** ) bind->value );
3446+
Descriptor *lobLocator = NULL;
3447+
3448+
if ( executeBaton->stmtIsReturning )
3449+
{
3450+
// TODO: Update this loop when support for array binds is added.
3451+
// For UPDATE, loop through all the rows returned for each bind
3452+
// For INSERT, eventually we will consider the iters.
3453+
// For now only 1 row will be inserted.
3454+
for ( unsigned int rowidx = 0; rowidx < bind->rowsReturned; rowidx++ )
3455+
{
3456+
lobLocator =
3457+
( ( Descriptor ** ) bind->value )[ rowidx ];
3458+
ProtoILob *protoILob = new ProtoILob ( executeBaton, lobLocator,
3459+
bind->type );
34213460

3422-
ProtoILob *protoILob = new ProtoILob ( executeBaton,
3423-
lobLocator,
3424-
bind->type );
3425-
*((Descriptor **)bind->value) =
3426-
reinterpret_cast<Descriptor *>( protoILob );
3461+
( ( Descriptor ** )( bind->value ) )[rowidx] =
3462+
reinterpret_cast<Descriptor *>( protoILob );
3463+
}
3464+
}
3465+
else
3466+
{
3467+
lobLocator = *( ( Descriptor ** ) bind->value );
3468+
3469+
ProtoILob *protoILob = new ProtoILob ( executeBaton,
3470+
lobLocator,
3471+
bind->type );
3472+
*((Descriptor **)bind->value) =
3473+
reinterpret_cast<Descriptor *>( protoILob );
3474+
}
34273475
}
34283476
}
34293477
}
@@ -3465,6 +3513,13 @@ void Connection::Async_AfterExecute(uv_work_t *req)
34653513
( executeBaton->extBinds[index]->fields.extLob.value );
34663514
iLob->postBind ();
34673515

3516+
// Auto-close the IN-LOB used for INOUT bind
3517+
if ( bind->isInOut )
3518+
{
3519+
// cleanupDPI() called in Async_Execute since it is a DPI/OCI call
3520+
iLob->cleanupNJS ();
3521+
}
3522+
34683523
executeBaton->extBinds[index]->fields.extLob.value = NULL;
34693524
}
34703525
}

‎src/njs/src/njsIntLob.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,14 +185,16 @@ class ILob : public Nan::ObjectWrap
185185

186186
static void Init(Handle<Object> target);
187187

188+
void cleanupDPI ();
189+
190+
void cleanupNJS ();
191+
188192

189193
private:
190194
ILob();
191195

192196
~ILob();
193197

194-
void cleanupDPI ();
195-
void cleanupNJS ();
196198
inline NJSErrorType getErrNumber ( bool processBind );
197199

198200
static NAN_METHOD(New);

‎test/devnull.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/* Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved. */
2+
3+
/******************************************************************************
4+
*
5+
* You may not use the identified files except in compliance with the Apache
6+
* License, Version 2.0 (the "License.")
7+
*
8+
* You may obtain a copy of the License at
9+
* http://www.apache.org/licenses/LICENSE-2.0.
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
13+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
*
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*
18+
* The node-oracledb test suite uses 'mocha', 'should' and 'async'.
19+
* See LICENSE.md for relevant licenses.
20+
*
21+
* NAME
22+
* devnull.js
23+
*
24+
* DESCRIPTION
25+
* /dev/null for Node streams
26+
*
27+
*****************************************************************************/
28+
'use strict';
29+
30+
var stream = require('stream');
31+
var util = require('util');
32+
33+
module.exports = DevNull;
34+
35+
// step 2 - to call the Writable constructor in our own constructor
36+
function DevNull(opts) {
37+
if ( !(this instanceof DevNull) ) return new DevNull(opts);
38+
39+
opts = opts || {};
40+
stream.Writable.call(this, opts);
41+
42+
};
43+
44+
// step 1 - to extend the Writable Class
45+
util.inherits(DevNull, stream.Writable);
46+
47+
// step 3 -define a '_write()' method in the prototype of our stream object
48+
DevNull.prototype._write = function(chunk, encoding, cb) {
49+
setImmediate(cb);
50+
};

‎test/list.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1055,7 +1055,7 @@ Overview of node-oracledb functional tests
10551055
71.1.4 BIND_IN, PL/SQL, a txt file
10561056
71.1.5 BIND_OUT, PL/SQL, a string
10571057
71.1.6 BIND_OUT, PL/SQL, a txt file
1058-
71.1.7 BIND_INOUT, PL/SQL, A String
1058+
71.1.7 BIND_INOUT, PL/SQL, A String. IN LOB gets auto closed.
10591059
71.1.8 BIND_INOUT, PL/SQL, a txt file
10601060
71.1.9 Negative: BIND_INOUT, PL/SQL, String as bind IN Value
10611061
71.2 persistent BLOB

‎test/lobBind1.js

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ var should = require('should');
3939
var async = require('async');
4040
var dbConfig = require('./dbconfig.js');
4141
var assist = require('./dataTypeAssist.js');
42+
var devnull = require('./devnull');
4243

4344
describe('71. lobBind1.js', function() {
4445

@@ -579,7 +580,7 @@ describe('71. lobBind1.js', function() {
579580

580581
}); // 71.1.6
581582

582-
it('71.1.7 BIND_INOUT, PL/SQL, A String. IN LOB value does not change.', function(done) {
583+
it('71.1.7 BIND_INOUT, PL/SQL, A String. IN LOB gets auto closed.', function(done) {
583584

584585
var seq = 7;
585586
var inStr = "I love the sunshine today!",
@@ -621,13 +622,13 @@ describe('71. lobBind1.js', function() {
621622
should.not.exist(err);
622623
(result.rows.length).should.not.eql(0);
623624

624-
var lob = result.rows[0][0];
625+
var lobin = result.rows[0][0];
625626

626627
connection.execute(
627628
sql2,
628629
{
629630
id: seq,
630-
io: { val: lob, type: oracledb.CLOB, dir: oracledb.BIND_INOUT }
631+
io: { val: lobin, type: oracledb.CLOB, dir: oracledb.BIND_INOUT }
631632
},
632633
{ autoCommit: true },
633634
function(err, result2) {
@@ -636,7 +637,7 @@ describe('71. lobBind1.js', function() {
636637
var lobout = result2.outBinds.io;
637638

638639
async.parallel([
639-
function(callback) {
640+
function verifyOUT(callback) {
640641
lobout.setEncoding("utf8");
641642
var clobData = "";
642643

@@ -654,23 +655,18 @@ describe('71. lobBind1.js', function() {
654655
return callback();
655656
});
656657
},
657-
function(callback) {
658-
lob.setEncoding("utf8");
659-
var clobData = "";
660-
661-
lob.on("data", function(chunk) {
662-
clobData += chunk;
663-
});
658+
function verifyIN(callback) {
664659

665-
lob.on("error", function(err) {
666-
should.not.exist(err, "lob.on 'error' event.");
667-
});
660+
lobin.pipe(devnull());
668661

669-
lob.on("end", function() {
670-
should.strictEqual(clobData, inStr);
662+
lobin.on('error', function(err) {
663+
should.exist(err);
664+
(err.message).should.startWith('NJS-022:');
665+
// Error: NJS-022: invalid Lob
671666

672667
return callback();
673668
});
669+
674670
}
675671
], function(err) {
676672
should.not.exist(err);

0 commit comments

Comments
 (0)