Fix client remove clearing unrelated idle timers (#71)

* Add failing test for idle timer continuation after removal

* Clear idle timeout only for removed client

* Copy list of idle clients for modification during iteration

Author: charmander

index.js

@@ -3,6 +3,14 @@ const EventEmitter = require('events').EventEmitter
 
 const NOOP = function () { }
 
+const removeWhere = (list, predicate) => {
+  const i = list.findIndex(predicate)
+
+  return i === -1
+    ? undefined
+    : list.splice(i, 1)[0]
+}
+
 class IdleItem {
   constructor (client, timeoutId) {
     this.client = client
@@ -80,7 +88,7 @@ class Pool extends EventEmitter {
     if (this.ending) {
       this.log('pulse queue on ending')
       if (this._idle.length) {
-        this._idle.map(item => {
+        this._idle.slice().map(item => {
           this._remove(item.client)
         })
       }
@@ -114,10 +122,15 @@ class Pool extends EventEmitter {
   }
 
   _remove (client) {
-    this._idle = this._idle.filter(item => {
-      clearTimeout(item.timeoutId)
-      return item.client !== client
-    })
+    const removed = removeWhere(
+      this._idle,
+      item => item.client === client
+    )
+
+    if (removed !== undefined) {
+      clearTimeout(removed.timeoutId)
+    }
+
     this._clients = this._clients.filter(c => c !== client)
     client.end()
     this.emit('remove', client)

test/idle-timeout.js

@@ -20,6 +20,31 @@ describe('idle timeout', () => {
     })
   })
 
+  it('times out and removes clients when others are also removed', co.wrap(function * () {
+    const pool = new Pool({ idleTimeoutMillis: 10 })
+    const clientA = yield pool.connect()
+    const clientB = yield pool.connect()
+    clientA.release()
+    clientB.release(new Error())
+
+    const removal = new Promise((resolve) => {
+      pool.on('remove', () => {
+        expect(pool.idleCount).to.equal(0)
+        expect(pool.totalCount).to.equal(0)
+        resolve()
+      })
+    })
+
+    const timeout = wait(100).then(() =>
+      Promise.reject(new Error('Idle timeout failed to occur')))
+
+    try {
+      yield Promise.race([removal, timeout])
+    } finally {
+      pool.end()
+    }
+  }))
+
   it('can remove idle clients and recreate them', co.wrap(function * () {
     const pool = new Pool({ idleTimeoutMillis: 1 })
     const results = []