Repository: commons-pool Updated Branches: refs/heads/master 44e087d29 -> 12ba9290e
[POOL-336] GenericObjectPool's borrowObject lock if create() fails with Error. Project: http://git-wip-us.apache.org/repos/asf/commons-pool/repo Commit: http://git-wip-us.apache.org/repos/asf/commons-pool/commit/12ba9290 Tree: http://git-wip-us.apache.org/repos/asf/commons-pool/tree/12ba9290 Diff: http://git-wip-us.apache.org/repos/asf/commons-pool/diff/12ba9290 Branch: refs/heads/master Commit: 12ba9290e055c2cb86701530b66880e459794ebb Parents: 44e087d Author: Gary Gregory <ggreg...@apache.org> Authored: Sat Dec 30 23:29:51 2017 -0700 Committer: Gary Gregory <ggreg...@apache.org> Committed: Sat Dec 30 23:29:51 2017 -0700 ---------------------------------------------------------------------- src/changes/changes.xml | 5 ++ .../commons/pool2/impl/GenericObjectPool.java | 2 +- .../pool2/impl/TestGenericObjectPool.java | 75 +++++++++++++++++++- 3 files changed, 80 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/commons-pool/blob/12ba9290/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 1e225fb..95460e0 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -43,6 +43,11 @@ The <action> type attribute can be add,update,fix,remove. <title>Apache Commons Pool Changes</title> </properties> <body> + <release version="2.5.1" date="2017-12-16" description="This is a maintenance release."> + <action dev="ggregory" issue="POOL-336" type="update" due-to="Wolfgang Glas"> + GenericObjectPool's borrowObject lock if create() fails with Error. + </action> + </release> <release version="2.5.0" date="2017-12-16" description="This is a minor release, updating to Java 7."> <action dev="ggregory" issue="POOL-331" type="update"> Update from Java 6 to 7. http://git-wip-us.apache.org/repos/asf/commons-pool/blob/12ba9290/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java index 02d8d02..a6a9f53 100644 --- a/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool2/impl/GenericObjectPool.java @@ -887,7 +887,7 @@ public class GenericObjectPool<T> extends BaseGenericObjectPool<T> final PooledObject<T> p; try { p = factory.makeObject(); - } catch (final Exception e) { + } catch (final Throwable e) { createCount.decrementAndGet(); throw e; } finally { http://git-wip-us.apache.org/repos/asf/commons-pool/blob/12ba9290/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java index 915b4c4..0abc55b 100644 --- a/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java +++ b/src/test/java/org/apache/commons/pool2/impl/TestGenericObjectPool.java @@ -2126,7 +2126,7 @@ public class TestGenericObjectPool extends TestBaseObjectPool { Thread.sleep(_pause); _pool.returnObject(obj); postreturn = System.currentTimeMillis(); - } catch (final Exception e) { + } catch (final Throwable e) { _thrown = e; } finally{ ended = System.currentTimeMillis(); @@ -2635,6 +2635,79 @@ public class TestGenericObjectPool extends TestBaseObjectPool { } } + @Test + public void testErrorFactoryDoesNotBlockThreads() throws Exception { + + final CreateErrorFactory factory = new CreateErrorFactory(); + try (final GenericObjectPool<String> createFailFactoryPool = new GenericObjectPool<>(factory)) { + + createFailFactoryPool.setMaxTotal(1); + + // Try and borrow the first object from the pool + final WaitingTestThread thread1 = new WaitingTestThread(createFailFactoryPool, 0); + thread1.start(); + + // Wait for thread to reach semaphore + while (!factory.hasQueuedThreads()) { + Thread.sleep(200); + } + + // Try and borrow the second object from the pool + final WaitingTestThread thread2 = new WaitingTestThread(createFailFactoryPool, 0); + thread2.start(); + // Pool will not call factory since maximum number of object creations + // are already queued. + + // Thread 2 will wait on an object being returned to the pool + // Give thread 2 a chance to reach this state + Thread.sleep(1000); + + // Release thread1 + factory.release(); + // Pre-release thread2 + factory.release(); + + // Both threads should now complete. + boolean threadRunning = true; + int count = 0; + while (threadRunning && count < 15) { + threadRunning = thread1.isAlive(); + threadRunning = thread2.isAlive(); + Thread.sleep(200); + count++; + } + Assert.assertFalse(thread1.isAlive()); + Assert.assertFalse(thread2.isAlive()); + + Assert.assertTrue(thread1._thrown instanceof UnknownError); + Assert.assertTrue(thread2._thrown instanceof UnknownError); + } + } + + private static class CreateErrorFactory extends BasePooledObjectFactory<String> { + + private final Semaphore semaphore = new Semaphore(0); + + @Override + public String create() throws Exception { + semaphore.acquire(); + throw new UnknownError("wiggle"); + } + + @Override + public PooledObject<String> wrap(final String obj) { + return new DefaultPooledObject<>(obj); + } + + public void release() { + semaphore.release(); + } + + public boolean hasQueuedThreads() { + return semaphore.hasQueuedThreads(); + } + } + private BasePooledObjectFactory<String> createBasePooledObjectfactory() { return new BasePooledObjectFactory<String>() { @Override
