Author: markt
Date: Thu Mar 24 11:32:07 2011
New Revision: 1084904
URL: http://svn.apache.org/viewvc?rev=1084904&view=rev
Log:
Additional fixes for POOL-180. Ensure the evictor tracks internal processing
counts per key and remove unused keys only when they are truly unused.
Modified:
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
commons/proper/pool/trunk/src/test/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
Modified:
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
URL:
http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java?rev=1084904&r1=1084903&r2=1084904&view=diff
==============================================================================
---
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
(original)
+++
commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
Thu Mar 24 11:32:07 2011
@@ -1164,6 +1164,12 @@ public class GenericKeyedObjectPool<K,V>
if (pool != null) {
synchronized(this) {
pool.decrementActiveCount();
+ if (pool.queue.isEmpty() &&
+ pool.activeCount == 0 &&
+ pool.internalProcessingCount == 0) {
+ _poolMap.remove(key);
+ _poolList.remove(key);
+ }
allocate();
}
}
@@ -1244,6 +1250,12 @@ public class GenericKeyedObjectPool<K,V>
if (decrementNumActive) {
synchronized(this) {
pool.decrementActiveCount();
+ if (pool.queue.isEmpty() &&
+ pool.activeCount == 0 &&
+ pool.internalProcessingCount == 0) {
+ _poolMap.remove(key);
+ _poolList.remove(key);
+ }
allocate();
}
}
@@ -1461,8 +1473,9 @@ public class GenericKeyedObjectPool<K,V>
_evictionCursor.previous() :
_evictionCursor.next();
_evictionCursor.remove();
+ ObjectQueue objectQueue = _poolMap.get(key);
+ objectQueue.incrementInternalProcessingCount();
_totalIdle--;
- _totalInternalProcessing++;
}
boolean removeObject=false;
@@ -1497,28 +1510,19 @@ public class GenericKeyedObjectPool<K,V>
_factory.destroyObject(key, pair.getValue());
} catch(Exception e) {
// ignored
- } finally {
- // Do not remove the key from the _poolList or _poolmap,
- // even if the list stored in the _poolMap for this key is
- // empty when minIdle > 0.
- //
- // Otherwise if it was the last object for that key,
- // drop that pool
- if (this.minIdlePerKey == 0) {
- synchronized (this) {
- ObjectQueue objectQueue =
- _poolMap.get(key);
- if (objectQueue != null &&
- objectQueue.queue.isEmpty()) {
- _poolMap.remove(key);
- _poolList.remove(key);
- }
- }
- }
}
}
synchronized (this) {
- if (!removeObject) {
+ ObjectQueue objectQueue = _poolMap.get(key);
+ objectQueue.decrementInternalProcessingCount();
+ if (removeObject) {
+ if (objectQueue.queue.isEmpty() &&
+ objectQueue.activeCount == 0 &&
+ objectQueue.internalProcessingCount == 0) {
+ _poolMap.remove(key);
+ _poolList.remove(key);
+ }
+ } else {
_evictionCursor.add(pair);
_totalIdle++;
if (this.lifo) {
@@ -1526,8 +1530,8 @@ public class GenericKeyedObjectPool<K,V>
_evictionCursor.previous();
}
}
- _totalInternalProcessing--;
}
+
}
allocate();
}
Modified:
commons/proper/pool/trunk/src/test/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
URL:
http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java?rev=1084904&r1=1084903&r2=1084904&view=diff
==============================================================================
---
commons/proper/pool/trunk/src/test/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
(original)
+++
commons/proper/pool/trunk/src/test/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
Thu Mar 24 11:32:07 2011
@@ -755,36 +755,43 @@ public class TestGenericKeyedObjectPool
pool.evict(); // Kill (0,0),(0,1)
assertEquals(3, pool.getNumIdle(zero));
- Object obj = pool.borrowObject(zero);
- assertTrue(lifo ? obj.equals("04") : obj.equals("02"));
+ String objZeroA = pool.borrowObject(zero);
+ assertTrue(lifo ? objZeroA.equals("04") : objZeroA.equals("02"));
assertEquals(2, pool.getNumIdle(zero));
- obj = pool.borrowObject(zero);
- assertTrue(obj.equals("03"));
+ String objZeroB = pool.borrowObject(zero);
+ assertTrue(objZeroB.equals("03"));
assertEquals(1, pool.getNumIdle(zero));
pool.evict(); // Kill remaining 0 survivor and (1,5)
assertEquals(0, pool.getNumIdle(zero));
assertEquals(4, pool.getNumIdle(one));
- obj = pool.borrowObject(one);
- assertTrue(lifo ? obj.equals("19") : obj.equals("16"));
+ String objOneA = pool.borrowObject(one);
+ assertTrue(lifo ? objOneA.equals("19") : objOneA.equals("16"));
assertEquals(3, pool.getNumIdle(one));
- obj = pool.borrowObject(one);
- assertTrue(lifo ? obj.equals("18") : obj.equals("17"));
+ String objOneB = pool.borrowObject(one);
+ assertTrue(lifo ? objOneB.equals("18") : objOneB.equals("17"));
assertEquals(2, pool.getNumIdle(one));
pool.evict(); // Kill remaining 1 survivors
assertEquals(0, pool.getNumIdle(one));
pool.evict(); // Kill (2,10), (2,11)
assertEquals(3, pool.getNumIdle(two));
- obj = pool.borrowObject(two);
- assertTrue(lifo ? obj.equals("214") : obj.equals("212"));
+ String objTwoA = pool.borrowObject(two);
+ assertTrue(lifo ? objTwoA.equals("214") : objTwoA.equals("212"));
assertEquals(2, pool.getNumIdle(two));
pool.evict(); // All dead now
assertEquals(0, pool.getNumIdle(two));
pool.evict(); // Should do nothing - make sure no exception
- pool.evict();
-
+ // Currently 2 zero, 2 one and 1 two active. Return them
+ pool.returnObject(zero, objZeroA);
+ pool.returnObject(zero, objZeroB);
+ pool.returnObject(one, objOneA);
+ pool.returnObject(one, objOneB);
+ pool.returnObject(two, objTwoA);
+ // Remove all idle objects
+ pool.clear();
+
// Reload
pool.setMinEvictableIdleTimeMillis(500);
factory.counter = 0; // Reset counter
@@ -824,7 +831,7 @@ public class TestGenericKeyedObjectPool
pool.evict(); // kill (1,6), (1,7) - (1,5) missed
assertEquals(3, pool.getNumIdle(one));
assertEquals(5, pool.getNumIdle(two));
- obj = pool.borrowObject(one);
+ String obj = pool.borrowObject(one);
assertTrue(lifo ? obj.equals("19") : obj.equals("15"));
}
