From 4e3522634034ed954b8521b528088e4610101421 Mon Sep 17 00:00:00 2001
From: Charmander <~@charmander.me>
Date: Thu, 10 Aug 2017 14:00:49 +0000
Subject: [PATCH] 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
---
 index.js             | 23 ++++++++++++++++++-----
 test/idle-timeout.js | 25 +++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/index.js b/index.js
index 46c399f9..af86134b 100644
--- a/index.js
+++ b/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)
diff --git a/test/idle-timeout.js b/test/idle-timeout.js
index 68a2b78a..a24ab7b0 100644
--- a/test/idle-timeout.js
+++ b/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 = []