This is an automated email from the ASF dual-hosted git repository.
psteitz pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-pool.git
The following commit(s) were added to refs/heads/master by this push:
new 7d0b459 Complete fix for POOL-326
new 81ddcd7 Merge branch 'master' of
https://git-wip-us.apache.org/repos/asf/commons-pool
7d0b459 is described below
commit 7d0b45940238918033094684343551f90524e600
Author: psteitz <pste...@apache.org>
AuthorDate: Fri Oct 4 17:57:01 2019 -0700
Complete fix for POOL-326
---
.../commons/pool2/impl/GenericKeyedObjectPool.java | 12 +++--
.../pool2/impl/TestGenericKeyedObjectPool.java | 63 +++++++++++++++++++++-
2 files changed, 70 insertions(+), 5 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 6d8ec69..4cc69ed 100644
--- a/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
+++ b/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
@@ -1086,8 +1086,13 @@ public class GenericKeyedObjectPool<K, T> extends
BaseGenericObjectPool<T>
final ObjectDeque<T> objectDeque = register(key);
try {
- final boolean isIdle =
objectDeque.getIdleObjects().remove(toDestroy);
-
+ // Check idle state directly
+ boolean isIdle =
toDestroy.getState().equals(PooledObjectState.IDLE);
+ // If idle, not under eviction test, or always is true, remove
instance,
+ // updating isIdle if instance is found in idle objects
+ if (isIdle || always) {
+ isIdle = objectDeque.getIdleObjects().remove(toDestroy);
+ }
if (isIdle || always) {
objectDeque.getAllObjects().remove(new
IdentityWrapper<>(toDestroy.getObject()));
toDestroy.invalidate();
@@ -1170,8 +1175,7 @@ public class GenericKeyedObjectPool<K, T> extends
BaseGenericObjectPool<T>
lock.unlock();
lock = keyLock.writeLock();
lock.lock();
- if (objectDeque.getCreateCount().get() == 0 &&
objectDeque.getNumInterested().get() == 0
- && objectDeque.allObjects.isEmpty()) {
+ 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()
diff --git
a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
index f148bc0..d9247a7 100644
---
a/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
+++
b/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
@@ -2312,6 +2312,67 @@ public class TestGenericKeyedObjectPool extends
TestKeyedObjectPool {
assertEquals(1, gkoPool.getNumActive());
}
+ // POOL-326
+ @Test
+ public void testEvictorClearOldestRace() throws Exception {
+ gkoPool.setMinEvictableIdleTimeMillis(100);
+ gkoPool.setNumTestsPerEvictionRun(1);
+
+ // Introduce latency between when evictor starts looking at an
instance and when
+ // it decides to destroy it
+ gkoPool.setEvictionPolicy(new SlowEvictionPolicy<String>(1000));
+
+ // Borrow an instance
+ String val = gkoPool.borrowObject("foo");
+
+ // Add another idle one
+ gkoPool.addObject("foo");
+
+ // Sleep long enough so idle one is eligible for eviction
+ Thread.sleep(1000);
+
+ // Start evictor and race with clearOldest
+ gkoPool.setTimeBetweenEvictionRunsMillis(10);
+
+ // Wait for evictor to start
+ Thread.sleep(100);
+ gkoPool.clearOldest();
+
+ // Wait for slow evictor to complete
+ Thread.sleep(1500);
+
+ // See if we get NPE on return (POOL-326)
+ gkoPool.returnObject("foo", val);
+ }
+
+
+ /*
+ * DefaultEvictionPolicy modified to add latency
+ */
+ private static class SlowEvictionPolicy<T> extends
DefaultEvictionPolicy<T> {
+ private final long delay;
+
+ /**
+ * Create SlowEvictionPolicy with the given delay in ms
+ *
+ * @param delay number of ms of latency to inject in evict
+ */
+ public SlowEvictionPolicy(long delay) {
+ super();
+ this.delay = delay;
+ }
+
+ @Override
+ public boolean evict(final EvictionConfig config, final
PooledObject<T> underTest,
+ final int idleCount) {
+ try {
+ Thread.sleep(delay);
+ } catch (InterruptedException e) {
+ // ignore
+ }
+ return super.evict(config, underTest, idleCount);
+ }
+ }
private static class DummyFactory
extends BaseKeyedPooledObjectFactory<Object,Object> {
@@ -2378,7 +2439,7 @@ public class TestGenericKeyedObjectPool extends
TestKeyedObjectPool {
return new DefaultPooledObject<>(value);
}
}
-
+
}
