Fix release of broken connections to the pool

An attempt to resolve a pending acquisition request can be made when
connection is returned back to the pool. It will only be made when
there exists a pending acquisition request and returned connection
is valid.

It was previously possible for the release operation to deque a pending
acquisition request and forget about it, even when the returned
connection was invalid. This made the acquisition request either
fail with acquisition timeout or with "Cannot read property 'filter'
of undefined" error. Later happened because subsequent release operation
for the same key removed the array of pending acquisition requests.

This commit fixes the problem by making release operation only deque an
acquisition request when new request can be acquired.

Author: lutovich

src/v1/internal/pool.js

@@ -158,13 +158,16 @@ class Pool {
     // check if there are any pending requests
     const requests = this._acquireRequests[key];
     if (requests) {
-      const pending = requests.shift();
+      const pending = requests[0];
 
       if (pending) {
         const resource = this._acquire(key);
         if (resource) {
-          pending.resolve(resource);
+          // managed to acquire a valid resource from the pool to satisfy the pending acquire request
+          requests.shift(); // forget the pending request
+          pending.resolve(resource); // resolve the pending request with the acquired resource
 
+          // return early to not decrement the number of released resources
           return;
         }
       } else {

test/internal/pool.test.js

@@ -522,16 +522,72 @@ describe('Pool', () => {
     });
   });
 
+  it('should work fine when invalid resources released and acquisition attempt pending', done => {
+    const key = 'bolt://localhost:7687';
+    const acquisitionTimeout = 1000;
+    let counter = 0;
+
+    const pool = new Pool(
+      (url, release) => new Resource(url, counter++, release),
+      resource => {
+      },
+      resourceValidOnlyOnceValidationFunction,
+      new PoolConfig(2, acquisitionTimeout)
+    );
+
+    pool.acquire(key).then(resource1 => {
+      expect(resource1.id).toEqual(0);
+      expect(pool.activeResourceCount(key)).toEqual(1);
+
+      pool.acquire(key).then(resource2 => {
+        expect(resource2.id).toEqual(1);
+        expect(pool.activeResourceCount(key)).toEqual(2);
+
+        // release both resources before the acquisition timeout, they should be treated as invalid
+        setTimeout(() => {
+          expectNumberOfAcquisitionRequests(pool, key, 1);
+          resource1.close();
+          resource2.close();
+        }, acquisitionTimeout / 2);
+
+        pool.acquire(key).then(resource3 => {
+          expect(resource3.id).toEqual(2);
+          expectNoPendingAcquisitionRequests(pool);
+          expect(pool.activeResourceCount(key)).toEqual(1);
+          done();
+        }).catch(error => {
+          done.fail(error);
+        });
+      });
+    });
+  });
+
 });
 
 function expectNoPendingAcquisitionRequests(pool) {
-  expect(pool._acquireRequests).toEqual({});
+  const acquireRequests = pool._acquireRequests;
+  Object.values(acquireRequests).forEach(requests => {
+    if (Array.isArray(requests) && requests.length === 0) {
+      requests = undefined;
+    }
+    expect(requests).not.toBeDefined();
+  });
 }
 
 function expectNumberOfAcquisitionRequests(pool, key, expectedNumber) {
   expect(pool._acquireRequests[key].length).toEqual(expectedNumber);
 }
 
+function resourceValidOnlyOnceValidationFunction(resource) {
+  // all resources are valid only once
+  if (resource.validatedOnce) {
+    return false;
+  } else {
+    resource.validatedOnce = true;
+    return true;
+  }
+}
+
 class Resource {
 
   constructor(key, id, release) {