From 9d71adc4bbdbeecf78fa7e7c975ad10352f48594 Mon Sep 17 00:00:00 2001
From: Steve Cresswell <229672+cressie176@users.noreply.github.com>
Date: Sat, 12 Aug 2023 19:54:14 +0100
Subject: [PATCH] Handle resource destruction failure scenarios

---
 lib/Pool.js          |  6 +++-
 lib/ResourceStore.js | 16 +++++----
 test/Pool.test.js    | 77 ++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 89 insertions(+), 10 deletions(-)

diff --git a/lib/Pool.js b/lib/Pool.js
index 00c13d7..d34e154 100644
--- a/lib/Pool.js
+++ b/lib/Pool.js
@@ -246,7 +246,11 @@ module.exports = class Pool extends EventEmitter {
         throw new ResourceDestructionFailed('Error destroying resource', { cause });
       }
     };
-    return new TimedTask({ name: 'destroy', fn, timeout: this._destroyTimeout });
+    const onLateResolve = () => {
+      debug('[%d] Discarding resource', getContextId());
+      this._resourceStore.evictBadResource(resource);
+    };
+    return new TimedTask({ name: 'destroy', fn, timeout: this._destroyTimeout, onLateResolve });
   }
 
   _emit(err) {
diff --git a/lib/ResourceStore.js b/lib/ResourceStore.js
index 84071f6..f881756 100644
--- a/lib/ResourceStore.js
+++ b/lib/ResourceStore.js
@@ -75,27 +75,31 @@ module.exports = class ResourceSstore {
   }
 
   releaseAcquiredResource(resource) {
-    if (this._removeAcquiredResource(resource)) this.addIdleResource(resource);
+    if (this._removeResource(this._acquired, resource)) this.addIdleResource(resource);
   }
 
   removeAcquiredResource(resource) {
-    this._removeAcquiredResource(resource);
+    this._removeResource(this._acquired, resource);
   }
 
   excludeBadResource(resource) {
-    this._removeAcquiredResource(resource);
+    this._removeResource(this._acquired, resource);
     this._bad.push(resource);
   }
 
+  evictBadResource(resource) {
+    this._removeResource(this._bad, resource);
+  }
+
   evictBadResources() {
     this._bad.length = 0;
   }
 
-  _removeAcquiredResource(resource) {
-    const index = this._acquired.indexOf(resource);
+  _removeResource(list, resource) {
+    const index = list.indexOf(resource);
     if (index < 0) return false;
 
-    this._acquired.splice(index, 1);
+    list.splice(index, 1);
     return true;
   }
 };
diff --git a/test/Pool.test.js b/test/Pool.test.js
index 2d05902..c97d4c5 100644
--- a/test/Pool.test.js
+++ b/test/Pool.test.js
@@ -468,7 +468,7 @@ describe('Pool', () => {
 
     describe('release', () => {
 
-      it('should release the given managed resource', async () => {
+      it('should release the supplied resource', async () => {
         const resources = ['R1'];
         const factory = new TestFactory(resources);
         const pool = createPool({ factory });
@@ -496,7 +496,7 @@ describe('Pool', () => {
 
     describe('destroy', () => {
 
-      it('should remove the given managed resource from the pool eventually', async () => {
+      it('should remove the supplied resource from the pool eventually', async () => {
         const resources = ['R1'];
         const factory = new TestFactory(resources);
         const pool = createPool({ factory });
@@ -512,7 +512,7 @@ describe('Pool', () => {
         }, 100).unref();
       });
 
-      it('should destroy the given managed resource eventually', async (t, done) => {
+      it('should destroy the supplied resource eventually', async (t, done) => {
         const resources = ['R1'];
         const factory = new TestFactory(resources);
         const pool = createPool({ factory });
@@ -587,6 +587,56 @@ describe('Pool', () => {
         pool.destroy(resource);
       });
 
+      it('should quaranteen resources that failed to be destroyed due to error', async (t, done) => {
+        const resources = [{ destroyError: 'Oh Noes!', value: 'R1' }];
+        const factory = new TestFactory(resources);
+        const pool = createPool({ factory });
+
+        pool.once('ERR_X-POOL_RESOURCE_DESTRUCTION_FAILED', () => {
+          const { size, acquired, bad } = pool.stats();
+          eq(size, 1);
+          eq(acquired, 0);
+          eq(bad, 1);
+          done();
+        });
+
+        const resource = await pool.acquire();
+        pool.destroy(resource);
+      });
+
+      it('should quaranteen resources that failed to be destroyed due to timeout', async (t, done) => {
+        const resources = [{ destroyDelay: 200, value: 'R1' }];
+        const factory = new TestFactory(resources);
+        const pool = createPool({ factory, destroyTimeout: 100 });
+
+        pool.once('ERR_X-POOL_OPERATION_TIMEDOUT', () => {
+          const { size, acquired, bad } = pool.stats();
+          eq(size, 1);
+          eq(acquired, 0);
+          eq(bad, 1);
+          done();
+        });
+
+        const resource = await pool.acquire();
+        pool.destroy(resource);
+      });
+
+      it('should discard quaranteened resources that were destroyed after the timeout expired', async (t, done) => {
+        const resources = [{ destroyDelay: 200, value: 'R1' }];
+        const factory = new TestFactory(resources);
+        const pool = createPool({ factory, destroyTimeout: 100 });
+
+        const resource = await pool.acquire();
+        pool.destroy(resource);
+
+        setTimeout(() => {
+          const { size, acquired, bad } = pool.stats();
+          eq(size, 0);
+          eq(acquired, 0);
+          eq(bad, 0);
+          done();
+        }, 300).unref();
+      });
     });
 
     describe('evictBadResources', () => {
@@ -917,6 +967,27 @@ describe('Pool', () => {
       });
     });
   });
+
+  describe('Resource Management', () => {
+
+    describe('Eviction', () => {
+
+      it('should evict idle resources once their evictionThreshold has been exceeded', () => {
+      });
+
+      it('should not evict idle resources when the pool size is at minimum', () => {
+      });
+
+      it('should quaranteen evicted resources that failed to be destroyed due to error', () => {
+      });
+
+      it('should quaranteen evicted resources that failed to be destroyed before the destroyTimeout was exceeded', () => {
+      });
+
+      it('should discard bad resources that are destroyed after the timeout expired', () => {
+      });
+    });
+  });
 });
 
 function createPool({ factory, minSize, maxSize, initialiseTimeout, acquireTimeout = 1000, acquireRetryInterval, destroyTimeout = 1000 }) {