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 {
