You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Phil Steitz <ph...@gmail.com> on 2011/08/31 21:28:20 UTC
Re: svn commit: r1163741 - in /commons/proper/pool/trunk/src: java/org/apache/commons/pool2/impl/GenericObjectPool.java
java/org/apache/commons/pool2/impl/PooledObject.java test/org/apache/commons/pool2/impl/TestGenericObjectPool.java
Is 1.x vulnerable to this?
On 8/31/11 11:50 AM, markt@apache.org wrote:
> Author: markt
> Date: Wed Aug 31 18:50:36 2011
> New Revision: 1163741
>
> URL: http://svn.apache.org/viewvc?rev=1163741&view=rev
> Log:
> Fix a bug in eviction that could see a destroyed object returned to the idle object pool if:
> - it was being validated by the evictor
> - validation failed
> - an attempt was made to borrow the object during validation
>
> Modified:
> commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObject.java
> commons/proper/pool/trunk/src/test/org/apache/commons/pool2/impl/TestGenericObjectPool.java
>
> Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java?rev=1163741&r1=1163740&r2=1163741&view=diff
> ==============================================================================
> --- commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java (original)
> +++ commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/GenericObjectPool.java Wed Aug 31 18:50:36 2011
> @@ -1138,6 +1138,7 @@ public class GenericObjectPool<T> extend
> }
>
> private void destroy(PooledObject<T> toDestory) throws Exception {
> + toDestory.invalidate();
> idleObjects.remove(toDestory);
> allObjects.remove(toDestory.getObject());
> try {
>
> Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObject.java
> URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObject.java?rev=1163741&r1=1163740&r2=1163741&view=diff
> ==============================================================================
> --- commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObject.java (original)
> +++ commons/proper/pool/trunk/src/java/org/apache/commons/pool2/impl/PooledObject.java Wed Aug 31 18:50:36 2011
> @@ -145,4 +145,8 @@ public class PooledObject<T> implements
>
> return false;
> }
> +
> + public synchronized void invalidate() {
> + state = PooledObjectState.INVALID;
> + }
> }
>
> Modified: commons/proper/pool/trunk/src/test/org/apache/commons/pool2/impl/TestGenericObjectPool.java
> URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/org/apache/commons/pool2/impl/TestGenericObjectPool.java?rev=1163741&r1=1163740&r2=1163741&view=diff
> ==============================================================================
> --- commons/proper/pool/trunk/src/test/org/apache/commons/pool2/impl/TestGenericObjectPool.java (original)
> +++ commons/proper/pool/trunk/src/test/org/apache/commons/pool2/impl/TestGenericObjectPool.java Wed Aug 31 18:50:36 2011
> @@ -903,6 +903,70 @@ public class TestGenericObjectPool exten
> }
>
> @Test
> + public void testEvictionInvalid() throws Exception {
> + class InvalidFactory extends BasePoolableObjectFactory<Object> {
> +
> + @Override
> + public Object makeObject() throws Exception {
> + return new Object();
> + }
> +
> + @Override
> + public boolean validateObject(Object obj) {
> + try {
> + Thread.sleep(1000);
> + } catch (InterruptedException e) {
> + // Ignore
> + }
> + return false;
> + }
> + }
> +
> + final GenericObjectPool<Object> pool =
> + new GenericObjectPool<Object>(new InvalidFactory());
> +
> + pool.setMaxIdle(1);
> + pool.setMaxTotal(1);
> + pool.setTestOnBorrow(false);
> + pool.setTestOnReturn(false);
> + pool.setTestWhileIdle(true);
> + pool.setMinEvictableIdleTimeMillis(100000);
> + pool.setNumTestsPerEvictionRun(1);
> +
> + Object p = pool.borrowObject();
> + pool.returnObject(p);
> +
> + // Run eviction in a separate thread
> + Thread t = new Thread() {
> + @Override
> + public void run() {
> + try {
> + pool.evict();
> + } catch (Exception e) {
> + // Ignore
> + }
> + }
> + };
> + t.start();
> +
> + // Sleep to make sure evictor has started
> + Thread.sleep(300);
> +
> + try {
> + p = pool.borrowObject(1);
> + } catch (NoSuchElementException nsee) {
> + // Ignore
> + }
> +
> + // Make sure evictor has finished
> + Thread.sleep(1000);
> +
> + // Should have an empty pool
> + assertEquals("Idle count different than expected.", 0, pool.getNumIdle());
> + assertEquals("Total count different than expected.", 0, pool.getNumActive());
> + }
> +
> + @Test
> public void testMinIdle() throws Exception {
> pool.setMaxIdle(500);
> pool.setMinIdle(5);
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1163741 - in /commons/proper/pool/trunk/src: java/org/apache/commons/pool2/impl/GenericObjectPool.java
java/org/apache/commons/pool2/impl/PooledObject.java test/org/apache/commons/pool2/impl/TestGenericObjectPool.java
Posted by Phil Steitz <ph...@gmail.com>.
On 8/31/11 2:17 PM, Mark Thomas wrote:
> On 31/08/2011 20:28, Phil Steitz wrote:
>> Is 1.x vulnerable to this?
> I don't think so. I believe this was the result of my re-factoring.
Good. I vaguely recall similar bugs and some 1.x tests like this.
I will check.
> The
> good news was that I spotted it just by looking at the code with a fresh
> set of eyes. Things are a lot easier to follow in 2.x
Indeed!
I have just started fixing up [performance] and reviewing all of the
changes and I can say one thing for sure: the new code is much more
fun to work with :)
Phil
>
> Mark
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1163741 - in /commons/proper/pool/trunk/src: java/org/apache/commons/pool2/impl/GenericObjectPool.java
java/org/apache/commons/pool2/impl/PooledObject.java test/org/apache/commons/pool2/impl/TestGenericObjectPool.java
Posted by Mark Thomas <ma...@apache.org>.
On 31/08/2011 20:28, Phil Steitz wrote:
> Is 1.x vulnerable to this?
I don't think so. I believe this was the result of my re-factoring. The
good news was that I spotted it just by looking at the code with a fresh
set of eyes. Things are a lot easier to follow in 2.x
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org