diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 1930f8227..5933e84da 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -68,6 +68,7 @@ The type attribute can be add,update,fix,remove. Use java.time.Instant precision in org.apache.commons.pool2.impl.ThrowableCallStack.Snapshot throwable message. GenericObjectPool.borrowObject(Duration) doesn't obey its borrowMaxWait Duration argument when the argument is different from GenericObjectPool.getMaxWaitDuration(). + The maximum wait time for GenericObjectPool.borrowObject(*) may exceed expectations due to a spurious thread wakeup. Bump org.apache.commons:commons-parent from 62 to 73. diff --git a/src/main/java/org/apache/commons/pool3/impl/GenericObjectPool.java b/src/main/java/org/apache/commons/pool3/impl/GenericObjectPool.java index 04d7f1b3f..7d9571773 100644 --- a/src/main/java/org/apache/commons/pool3/impl/GenericObjectPool.java +++ b/src/main/java/org/apache/commons/pool3/impl/GenericObjectPool.java @@ -88,7 +88,9 @@ public class GenericObjectPool extends BaseGenericObject "org.apache.commons.pool3:type=GenericObjectPool,name="; private static void wait(final Object obj, final Duration duration) throws InterruptedException { - obj.wait(duration.toMillis(), duration.getNano() % 1_000_000); + if (!duration.isNegative()) { + obj.wait(duration.toMillis(), duration.getNano() % 1_000_000); + } } private volatile String factoryType; @@ -509,15 +511,14 @@ public void close() { * @throws E if the object factory's {@code makeObject} fails */ private PooledObject create(final Duration maxWaitDuration) throws E { + final Instant startInstant = Instant.now(); + Duration remainingWaitDuration = maxWaitDuration.isNegative() ? Duration.ZERO : maxWaitDuration; int localMaxTotal = getMaxTotal(); // This simplifies the code later in this method if (localMaxTotal < 0) { localMaxTotal = Integer.MAX_VALUE; } - final Instant localStartInstant = Instant.now(); - final Duration localMaxWaitDuration = maxWaitDuration.isNegative() ? Duration.ZERO : maxWaitDuration; - // Flag that indicates if create should: // - TRUE: call the factory to create an object // - FALSE: return null @@ -525,6 +526,8 @@ private PooledObject create(final Duration maxWaitDuration) throws E { // call the factory Boolean create = null; while (create == null) { + // remainingWaitDuration handles spurious wakeup from wait(). + remainingWaitDuration = remainingWaitDuration.minus(durationSince(startInstant)); synchronized (makeObjectCountLock) { final long newCreateCount = createCount.incrementAndGet(); if (newCreateCount > localMaxTotal) { @@ -542,7 +545,7 @@ private PooledObject create(final Duration maxWaitDuration) throws E { // fail so wait until they complete and then re-test if // the pool is at capacity or not. try { - wait(makeObjectCountLock, localMaxWaitDuration); + wait(makeObjectCountLock, remainingWaitDuration); } catch (final InterruptedException e) { // Don't surface exception type of internal locking mechanism. throw cast(e); @@ -554,10 +557,9 @@ private PooledObject create(final Duration maxWaitDuration) throws E { create = Boolean.TRUE; } } - - // Do not block more if localMaxWaitDuration > 0. - if (create == null && localMaxWaitDuration.compareTo(Duration.ZERO) > 0 && - durationSince(localStartInstant).compareTo(localMaxWaitDuration) >= 0) { + // Do not block more if remainingWaitDuration > 0. + if (create == null && remainingWaitDuration.compareTo(Duration.ZERO) > 0 && + durationSince(localStartInstant).compareTo(remainingWaitDuration) >= 0) { create = Boolean.FALSE; } } diff --git a/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java b/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java index 6354d5b15..184a6db41 100644 --- a/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java +++ b/src/test/java/org/apache/commons/pool3/impl/TestGenericObjectPool.java @@ -1941,8 +1941,8 @@ public void testFailingFactoryDoesNotBlockThreads() throws Exception { assertFalse(thread1.isAlive()); assertFalse(thread2.isAlive()); - assertTrue(thread1.thrown instanceof UnsupportedCharsetException); - assertTrue(thread2.thrown instanceof UnsupportedCharsetException); + assertTrue(thread1.thrown instanceof UnsupportedCharsetException, () -> Objects.toString(thread1.thrown)); + assertTrue(thread2.thrown instanceof UnsupportedCharsetException, () -> Objects.toString(thread2.thrown)); } }