mysql - npm Package Compare versions

Comparing version 2.5.0 to 2.5.1

Changes.md

@@ -7,2 +7,7 @@ # Changes
 
+## v2.5.1 (2014-09-22)
+
+* Fix `pool.end` race conditions #915
+* Fix `pool.getConnection` race conditions
+
 ## v2.5.0 (2014-09-07)
@@ -9,0 +14,0 @@

lib/Pool.js

@@ -15,6 +15,7 @@ var mysql          = require('../');
 
-  this._allConnections   = [];
-  this._freeConnections  = [];
-  this._connectionQueue  = [];
-  this._closed           = false;
+  this._acquiringConnections = [];
+  this._allConnections       = [];
+  this._freeConnections      = [];
+  this._connectionQueue      = [];
+  this._closed               = false;
 }
@@ -31,2 +32,3 @@
   var connection;
+  var pool = this;
 
@@ -42,15 +44,21 @@ if (this._freeConnections.length > 0) {
 
+    this._acquiringConnections.push(connection);
     this._allConnections.push(connection);
 
-    return connection.connect({timeout: this.config.acquireTimeout}, function (err) {
-      if (this._closed) {
-        return cb(new Error('Pool is closed.'));
+    return connection.connect({timeout: this.config.acquireTimeout}, function onConnect(err) {
+      spliceConnection(pool._acquiringConnections, connection);
+
+      if (pool._closed) {
+        err = new Error('Pool is closed.');
       }
+
       if (err) {
-        return cb(err);
+        pool._purgeConnection(connection);
+        cb(err);
+        return;
       }
 
-      this.emit('connection', connection);
-      return cb(null, connection);
-    }.bind(this));
+      pool.emit('connection', connection);
+      cb(null, connection);
+    });
   }
@@ -74,13 +82,18 @@
 
-  connection._pool = null;
-  connection.ping({timeout: this.config.acquireTimeout}, function(err) {
-    if (!err) {
-      connection._pool = pool;
-      cb(null, connection);
+  this._acquiringConnections.push(connection);
+
+  connection.ping({timeout: this.config.acquireTimeout}, function onPing(err) {
+    spliceConnection(pool._acquiringConnections, connection);
+
+    if (pool._closed) {
+      err = new Error('Pool is closed.');
+    }
+
+    if (err) {
+      pool._connectionQueue.unshift(cb);
+      pool._purgeConnection(connection);
       return;
     }
 
-    connection.destroy();
-    pool._connectionQueue.unshift(cb);
-    pool._removeConnection(connection);
+    cb(null, connection);
   });
@@ -92,2 +105,7 @@ };
 
+  if (this._acquiringConnections.indexOf(connection) !== -1) {
+    // connection is being acquired
+    return;
+  }
+
   if (connection._pool) {
@@ -135,7 +153,6 @@ if (connection._pool !== this) {
 
-  var calledBack        = false;
-  var closedConnections = 0;
-  var connection;
+  var calledBack   = false;
+  var waitingClose = this._allConnections.length;
 
-  var endCB = function(err) {
+  function onEnd(err) {
     if (calledBack) {
@@ -145,17 +162,14 @@ return;
 
-    if (err || ++closedConnections >= this._allConnections.length) {
+    if (err || --waitingClose === 0) {
       calledBack = true;
       return cb(err);
     }
-  }.bind(this);
+  }
 
-  if (this._allConnections.length === 0) {
-    return process.nextTick(endCB);
+  if (waitingClose === 0) {
+    return process.nextTick(cb);
   }
 
-  while (this._allConnections.length) {
-    connection = this._allConnections[0];
-    connection._pool = null;
-    connection._realEnd(endCB);
-    this._removeConnection(connection);
+  while (this._allConnections.length !== 0) {
+    this._purgeConnection(this._allConnections[0], onEnd);
   }
@@ -214,28 +228,27 @@ };
 
-Pool.prototype._purgeConnection = function _purgeConnection(connection) {
-  var pool = this;
+Pool.prototype._purgeConnection = function _purgeConnection(connection, callback) {
+  var cb = callback || function () {};
 
-  connection._realEnd(function(err) {
-    if (err) {
-      connection.destroy();
-    }
+  if (connection.state === 'disconnected') {
+    connection.destroy();
+  }
 
-    pool._removeConnection(connection);
-  });
+  this._removeConnection(connection);
+
+  if (connection.state !== 'disconnected' && !connection._protocol._quitSequence) {
+    connection._realEnd(cb);
+    return;
+  }
+
+  process.nextTick(cb);
 };
 
 Pool.prototype._removeConnection = function(connection) {
-  var index;
-
   connection._pool = null;
 
-  if ((index = this._allConnections.indexOf(connection)) !== -1) {
-    // Remove connection from all connections
-    this._allConnections.splice(index, 1);
-  }
+  // Remove connection from all connections
+  spliceConnection(this._allConnections, connection);
 
-  if ((index = this._freeConnections.indexOf(connection)) !== -1) {
-    // Remove connection from free connections
-    this._freeConnections.splice(index, 1);
-  }
+  // Remove connection from free connections
+  spliceConnection(this._freeConnections, connection);
 
@@ -252,1 +265,9 @@ this.releaseConnection(connection);
 };
+
+function spliceConnection(array, connection) {
+  var index;
+  if ((index = array.indexOf(connection)) !== -1) {
+    // Remove connection from all connections
+    array.splice(index, 1);
+  }
+}

package.json

 {
   "name": "mysql",
   "description": "A node.js driver for mysql. It is written in JavaScript, does not require compiling, and is 100% MIT licensed.",
-  "version": "2.5.0",
+  "version": "2.5.1",
   "license": "MIT",
@@ -6,0 +6,0 @@ "author": "Felix Geisendörfer <[e-mail]> (http://debuggable.com/)",

Readme.md

@@ -367,2 +367,23 @@ # mysql
 
+## Closing all the connections in a pool
+
+When you are done using the pool, you have to end all the connections or the
+Node.js event loop will stay active until the connections are closed by the
+MySQL server. This is typically done if the pool is used in a script or when
+trying to gracefully shutdown a server. To end all the connections in the
+pool, use the `end` method on the pool:
+
+```js
+pool.end(function (err) {
+  // all connections in the pool have ended
+});
+```
+
+The `end` method takes an _optional_ callback that you can use to know once
+all the connections have ended. The connections end _gracefully_, so all
+pending queries will still complete and the time to end the pool will vary.
+
+**Once `pool.end()` has been called, `pool.getConnection` and other operations
+can no longer be performed**
+
 ## PoolCluster
@@ -369,0 +390,0 @@

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

306333

increased by0.47%

Lines of code

5488

increased by0.24%

Number of lines in readme file

1204

increased by1.78%

No dependency changes