You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ps...@apache.org on 2012/12/29 19:23:33 UTC
svn commit: r1426799 - in /commons/proper/pool/trunk/src:
main/java/org/apache/commons/pool2/impl/
test/java/org/apache/commons/pool2/impl/
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 {
Re: svn commit: r1426799 - in /commons/proper/pool/trunk/src: main/java/org/apache/commons/pool2/impl/
test/java/org/apache/commons/pool2/impl/
Posted by Phil Steitz <ph...@gmail.com>.
On 1/10/13 8:24 AM, Mark Thomas wrote:
> 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.
Right. In pool 1, multiple threads trying to invalidate the same
object violates the pool contract and adding sync protection won't
help. Serializing the repeated invalidates still results in pool
counters borked. This is a 1.x feature (multiple invalidates or
returns on the same object violate the contract and when you do
this, there is no guarantee the pool counters are going to be
correct). In pool 2, multiple invalidates cause an exception and
the pool counters maintain integrity. The fix below just makes sure
that when multiple threads try to concurrently invalidate an object,
at most one of them succeeds and the pool counters are updated only
once. Looking carefully again at the fix, I realize now that while
it does ensure counter integrity, it does not ensure that multiple
invalidates always throw. It might actually be best to add an else
that throws in the sync block. The javadoc should also probably be
a little more explicit about what the contract is. What do others
think?
Phil
>
> Mark
>
>
>> Gary
>>
>>
>> On Sat, Dec 29, 2012 at 1:23 PM, <ps...@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-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: r1426799 - in /commons/proper/pool/trunk/src: main/java/org/apache/commons/pool2/impl/
test/java/org/apache/commons/pool2/impl/
Posted by Mark Thomas <ma...@apache.org>.
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, <ps...@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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1426799 - in /commons/proper/pool/trunk/src:
main/java/org/apache/commons/pool2/impl/ test/java/org/apache/commons/pool2/impl/
Posted by Gary Gregory <ga...@gmail.com>.
Should this be ported to the 1.6 branch?
Gary
On Sat, Dec 29, 2012 at 1:23 PM, <ps...@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