Updated connection cleanup process to ensure we cleanup connections that have expired as well as the connections that exceed the config.maxIdle setting

Author: robertpitt

lib/pool.js

@@ -53,10 +53,10 @@ class Pool extends EventEmitter {
       this._allConnections.length < this.config.connectionLimit
     ) {
       connection = new PoolConnection(this, {
-        config: this.config.connectionConfig
+        config: this.config.connectionConfig,
       });
       this._allConnections.push(connection);
-      return connection.connect(err => {
+      return connection.connect((err) => {
         if (this._closed) {
           return cb(new Error('Pool is closed.'));
         }
@@ -102,7 +102,7 @@ class Pool extends EventEmitter {
     this._closed = true;
     clearTimeout(this._removeIdleTimeoutConnectionsTimer);
     if (typeof cb !== 'function') {
-      cb = function(err) {
+      cb = function (err) {
         if (err) {
           throw err;
         }
@@ -111,7 +111,7 @@ class Pool extends EventEmitter {
     let calledBack = false;
     let closedConnections = 0;
     let connection;
-    const endCB = function(err) {
+    const endCB = function (err) {
       if (calledBack) {
         return;
       }
@@ -136,10 +136,11 @@ class Pool extends EventEmitter {
       sql,
       values,
       cb,
-      this.config.connectionConfig
+      this.config.connectionConfig,
     );
     if (typeof cmdQuery.namedPlaceholders === 'undefined') {
-      cmdQuery.namedPlaceholders = this.config.connectionConfig.namedPlaceholders;
+      cmdQuery.namedPlaceholders =
+        this.config.connectionConfig.namedPlaceholders;
     }
     this.getConnection((err, conn) => {
       if (err) {
@@ -200,9 +201,10 @@ class Pool extends EventEmitter {
     this._removeIdleTimeoutConnectionsTimer = setTimeout(() => {
       try {
         while (
-          this._freeConnections.length > this.config.maxIdle &&
-          Date.now() - this._freeConnections.get(0).lastActiveTime >
-            this.config.idleTimeout
+          this._freeConnections.length > this.config.maxIdle ||
+          (this._freeConnections.length > 0 &&
+            Date.now() - this._freeConnections.get(0).lastActiveTime >
+              this.config.idleTimeout)
         ) {
           this._freeConnections.get(0).destroy();
         }
@@ -217,15 +219,15 @@ class Pool extends EventEmitter {
       sql,
       values,
       this.config.connectionConfig.stringifyObjects,
-      this.config.connectionConfig.timezone
+      this.config.connectionConfig.timezone,
     );
   }
 
   escape(value) {
     return mysql.escape(
       value,
       this.config.connectionConfig.stringifyObjects,
-      this.config.connectionConfig.timezone
+      this.config.connectionConfig.timezone,
     );
   }
 

test/integration/test-pool-release-idle-connection-timeout.test.cjs

@@ -0,0 +1,45 @@
+'use strict';
+
+const createPool = require('../common.test.cjs').createPool;
+const { assert } = require('poku');
+
+const pool = new createPool({
+  connectionLimit: 3, // 5 connections
+  maxIdle: 1, // 1 idle connection
+  idleTimeout: 5000, // 5 seconds
+});
+
+pool.getConnection((err1, connection1) => {
+  assert.ifError(err1);
+  assert.ok(connection1);
+  pool.getConnection((err2, connection2) => {
+    assert.ifError(err2);
+    assert.ok(connection2);
+    assert.notStrictEqual(connection1, connection2);
+    pool.getConnection((err3, connection3) => {
+      assert.ifError(err3);
+      assert.ok(connection3);
+      assert.notStrictEqual(connection1, connection3);
+      assert.notStrictEqual(connection2, connection3);
+      connection1.release();
+      connection2.release();
+      connection3.release();
+      assert(pool._allConnections.length === 3);
+      assert(pool._freeConnections.length === 3);
+      setTimeout(() => {
+        // After 5000ms, all connections should be considered idle and removed
+        assert(pool._allConnections.length === 0);
+        assert(pool._freeConnections.length === 0);
+        pool.getConnection((err4, connection4) => {
+          assert.ifError(err4);
+          assert.ok(connection4);
+          assert(pool._allConnections.length === 1);
+          assert(pool._freeConnections.length === 0);
+          connection4.release();
+          connection4.destroy();
+          pool.end();
+        });
+      }, 7000);
+    });
+  });
+});

test/integration/test-pool-release-idle-connection.test.cjs

@@ -39,7 +39,9 @@ pool.getConnection((err1, connection1) => {
           connection4.destroy();
           pool.end();
         });
-      }, 7000);
+        // Setting the time to a lower value than idleTimeout will ensure that the connection is not considered idle
+        // during our assertions
+      }, 4000);
     });
   });
 });