commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: svn commit: r1426799 - in /commons/proper/pool/trunk/src: main/java/org/apache/commons/pool2/impl/ test/java/org/apache/commons/pool2/impl/
Date Thu, 10 Jan 2013 16:21:52 GMT
Should this be ported to the 1.6 branch?

Gary


On Sat, Dec 29, 2012 at 1:23 PM, <psteitz@apache.org> wrote:

> Author: psteitz
> Date: Sat Dec 29 18:23:33 2012
> New Revision: 1426799
>
> URL: http://svn.apache.org/viewvc?rev=1426799&view=rev
> Log:
> Fixed threadsafety problem in GOP, GKOP invalidateObject. JIRA: POOL-231.
>
> Modified:
>
> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
>
> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
>
> commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
>
> commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
>
> Modified:
> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java?rev=1426799&r1=1426798&r2=1426799&view=diff
>
> ==============================================================================
> ---
> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> (original)
> +++
> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericKeyedObjectPool.java
> Sat Dec 29 18:23:33 2012
> @@ -551,7 +551,11 @@ public class GenericKeyedObjectPool<K,T>
>              throw new IllegalStateException(
>                      "Object not currently part of this pool");
>          }
> -        destroy(key, p, true);
> +        synchronized (p) {
> +            if (p.getState() != PooledObjectState.INVALID) {
> +                destroy(key, p, true);
> +            }
> +        }
>      }
>
>
>
> Modified:
> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java?rev=1426799&r1=1426798&r2=1426799&view=diff
>
> ==============================================================================
> ---
> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> (original)
> +++
> commons/proper/pool/trunk/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java
> Sat Dec 29 18:23:33 2012
> @@ -581,10 +581,14 @@ public class GenericObjectPool<T> extend
>                  return;
>              } else {
>                  throw new IllegalStateException(
> -                        "Returned object not currently part of this
> pool");
> +                        "Invalidated object not currently part of this
> pool");
>              }
>          }
> -        destroy(p);
> +        synchronized (p) {
> +            if (p.getState() != PooledObjectState.INVALID) {
> +                destroy(p);
> +            }
> +        }
>      }
>
>      /**
>
> Modified:
> commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java?rev=1426799&r1=1426798&r2=1426799&view=diff
>
> ==============================================================================
> ---
> commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> (original)
> +++
> commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericKeyedObjectPool.java
> Sat Dec 29 18:23:33 2012
> @@ -1433,6 +1433,88 @@ public class TestGenericKeyedObjectPool
>          // Check thread was interrupted
>          assertTrue(wtt._thrown instanceof InterruptedException);
>      }
> +
> +    /**
> +     * POOL-231 - verify that concurrent invalidates of the same object
> do not
> +     * corrupt pool destroyCount.
> +     */
> +    @Test
> +    public void testConcurrentInvalidate() throws Exception {
> +        // Get allObjects and idleObjects loaded with some instances
> +        final int nObjects = 1000;
> +        final String key = "one";
> +        pool.setMaxTotal(nObjects);
> +        pool.setMaxTotalPerKey(nObjects);
> +        pool.setMaxIdlePerKey(nObjects);
> +        final String [] obj = new String[nObjects];
> +        for (int i = 0; i < nObjects; i++) {
> +            obj[i] = pool.borrowObject(key);
> +        }
> +        for (int i = 0; i < nObjects; i++) {
> +            if (i % 2 == 0) {
> +                pool.returnObject(key, obj[i]);
> +            }
> +        }
> +        final int nThreads = 20;
> +        final int nIterations = 60;
> +        final InvalidateThread[] threads = new InvalidateThread[nThreads];
> +        // Randomly generated list of distinct invalidation targets
> +        final ArrayList<Integer> targets = new ArrayList<Integer>();
> +        final Random random = new Random();
> +        for (int j = 0; j < nIterations; j++) {
> +            // Get a random invalidation target
> +            Integer targ = new Integer(random.nextInt(nObjects));
> +            while (targets.contains(targ)) {
> +                targ = new Integer(random.nextInt(nObjects));
> +            }
> +            targets.add(targ);
> +            // Launch nThreads threads all trying to invalidate the target
> +            for (int i = 0; i < nThreads; i++) {
> +                threads[i] = new InvalidateThread(pool,key, obj[targ]);
> +            }
> +            for (int i = 0; i < nThreads; i++) {
> +                new Thread(threads[i]).start();
> +            }
> +            boolean done = false;
> +            while (!done) {
> +                done = true;
> +                for (int i = 0; i < nThreads; i++) {
> +                    done = done && threads[i].complete();
> +                }
> +                Thread.sleep(100);
> +            }
> +        }
> +        Assert.assertEquals(nIterations, pool.getDestroyedCount());
> +    }
> +
> +    /**
> +     * Attempts to invalidate an object, swallowing IllegalStateException.
> +     */
> +    static class InvalidateThread implements Runnable {
> +        private final String obj;
> +        private final KeyedObjectPool<String, String> pool;
> +        private final String key;
> +        private boolean done = false;
> +        public InvalidateThread(KeyedObjectPool<String, String> pool,
> String key, String obj) {
> +            this.obj = obj;
> +            this.pool = pool;
> +            this.key = key;
> +        }
> +        public void run() {
> +            try {
> +                pool.invalidateObject(key, obj);
> +            } catch (IllegalStateException ex) {
> +                // Ignore
> +            } catch (Exception ex) {
> +                Assert.fail("Unexpected exception " + ex.toString());
> +            } finally {
> +                done = true;
> +            }
> +        }
> +        public boolean complete() {
> +            return done;
> +        }
> +    }
>
>      /*
>       * Very simple test thread that just tries to borrow an object from
>
> Modified:
> commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java?rev=1426799&r1=1426798&r2=1426799&view=diff
>
> ==============================================================================
> ---
> commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
> (original)
> +++
> commons/proper/pool/trunk/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java
> Sat Dec 29 18:23:33 2012
> @@ -1100,6 +1100,84 @@ public class TestGenericObjectPool exten
>          assertEquals("Total count different than expected.", 0,
> pool.getNumActive());
>          pool.close();
>      }
> +
> +    /**
> +     * POOL-231 - verify that concurrent invalidates of the same object
> do not
> +     * corrupt pool destroyCount.
> +     */
> +    @Test
> +    public void testConcurrentInvalidate() throws Exception {
> +        // Get allObjects and idleObjects loaded with some instances
> +        final int nObjects = 1000;
> +        pool.setMaxTotal(nObjects);
> +        pool.setMaxIdle(nObjects);
> +        final Object[] obj = new Object[nObjects];
> +        for (int i = 0; i < nObjects; i++) {
> +            obj[i] = pool.borrowObject();
> +        }
> +        for (int i = 0; i < nObjects; i++) {
> +            if (i % 2 == 0) {
> +                pool.returnObject(obj[i]);
> +            }
> +        }
> +        final int nThreads = 20;
> +        final int nIterations = 60;
> +        final InvalidateThread[] threads = new InvalidateThread[nThreads];
> +        // Randomly generated list of distinct invalidation targets
> +        final ArrayList<Integer> targets = new ArrayList<Integer>();
> +        final Random random = new Random();
> +        for (int j = 0; j < nIterations; j++) {
> +            // Get a random invalidation target
> +            Integer targ = new Integer(random.nextInt(nObjects));
> +            while (targets.contains(targ)) {
> +                targ = new Integer(random.nextInt(nObjects));
> +            }
> +            targets.add(targ);
> +            // Launch nThreads threads all trying to invalidate the target
> +            for (int i = 0; i < nThreads; i++) {
> +                threads[i] = new InvalidateThread(pool, obj[targ]);
> +            }
> +            for (int i = 0; i < nThreads; i++) {
> +                new Thread(threads[i]).start();
> +            }
> +            boolean done = false;
> +            while (!done) {
> +                done = true;
> +                for (int i = 0; i < nThreads; i++) {
> +                    done = done && threads[i].complete();
> +                }
> +                Thread.sleep(100);
> +            }
> +        }
> +        Assert.assertEquals(nIterations, pool.getDestroyedCount());
> +    }
> +
> +    /**
> +     * Attempts to invalidate an object, swallowing IllegalStateException.
> +     */
> +    static class InvalidateThread implements Runnable {
> +        private final Object obj;
> +        private final ObjectPool<Object> pool;
> +        private boolean done = false;
> +        public InvalidateThread(ObjectPool<Object> pool, Object obj) {
> +            this.obj = obj;
> +            this.pool = pool;
> +        }
> +        public void run() {
> +            try {
> +                pool.invalidateObject(obj);
> +            } catch (IllegalStateException ex) {
> +                // Ignore
> +            } catch (Exception ex) {
> +                Assert.fail("Unexpected exception " + ex.toString());
> +            } finally {
> +                done = true;
> +            }
> +        }
> +        public boolean complete() {
> +            return done;
> +        }
> +    }
>
>      @Test(timeout=60000)
>      public void testMinIdle() throws Exception {
>
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
JUnit in Action, 2nd Ed: <http://goog_1249600977>http://bit.ly/ECvg0
Spring Batch in Action: <http://s.apache.org/HOq>http://bit.ly/bqpbCK
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message