Return-Path: X-Original-To: apmail-commons-commits-archive@minotaur.apache.org Delivered-To: apmail-commons-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 4F740E9E9 for ; Sat, 29 Dec 2012 18:23:58 +0000 (UTC) Received: (qmail 67599 invoked by uid 500); 29 Dec 2012 18:23:58 -0000 Delivered-To: apmail-commons-commits-archive@commons.apache.org Received: (qmail 67528 invoked by uid 500); 29 Dec 2012 18:23:58 -0000 Mailing-List: contact commits-help@commons.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@commons.apache.org Delivered-To: mailing list commits@commons.apache.org Received: (qmail 67519 invoked by uid 99); 29 Dec 2012 18:23:57 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 29 Dec 2012 18:23:57 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 29 Dec 2012 18:23:54 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id A16FF23888E7; Sat, 29 Dec 2012 18:23:33 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1426799 - in /commons/proper/pool/trunk/src: main/java/org/apache/commons/pool2/impl/ test/java/org/apache/commons/pool2/impl/ Date: Sat, 29 Dec 2012 18:23:33 -0000 To: commits@commons.apache.org From: psteitz@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20121229182333.A16FF23888E7@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org 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 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 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 targets = new ArrayList(); + 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 pool; + private final String key; + private boolean done = false; + public InvalidateThread(KeyedObjectPool 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 targets = new ArrayList(); + 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 pool; + private boolean done = false; + public InvalidateThread(ObjectPool 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 {