This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-pool.git
The following commit(s) were added to refs/heads/master by this push:
new 466c7e51 The maximum wait time for GenericObjectPool.borrowObject(*)
may exceed expectations due to a spurious thread wakeup
466c7e51 is described below
commit 466c7e51fb621cbf5deb67082af09b07d1120d5f
Author: Gary Gregory <garydgreg...@gmail.com>
AuthorDate: Sun Nov 24 18:44:07 2024 -0500
The maximum wait time for GenericObjectPool.borrowObject(*) may exceed
expectations due to a spurious thread wakeup
---
src/changes/changes.xml | 1 +
.../apache/commons/pool3/impl/GenericObjectPool.java | 20 +++++++++++---------
.../commons/pool3/impl/TestGenericObjectPool.java | 4 ++--
3 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 1930f822..5933e84d 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -68,6 +68,7 @@ The <action> type attribute can be add,update,fix,remove.
<!-- FIX -->
<action type="fix" dev="ggregory" due-to="Gary Gregory">Use
java.time.Instant precision in
org.apache.commons.pool2.impl.ThrowableCallStack.Snapshot throwable
message.</action>
<action type="fix" dev="ggregory" due-to="Gary
Gregory">GenericObjectPool.borrowObject(Duration) doesn't obey its
borrowMaxWait Duration argument when the argument is different from
GenericObjectPool.getMaxWaitDuration().</action>
+ <action type="fix" issue="POOL-418" dev="ggregory" due-to="Gary
Gregory">The maximum wait time for GenericObjectPool.borrowObject(*) may exceed
expectations due to a spurious thread wakeup.</action>
<!-- ADD -->
<!-- UPDATE -->
<action type="update" dev="ggregory">Bump
org.apache.commons:commons-parent from 62 to 73.</action>
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 04d7f1b3..7d957177 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<T, E extends Exception>
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 class GenericObjectPool<T, E extends Exception>
extends BaseGenericObject
* @throws E if the object factory's {@code makeObject} fails
*/
private PooledObject<T> 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 @@ public class GenericObjectPool<T, E extends Exception>
extends BaseGenericObject
// 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 @@ public class GenericObjectPool<T, E extends Exception>
extends BaseGenericObject
// 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 @@ public class GenericObjectPool<T, E extends Exception>
extends BaseGenericObject
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 6354d5b1..184a6db4 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 class TestGenericObjectPool extends
TestBaseObjectPool {
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));
}
}
