From 6d0ef02e907ed6cf2f331042fa816d244b7a3f92 Mon Sep 17 00:00:00 2001
From: Jack Christensen <jack@jackchristensen.com>
Date: Thu, 20 Aug 2020 21:56:47 -0500
Subject: [PATCH] Fix: Resource.Destroy removed self from pool after destructor
 complete

---
 CHANGELOG.md |  4 ++++
 pool.go      |  6 ++----
 pool_test.go | 11 +++++++++--
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index e671d16..02f3d08 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,7 @@
+# Unreleased
+
+* Fix: Resource.Destroy no longer removes itself from the pool before its destructor has completed.
+
 # 1.1.1 (April 2, 2020)
 
 * Pool.Close can be safely called multiple times
diff --git a/pool.go b/pool.go
index 5d9ea66..e22b43a 100644
--- a/pool.go
+++ b/pool.go
@@ -64,7 +64,7 @@ func (res *Resource) Destroy() {
 	if res.status != resourceStatusAcquired {
 		panic("tried to destroy resource that is not acquired")
 	}
-	res.pool.destroyAcquiredResource(res)
+	go res.pool.destroyAcquiredResource(res)
 }
 
 // Hijack assumes ownership of the resource from the pool. Caller is responsible
@@ -435,11 +435,9 @@ func (p *Pool) releaseAcquiredResource(res *Resource, lastUsedNano int64) {
 // Remove removes res from the pool and closes it. If res is not part of the
 // pool Remove will panic.
 func (p *Pool) destroyAcquiredResource(res *Resource) {
+	p.destructResourceValue(res.value)
 	p.cond.L.Lock()
-
 	p.allResources = removeResource(p.allResources, res)
-	go p.destructResourceValue(res.value)
-
 	p.cond.L.Unlock()
 	p.cond.Signal()
 }
diff --git a/pool_test.go b/pool_test.go
index 3d50612..b05e32d 100644
--- a/pool_test.go
+++ b/pool_test.go
@@ -482,7 +482,7 @@ func TestPoolStatCanceledAcquireDuringWait(t *testing.T) {
 	assert.Equal(t, int64(1), stat.CanceledAcquireCount())
 }
 
-func TestResourceDestroyRemovesResourceFromPool(t *testing.T) {
+func TestResourceHijackRemovesResourceFromPoolButDoesNotDestroy(t *testing.T) {
 	constructor, _ := createConstructor()
 	var destructorCalls Counter
 	destructor := func(interface{}) {
@@ -506,7 +506,7 @@ func TestResourceDestroyRemovesResourceFromPool(t *testing.T) {
 	res.IdleDuration()
 }
 
-func TestResourceHijackRemovesResourceFromPoolButDoesNotDestroy(t *testing.T) {
+func TestResourceDestroyRemovesResourceFromPool(t *testing.T) {
 	constructor, _ := createConstructor()
 	pool := puddle.NewPool(constructor, stubDestructor, 10)
 
@@ -516,6 +516,13 @@ func TestResourceHijackRemovesResourceFromPoolButDoesNotDestroy(t *testing.T) {
 
 	assert.EqualValues(t, 1, pool.Stat().TotalResources())
 	res.Destroy()
+	for i := 0; i < 1000; i++ {
+		if pool.Stat().TotalResources() == 0 {
+			break
+		}
+		time.Sleep(time.Millisecond)
+	}
+
 	assert.EqualValues(t, 0, pool.Stat().TotalResources())
 }