You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ps...@apache.org on 2023/07/31 19:49:10 UTC

[commons-pool] branch POOL_2_X updated: GKOP fixes * Fix performance regression * Close remaining GKOP lock window register/deregister problems (JIRA: POOL-411) * Add checks in deregister for null deque, negative numInterested

This is an automated email from the ASF dual-hosted git repository.

psteitz pushed a commit to branch POOL_2_X
in repository https://gitbox.apache.org/repos/asf/commons-pool.git


The following commit(s) were added to refs/heads/POOL_2_X by this push:
     new 980aae56 GKOP fixes * Fix performance regression * Close remaining GKOP lock window register/deregister problems (JIRA: POOL-411) * Add checks in deregister for null deque, negative numInterested
980aae56 is described below

commit 980aae56bf732178a9e02a3cb686f0104bffb6d3
Author: psteitz <ph...@gmail.com>
AuthorDate: Mon Jul 31 12:49:02 2023 -0700

    GKOP fixes
    * Fix performance regression
    * Close remaining GKOP lock window register/deregister problems (JIRA: POOL-411)
    * Add checks in deregister for null deque, negative numInterested
---
 .../commons/pool2/impl/GenericKeyedObjectPool.java   | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
index 4f927fb5..dad106cb 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
@@ -838,14 +838,24 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
         Lock lock = keyLock.readLock();
         try {
             lock.lock();
-            final ObjectDeque<T> objectDeque = poolMap.get(k);
-            if (objectDeque != null) {
+            ObjectDeque<T> objectDeque = poolMap.get(k);
+            if (objectDeque == null) {
+                throw new IllegalStateException("Attempt to de-register a key for a non-existent pool");
+            }
+            final long numInterested = objectDeque.getNumInterested().decrementAndGet();
+            if (numInterested < 0) {
+                throw new IllegalStateException("numInterested count for key " + k + " is less than zero");
+            }
+            if (numInterested == 0 && objectDeque.getCreateCount().get() == 0) {
                 // Potential to remove key
                 // Upgrade to write lock
                 lock.unlock();
                 lock = keyLock.writeLock();
                 lock.lock();
-                if (objectDeque.getNumInterested().decrementAndGet() == 0 && objectDeque.getCreateCount().get() == 0) {
+                // Pool may have changed since we released the read lock
+                // numInterested decrement could lead to removal while waiting for write lock
+                objectDeque = poolMap.get(k);
+                if (null != objectDeque && objectDeque.getNumInterested().get() == 0 && objectDeque.getCreateCount().get() == 0) {
                     // NOTE: Keys must always be removed from both poolMap and
                     // poolKeyList at the same time while protected by
                     // keyLock.writeLock()
@@ -1403,10 +1413,14 @@ public class GenericKeyedObjectPool<K, T> extends BaseGenericObjectPool<T>
                     deque.getNumInterested().incrementAndGet();
                     // NOTE: Keys must always be added to both poolMap and
                     //       poolKeyList at the same time while protected by
+                    //       the write lock
                     poolKeyList.add(k);
                     return deque;
                 });
                 if (!allocated.get()) {
+                    // Another thread could have beaten us to creating the deque, while we were
+                    // waiting for the write lock, so re-get it from the map.
+                    objectDeque = poolMap.get(k);
                     objectDeque.getNumInterested().incrementAndGet();
                 }
             } else {