[ 
https://issues.apache.org/jira/browse/POOL-411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17741210#comment-17741210
 ] 

Gary D. Gregory commented on POOL-411:
--------------------------------------

The following allows the test to pass locally for me in Eclipse, running it 
over and over:
{noformat}
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 0f8a275..b67cc00 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
@@ -30,6 +30,7 @@
 import java.util.TreeMap;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.locks.Lock;
@@ -833,13 +834,13 @@
             lock.lock();
             final ObjectDeque<T> objectDeque = poolMap.get(k);
             if (objectDeque != null) {
+                // Potential to remove key
+                // Upgrade to write lock
+                lock.unlock();
+                lock = keyLock.writeLock();
+                lock.lock();
                 final long numInterested = 
objectDeque.getNumInterested().decrementAndGet();
                 if (numInterested == 0 && objectDeque.getCreateCount().get() 
== 0) {
-                    // Potential to remove key
-                    // Upgrade to write lock
-                    lock.unlock();
-                    lock = keyLock.writeLock();
-                    lock.lock();
                     if (objectDeque.getCreateCount().get() == 0 && 
objectDeque.getNumInterested().get() == 0) {
                         // NOTE: Keys must always be removed from both poolMap 
and
                         // poolKeyList at the same time while protected by
@@ -1385,16 +1386,17 @@
                 lock.unlock();
                 lock = keyLock.writeLock();
                 lock.lock();
-                objectDeque = poolMap.get(k);
-                if (objectDeque == null) {
-                    objectDeque = new ObjectDeque<>(fairness);
-                    objectDeque.getNumInterested().incrementAndGet();
+                final AtomicBoolean allocated = new AtomicBoolean(); 
+                objectDeque = poolMap.computeIfAbsent(k, key -> {
+                    allocated.set(true);
+                    final ObjectDeque<T> deque = new ObjectDeque<>(fairness);
+                    deque.getNumInterested().incrementAndGet();
                     // NOTE: Keys must always be added to both poolMap and
                     //       poolKeyList at the same time while protected by
-                    //       keyLock.writeLock()
-                    poolMap.put(k, objectDeque);
                     poolKeyList.add(k);
-                } else {
+                    return deque;
+                });
+                if (!allocated.get()) {
                     objectDeque.getNumInterested().incrementAndGet();
                 }
             } else {
 {noformat}

> NPE when deregistering key at end of borrow
> -------------------------------------------
>
>                 Key: POOL-411
>                 URL: https://issues.apache.org/jira/browse/POOL-411
>             Project: Commons Pool
>          Issue Type: Task
>    Affects Versions: 2.11.1
>            Reporter: Richard Eckart de Castilho
>            Priority: Major
>             Fix For: 2.12.0
>
>
> There is a potential for an NPE happening in the finally block of 
> borrowObject:
> {noformat}
> Caused by: java.lang.NullPointerException: Cannot invoke 
> "org.apache.commons.pool2.impl.GenericKeyedObjectPool$ObjectDeque.getNumInterested()"
>  because "objectDeque" is null
>       at 
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.deregister(GenericKeyedObjectPool.java:821)
>       at 
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.borrowObject(GenericKeyedObjectPool.java:507)
>       at 
> org.apache.commons.pool2.impl.GenericKeyedObjectPool.borrowObject(GenericKeyedObjectPool.java:350)
>  
> {noformat}
> From reading the code, it seems this could happen e.g. if a pool is 
> concurrently cleared while a borrow is in progress.
> Not sure what a proper solution here would be. Maybe deregister should 
> silently do nothing if poolMap.get(k) returns null? 



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to