[
https://issues.apache.org/jira/browse/POOL-411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17741435#comment-17741435
]
Phil Steitz commented on POOL-411:
----------------------------------
[~sebb] I added sleeps temporarily between the locks in register/deregister and
a couple of timing-related tests failed because the latency controls in the
tests did not work correctly, but nothing else failed. I am not sure how to
elegantly set this up so it can be done periodically, but I agree it is a good
idea to do maybe in the runup to releases (like running soak and performance
tests with commons-performance).
On the double-checking, there are two cases to consider: first, register.
There is basically double-checking there or more precisely assurance provided
by computeIfAbsent. That is really where the bug fix is. It makes sure that
if there was no pool under the key when the read lock was acquired and there is
one after the write lock is acquired, no new pool is created. Second is
deregister. If poolMap.get(k) returns null after getting the write lock that
means that the original pool has been removed and we are in a deregister
applied to the wrong pool situation. The right thing to do there is throw ISE,
but it should not be able to happen (which is why there was no null check
before this bug was reported). We could add a test there, but I am concerned
that it would be a check that should never be able to fail and there is small,
but not zero performance cost to performing it for pretty much every pool
action (register/deregister are very hot methods). The same applies to testing
the counters on every inspection.
Perhaps best is to temporarily insert these checks along with the sleeps as
part of test runs. Any suggestions about how to do that in a simple,
maintainable way?
> 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)
