On 10/01/2013 16:21, Gary Gregory wrote: > Should this be ported to the 1.6 branch?
Don't think so. That looks to be specific to pool2. Mark > > Gary > > > On Sat, Dec 29, 2012 at 1:23 PM, <pste...@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 { >> >> >> > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
