Skip to content
This repository has been archived by the owner on Jun 18, 2020. It is now read-only.

Commit

Permalink
Connection is not closed prematurely until a query is complete
Browse files Browse the repository at this point in the history
Closing a connection explicitly would close the connection immediately
rather than wait until ongoing operations were complete on that
connection.  The close operation now waits until all other operations
are finished.

Since there are legitimate times to forcibly close the connection
immediately, a new (optional) flag has been added to the close call:

conn.close( immediately, callback )

If the immediately parameter is supplied and is true, then the
connection will be closed rather than waiting for pending
operations/queries.  Note: if immediately is supplied but is not a
boolean, then an error is thrown.

close can still be called with either nothing or a callback as before.

Tests added to verify the new parameter.

Fixes #32
  • Loading branch information
jkint committed Mar 20, 2013
1 parent ae38572 commit 396750b
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 4 deletions.
28 changes: 26 additions & 2 deletions lib/sql.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,11 +343,25 @@ function open(connectionString, callback) {

function Connection() {

this.close = function (callback) {
this.close = function (immediately, callback) {

// require only callback
if( typeof immediately == 'function' ) {

callback = immediately;
immediately = false;
}
else if( typeof immediately != 'boolean' && typeof immediately != 'undefined' ) {

throw new Error( "[msnodesql] Invalid parameters passed to close." );
}

function onClose( err ) {

callback( err );

// empty the queue
q = [];
}

callback = callback || defaultCallback;
Expand All @@ -360,7 +374,17 @@ function open(connectionString, callback) {
this.commit = function() { throw new Error( "[msnodesql] Connection is closed." ); }
this.rollback = function() { throw new Error( "[msnodesql] Connection is closed." ); }

ext.close( onClose );
if( immediately || q.length == 0 ) {

ext.close( onClose );
}
else {

// since the queue is not empty, there is a pending operation, so add the close to be done
// after that operation finishes
var op = { fn: function( callback ) { ext.close( callback ) }, args: [ onClose ] };
q.push( op );
}
}

this.queryRaw = function (query, paramsOrCallback, callback) {
Expand Down
48 changes: 46 additions & 2 deletions test/connect.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ suite( 'open', function() {
});
});

test('verify connection is closed after a query', function( done ) {
test('verify closed connection throws an exception', function( done ) {

sql.open( config.conn_str, function( err, conn ) {

Expand All @@ -60,6 +60,50 @@ suite( 'open', function() {
assert( thrown );
done();
});
})
});

test( 'verify connection is not closed prematurely until a query is complete', function( done ) {

sql.open( config.conn_str, function( err, conn ) {

assert.ifError( err );

var closeCalled = false;
var stmt = conn.queryRaw( "select 1" );

stmt.on( 'meta', function( m ) { });
stmt.on( 'done', function( ) { assert( closeCalled ); done(); });
stmt.on( 'column', function( c, d ) { assert( c == 0 && d == 1 ); });
stmt.on( 'error', function( e ) { assert.ifError( e ); });
stmt.on( 'row', function( r ) { assert( r == 0 ); conn.close(); closeCalled = true; });
});
});

test( 'verify that close immediately flag only accepts booleans', function( done ) {

sql.open( config.conn_str, function( err, conn ) {

assert.ifError( err );

var thrown = false;

try {
conn.close( "SELECT 1", function( err ) {

assert.ifError( err )
});
}
catch( e ) {

assert( e == "Error: [msnodesql] Invalid parameters passed to close.")
thrown = true;
}

conn.close();
assert( thrown );
done();
});

});
});

0 comments on commit 396750b

Please sign in to comment.