From c6a8cc798c0a6b08d01cb858e6ebbc33f18e17f1 Mon Sep 17 00:00:00 2001
From: Zhen Li <zhen.li@neotechnology.com>
Date: Wed, 20 Nov 2019 16:12:42 +0100
Subject: [PATCH] Fixed a bug where connection pool size can go beyond max
 connection pool size. Added more logging around connection pool release.

---
 src/v1/internal/pool.js | 26 ++++++++++++++++++--------
 test/v1/driver.test.js  | 31 +++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/src/v1/internal/pool.js b/src/v1/internal/pool.js
index 4a862f0ec..e7c4c5a8f 100644
--- a/src/v1/internal/pool.js
+++ b/src/v1/internal/pool.js
@@ -69,11 +69,18 @@ class Pool {
       const key = address.asKey()
 
       if (resource) {
-        resourceAcquired(key, this._activeResourceCounts)
-        if (this._log.isDebugEnabled()) {
-          this._log.debug(`${resource} acquired from the pool ${key}`)
+        if (
+          this._maxSize &&
+          this.activeResourceCount(address) >= this._maxSize
+        ) {
+          this._destroy(resource)
+        } else {
+          resourceAcquired(key, this._activeResourceCounts)
+          if (this._log.isDebugEnabled()) {
+            this._log.debug(`${resource} acquired from the pool ${key}`)
+          }
+          return resource
         }
-        return resource
       }
 
       // We're out of resources and will try to acquire later on when an existing resource is released.
@@ -100,11 +107,11 @@ class Pool {
             // request already resolved/rejected by the release operation; nothing to do
           } else {
             // request is still pending and needs to be failed
+            const activeCount = this.activeResourceCount(address)
+            const idleCount = this.has(address) ? this._pools[key].length : 0
             request.reject(
               newError(
-                `Connection acquisition timed out in ${
-                  this._acquisitionTimeout
-                } ms.`
+                `Connection acquisition timed out in ${this._acquisitionTimeout} ms. Poos status: Active conn count = ${activeCount}, Idle conn count = ${idleCount}.`
               )
             )
           }
@@ -209,7 +216,10 @@ class Pool {
         }
         if (this._installIdleObserver) {
           this._installIdleObserver(resource, {
-            onError: () => {
+            onError: error => {
+              this._log.debug(
+                `Idle connection ${resource} destroyed because of error: ${error}`
+              )
               const pool = this._pools[key]
               if (pool) {
                 this._pools[key] = pool.filter(r => r !== resource)
diff --git a/test/v1/driver.test.js b/test/v1/driver.test.js
index a04269d03..2e23fecfd 100644
--- a/test/v1/driver.test.js
+++ b/test/v1/driver.test.js
@@ -57,6 +57,37 @@ describe('driver', () => {
     }
   })
 
+  it('should not decrease active connection count after driver close', done => {
+    // Given
+    const config = {
+      maxConnectionPoolSize: 2,
+      connectionAcquisitionTimeout: 0,
+      encrypted: false
+    }
+    driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken, config)
+
+    function beginTxWithoutCommit (driver) {
+      const session = driver.session()
+      const tx = session.beginTransaction()
+      return tx.run('RETURN 1')
+    }
+    // When
+
+    const result1 = beginTxWithoutCommit(driver)
+    const result2 = beginTxWithoutCommit(driver)
+
+    Promise.all([result1, result2]).then(results => {
+      driver.close()
+      beginTxWithoutCommit(driver).catch(() => {
+        var serverKey = Object.keys(driver._pool._activeResourceCounts)[0]
+        expect(driver._pool._activeResourceCounts[serverKey]).toEqual(2)
+        expect(driver._pool._pools[serverKey].length).toEqual(0)
+        expect(Object.keys(driver._openConnections).length).toEqual(2)
+        done()
+      })
+    })
+  }, 10000)
+
   it('should expose sessions', () => {
     // Given
     driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken)