fix: prevent double release from corrupting the connection pool (#4186)

* fix: prevent double release from corrupting the connection pool

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* fix: defer _released reset until connection is delivered in getConnection

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Author: 3 people

lib/base/pool.js

@@ -62,7 +62,10 @@ class BasePool extends EventEmitter {
       if (this._freeConnections.length > 0) {
         connection = this._freeConnections.pop();
         this.emit('acquire', connection);
-        return process.nextTick(() => cb(null, connection));
+        return process.nextTick(() => {
+          connection._released = false;
+          cb(null, connection);
+        });
       }
       if (
         this.config.connectionLimit === 0 ||
@@ -126,7 +129,10 @@ class BasePool extends EventEmitter {
       }
     } else if (this._connectionQueue.length) {
       cb = this._connectionQueue.shift();
-      process.nextTick(cb.bind(null, null, connection));
+      process.nextTick(() => {
+        connection._released = false;
+        cb(null, connection);
+      });
     } else {
       this._freeConnections.push(connection);
       this.emit('release', connection);

lib/pool_connection.js

@@ -6,6 +6,7 @@ class PoolConnection extends Connection {
   constructor(pool, options) {
     super(options);
     this._pool = pool;
+    this._released = false;
     this.lastActiveTime = Date.now();
     this.once('end', () => {
       this._removeFromPool();
@@ -16,9 +17,13 @@ class PoolConnection extends Connection {
   }
 
   release() {
+    if (this._released) {
+      return;
+    }
     if (!this._pool || this._pool._closed) {
       return;
     }
+    this._released = true;
     this.lastActiveTime = Date.now();
     this._pool.releaseConnection(this);
   }

test/integration/test-pool-double-release.test.mts

@@ -0,0 +1,131 @@
+import type { PoolConnection } from '../../index.js';
+import { describe, it, strict } from 'poku';
+import { createPool } from '../common.test.mjs';
+
+await describe('Pool Double Release', async () => {
+  await describe('should not duplicate _freeConnections on double release', async () => {
+    const pool = createPool({
+      connectionLimit: 5,
+      maxIdle: 5,
+      idleTimeout: 15000,
+    });
+
+    let freeConnectionsLength = -1;
+
+    await new Promise<void>((resolve, reject) => {
+      pool.getConnection(
+        (err: NodeJS.ErrnoException | null, connection: PoolConnection) => {
+          if (err) return reject(err);
+
+          connection.release();
+          connection.release();
+
+          // @ts-expect-error: internal access
+          freeConnectionsLength = pool._freeConnections.length;
+          resolve();
+        }
+      );
+    });
+
+    it('should have exactly 1 free connection, not 2', () => {
+      strict.equal(freeConnectionsLength, 1);
+    });
+
+    await pool.promise().end();
+  });
+
+  await describe('should not give the same connection to two different handlers', async () => {
+    const pool = createPool({
+      connectionLimit: 2,
+      idleTimeout: 15000,
+    });
+
+    let conn1!: PoolConnection;
+    let conn2!: PoolConnection;
+
+    await new Promise<void>((resolve, reject) => {
+      pool.getConnection(
+        (err: NodeJS.ErrnoException | null, connection: PoolConnection) => {
+          if (err) return reject(err);
+
+          connection.release();
+          connection.release();
+
+          pool.getConnection(
+            (err1: NodeJS.ErrnoException | null, c1: PoolConnection) => {
+              if (err1) return reject(err1);
+              conn1 = c1;
+
+              pool.getConnection(
+                (err2: NodeJS.ErrnoException | null, c2: PoolConnection) => {
+                  if (err2) return reject(err2);
+                  conn2 = c2;
+
+                  conn1.release();
+                  conn2.release();
+                  resolve();
+                }
+              );
+            }
+          );
+        }
+      );
+    });
+
+    it('should return unique connections', () => {
+      strict.notStrictEqual(conn1, conn2);
+    });
+
+    await pool.promise().end();
+  });
+
+  await describe('should not have duplicate references in _freeConnections after double release', async () => {
+    const pool = createPool({
+      connectionLimit: 3,
+      maxIdle: 3,
+      idleTimeout: 15000,
+    });
+
+    let hasDuplicates = true;
+
+    await new Promise<void>((resolve, reject) => {
+      pool.getConnection(
+        (err1: NodeJS.ErrnoException | null, c1: PoolConnection) => {
+          if (err1) return reject(err1);
+
+          pool.getConnection(
+            (err2: NodeJS.ErrnoException | null, c2: PoolConnection) => {
+              if (err2) return reject(err2);
+
+              c1.release();
+              c1.release();
+              c2.release();
+              c2.release();
+
+              // @ts-expect-error: internal access
+              const freeConns = pool._freeConnections;
+              const seen = new Set();
+              hasDuplicates = false;
+              for (let i = 0; i < freeConns.length; i++) {
+                const conn = freeConns.get(i);
+                if (seen.has(conn)) {
+                  hasDuplicates = true;
+                  break;
+                }
+                seen.add(conn);
+              }
+
+              resolve();
+            }
+          );
+        }
+      );
+    });
+
+    it('should have no duplicate connection references', () => {
+      strict.equal(hasDuplicates, false);
+    });
+
+    await pool.promise().end();
+  });
+});