Author: markt
Date: Thu Mar 24 11:32:21 2011
New Revision: 1084905
URL: http://svn.apache.org/viewvc?rev=1084905&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/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericKeyedObjectPool.java
Modified:
commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
URL:
http://svn.apache.org/viewvc/commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java?rev=1084905&r1=1084904&r2=1084905&view=diff
==============================================================================
---
commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
(original)
+++
commons/proper/pool/branches/POOL_1_X/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
Thu Mar 24 11:32:21 2011
@@ -1568,6 +1568,12 @@ public class GenericKeyedObjectPool exte
if (pool != null) {
synchronized(this) {
pool.decrementActiveCount();
+ if (pool.queue.isEmpty() &&
+ pool.activeCount == 0 &&
+ pool.internalProcessingCount == 0) {
+ _poolMap.remove(key);
+ _poolList.remove(key);
+ }
allocate();
}
}
@@ -1649,6 +1655,12 @@ public class GenericKeyedObjectPool exte
if (decrementNumActive) {
synchronized(this) {
pool.decrementActiveCount();
+ if (pool.queue.isEmpty() &&
+ pool.activeCount == 0 &&
+ pool.internalProcessingCount == 0) {
+ _poolMap.remove(key);
+ _poolList.remove(key);
+ }
allocate();
}
}
@@ -1909,8 +1921,9 @@ public class GenericKeyedObjectPool exte
(ObjectTimestampPair) _evictionCursor.previous() :
(ObjectTimestampPair) _evictionCursor.next();
_evictionCursor.remove();
+ ObjectQueue objectQueue = (ObjectQueue) _poolMap.get(key);
+ objectQueue.incrementInternalProcessingCount();
_totalIdle--;
- _totalInternalProcessing++;
}
boolean removeObject=false;
@@ -1945,28 +1958,20 @@ public class GenericKeyedObjectPool exte
_factory.destroyObject(key, pair.value);
} 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 (_minIdle == 0) {
- synchronized (this) {
- ObjectQueue objectQueue =
- (ObjectQueue)_poolMap.get(key);
- if (objectQueue != null &&
- objectQueue.queue.isEmpty()) {
- _poolMap.remove(key);
- _poolList.remove(key);
- }
- }
- }
}
}
synchronized (this) {
- if (!removeObject) {
+ ObjectQueue 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 (_lifo) {
@@ -1974,7 +1979,6 @@ public class GenericKeyedObjectPool exte
_evictionCursor.previous();
}
}
- _totalInternalProcessing--;
}
}
allocate();
Modified:
commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericKeyedObjectPool.java
URL:
http://svn.apache.org/viewvc/commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericKeyedObjectPool.java?rev=1084905&r1=1084904&r2=1084905&view=diff
==============================================================================
---
commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericKeyedObjectPool.java
(original)
+++
commons/proper/pool/branches/POOL_1_X/src/test/org/apache/commons/pool/impl/TestGenericKeyedObjectPool.java
Thu Mar 24 11:32:21 2011
@@ -725,35 +725,42 @@ 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"));
+ Object 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"));
+ Object 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"));
+ Object 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"));
+ Object 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"));
+ Object 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);
@@ -794,8 +801,12 @@ 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);
- assertTrue(lifo ? obj.equals("19") : obj.equals("15"));
+ Object obj = pool.borrowObject(one);
+ if (lifo) {
+ assertEquals("19", obj);
+ } else {
+ assertEquals("15", obj);
+ }
}
