On 5/18/11 10:48 AM, Phil Steitz wrote: > On 5/17/11 3:21 PM, ma...@apache.org wrote: >> Author: markt >> Date: Tue May 17 22:21:43 2011 >> New Revision: 1104601 >> >> URL: http://svn.apache.org/viewvc?rev=1104601&view=rev >> Log: >> Make Phil's suggested changes to (hopefully) fix issues when testing with >> commons-performance > I have not rerun all of the commons-performance tests yet with the > new code, but the test case that I just committed failed before this > change and now succeeds.
One more comment on this. The itemsToRemove counter in clearOldest should probably not be decremented when a checked out instance is encountered. I am personally OK leaving as is, though, because I don't think clearOldest (actually borrowObject) makes a promise of precision at that level and unless destroy has a lot of latency, occurrence should be rare. Phil > Phil >> Modified: >> >> commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.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=1104601&r1=1104600&r2=1104601&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 >> Tue May 17 22:21:43 2011 >> @@ -1078,7 +1078,7 @@ public class GenericKeyedObjectPool<K,T> >> _factory.activateObject(key, p.getObject()); >> } catch (Exception e) { >> try { >> - destroy(key, p); >> + destroy(key, p, true); >> } catch (Exception e1) { >> // Ignore - activation failure is more important >> } >> @@ -1100,7 +1100,7 @@ public class GenericKeyedObjectPool<K,T> >> } >> if (!validate) { >> try { >> - destroy(key, p); >> + destroy(key, p, true); >> } catch (Exception e) { >> // Ignore - validation failure is more >> important >> } >> @@ -1155,7 +1155,7 @@ public class GenericKeyedObjectPool<K,T> >> if (getTestOnReturn()) { >> if (!_factory.validateObject(key, t)) { >> try { >> - destroy(key, p); >> + destroy(key, p, true); >> } catch (Exception e) { >> // TODO - Ignore? >> } >> @@ -1167,7 +1167,7 @@ public class GenericKeyedObjectPool<K,T> >> _factory.passivateObject(key, t); >> } catch (Exception e1) { >> try { >> - destroy(key, p); >> + destroy(key, p, true); >> } catch (Exception e) { >> // TODO - Ignore? >> } >> @@ -1184,7 +1184,7 @@ public class GenericKeyedObjectPool<K,T> >> >> if (isClosed() || maxIdle > -1 && maxIdle <= idleObjects.size()) { >> try { >> - destroy(key, p); >> + destroy(key, p, true); >> } catch (Exception e) { >> // TODO - Ignore? >> } >> @@ -1200,8 +1200,8 @@ public class GenericKeyedObjectPool<K,T> >> >> /** >> * {@inheritDoc} >> - * <p>Activation of this method decrements the active count associated >> with the given keyed pool >> - * and attempts to destroy <code>obj.</code></p> >> + * <p>Activation of this method decrements the active count associated >> with >> + * the given keyed pool and attempts to destroy <code>obj.</code></p> >> * >> * @param key pool key >> * @param obj instance to invalidate >> @@ -1217,7 +1217,7 @@ public class GenericKeyedObjectPool<K,T> >> throw new IllegalStateException( >> "Object not currently part of this pool"); >> } >> - destroy(key, p); >> + destroy(key, p, true); >> } >> >> >> @@ -1247,7 +1247,8 @@ public class GenericKeyedObjectPool<K,T> >> >> >> /** >> - * Clears the specified pool, removing all pooled instances >> corresponding to the given <code>key</code>. >> + * Clears the specified pool, removing all pooled instances >> corresponding >> + * to the given <code>key</code>. >> * >> * @param key the key to clear >> */ >> @@ -1268,7 +1269,7 @@ public class GenericKeyedObjectPool<K,T> >> >> while (p != null) { >> try { >> - destroy(key, p); >> + destroy(key, p, true); >> } catch (Exception e) { >> // TODO - Ignore? >> } >> @@ -1417,7 +1418,7 @@ public class GenericKeyedObjectPool<K,T> >> K key = entry.getValue(); >> PooledObject<T> p = entry.getKey(); >> try { >> - destroy(key, p); >> + destroy(key, p, false); >> } catch (Exception e) { >> // TODO - Ignore? >> } >> @@ -1506,7 +1507,7 @@ public class GenericKeyedObjectPool<K,T> >> } >> >> if (idleEvictTime < underTest.getIdleTimeMillis()) { >> - destroy(evictionKey, underTest); >> + destroy(evictionKey, underTest, true); >> } else { >> if (testWhileIdle) { >> boolean active = false; >> @@ -1515,18 +1516,18 @@ public class GenericKeyedObjectPool<K,T> >> underTest.getObject()); >> active = true; >> } catch (Exception e) { >> - destroy(evictionKey, underTest); >> + destroy(evictionKey, underTest, true); >> } >> if (active) { >> if (!_factory.validateObject(evictionKey, >> underTest.getObject())) { >> - destroy(evictionKey, underTest); >> + destroy(evictionKey, underTest, true); >> } else { >> try { >> _factory.passivateObject(evictionKey, >> underTest.getObject()); >> } catch (Exception e) { >> - destroy(evictionKey, underTest); >> + destroy(evictionKey, underTest, true); >> } >> } >> } >> @@ -1585,20 +1586,24 @@ public class GenericKeyedObjectPool<K,T> >> return p; >> } >> >> - private void destroy(K key, PooledObject<T> toDestory) throws Exception >> { >> + private void destroy(K key, PooledObject<T> toDestory, boolean always) >> + throws Exception { >> >> register(key); >> >> try { >> ObjectDeque<T> objectDeque = poolMap.get(key); >> - objectDeque.getIdleObjects().remove(toDestory); >> - objectDeque.getAllObjects().remove(toDestory.getObject()); >> - >> - try { >> - _factory.destroyObject(key, toDestory.getObject()); >> - } finally { >> - objectDeque.getCreateCount().decrementAndGet(); >> - numTotal.decrementAndGet(); >> + boolean isIdle = objectDeque.getIdleObjects().remove(toDestory); >> + >> + if (isIdle || always) { >> + objectDeque.getAllObjects().remove(toDestory.getObject()); >> + >> + try { >> + _factory.destroyObject(key, toDestory.getObject()); >> + } finally { >> + objectDeque.getCreateCount().decrementAndGet(); >> + numTotal.decrementAndGet(); >> + } >> } >> } finally { >> deregister(key); >> >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
