[
https://issues.apache.org/jira/browse/POOL-411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17741331#comment-17741331
]
Phil Steitz edited comment on POOL-411 at 7/8/23 9:16 PM:
----------------------------------------------------------
[~sebb] I am not sure I understand your last comment. With the test parms (now
committed) above, the test fails regularly before Gary's last commit, so I am
not sure we need to add the sleeps to verify. I agree with what I think you
are implying - that the problem in prior code was likely between the read lock
release and write lock acquire, resulting in the possibility of two registering
threads ending up creating two different pools under the same key, one of which
ends up orphaned. More importantly, the deregister from one thread ends up
decrementing numInterested on a different deque from the one it created but
orphaned in its register activation, resulting in its premature demise. That
would lead to exactly what we saw in prior code. Gary's change effectively
double checks that the pool exists before adding to the poolMap. There is no
need to null check the deque because the computeIfAbsent creates it if it is
not there. I think we are good to resolve this unless I am missing something.
was (Author: psteitz):
[~sebb] I am not sure I understand your last comment. With the test parms (now
committed) above, the test fails regularly before Gary's last commit, so I am
not sure we need to add the sleeps to verify. I agree with what I think you
are implying - that the problem in prior code was likely between the read lock
release and write lock acquire, resulting in the possibility of two registering
threads ending up creating two different pools under the same key, one of which
ends up orphaned. Gary's change effectively double checks that the pool exists
before adding to the poolMap. I haven't looked at the ConcurrentHashMap
sources to verify, but I suspect that you can't have two threads concurrently
executing the lambda, so this should fully close the hole. There is no need
to null check the deque because the computeIfAbsent creates it if it is not
there. I think we are good to resolve this unless I am missing something.
> 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)
