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

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

It would be easy enough to do this:
{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 0697ef8..26034a6 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
@@ -834,19 +834,21 @@
         try {
             lock.lock();
             final ObjectDeque<T> objectDeque = poolMap.get(k);
-            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
-                    //       keyLock.writeLock()
-                    poolMap.remove(k);
-                    poolKeyList.remove(k);
+            if (objectDeque != null) {
+                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
+                        // keyLock.writeLock()
+                        poolMap.remove(k);
+                        poolKeyList.remove(k);
+                    }
                 }
             }
         } finally {
{noformat}

BUT, I would want to know if we are missing the application of a lock 
elsewhere. The above NPE proofing would not hurt even if we found and fixed a 
locking bug.

> Potential 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
>
> 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