This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git
The following commit(s) were added to refs/heads/master by this push:
new 5914a8e80 TimedSemaphore.shutdown() must wake threads blocked in
acquire(). (#1639)
5914a8e80 is described below
commit 5914a8e80a547301b76bf8ef691053e4dfc901a7
Author: Gary Gregory <[email protected]>
AuthorDate: Thu May 7 10:05:15 2026 -0400
TimedSemaphore.shutdown() must wake threads blocked in acquire(). (#1639)
---
.../commons/lang3/concurrent/TimedSemaphore.java | 4 ++
.../lang3/concurrent/TimedSemaphoreTest.java | 61 ++++++++++++++++++++++
2 files changed, 65 insertions(+)
diff --git
a/src/main/java/org/apache/commons/lang3/concurrent/TimedSemaphore.java
b/src/main/java/org/apache/commons/lang3/concurrent/TimedSemaphore.java
index f601ab4f1..b389f33bf 100644
--- a/src/main/java/org/apache/commons/lang3/concurrent/TimedSemaphore.java
+++ b/src/main/java/org/apache/commons/lang3/concurrent/TimedSemaphore.java
@@ -297,6 +297,9 @@ public synchronized void acquire() throws
InterruptedException {
canPass = acquirePermit();
if (!canPass) {
wait();
+ if (shutdown) {
+ throw new IllegalStateException("TimedSemaphore is shut
down.");
+ }
}
} while (!canPass);
}
@@ -454,6 +457,7 @@ public synchronized void shutdown() {
task.cancel(false);
}
shutdown = true;
+ notifyAll();
}
}
diff --git
a/src/test/java/org/apache/commons/lang3/concurrent/TimedSemaphoreTest.java
b/src/test/java/org/apache/commons/lang3/concurrent/TimedSemaphoreTest.java
index 6cd284426..10c498844 100644
--- a/src/test/java/org/apache/commons/lang3/concurrent/TimedSemaphoreTest.java
+++ b/src/test/java/org/apache/commons/lang3/concurrent/TimedSemaphoreTest.java
@@ -21,6 +21,7 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.time.Duration;
@@ -474,6 +475,66 @@ void testShutdownSharedExecutorTask() throws
InterruptedException {
EasyMock.verify(service, future);
}
+ /**
+ * TimedSemaphore.shutdown() must wake threads blocked in acquire().
+ *
+ * <p>
+ * Pre-patch ({@code shutdown()} sets the flag but does not call {@code
notifyAll()}): a thread parked in {@code wait()} inside {@code acquire()} stays
+ * parked indefinitely — until the periodic {@code endOfPeriod()} task
fires, which never happens here because we deliberately use a long period (60
s).
+ * </p>
+ *
+ * <p>
+ * Post-patch: {@code shutdown()} calls {@code notifyAll()} after setting
the flag, and {@code acquire()} re-checks the flag on wake to throw
+ * {@link IllegalStateException}.
+ * </p>
+ *
+ * <p>
+ * Differences from the previous (vacuous) version of this PoC:
+ * </p>
+ * <ul>
+ * <li>Uses period = 60 s (was 1 s). With a 1-second period, the periodic
{@code endOfPeriod()} task wakes the blocker on the next tick, hiding the
+ * bug.</li>
+ * <li>Asserts {@code !blocker.isAlive()} after the join. {@code
Thread.join(timeout)} returns silently after the timeout regardless of whether
the thread
+ * terminated, so the previous assertion ({@code assertTimeout(2s, () ->
blocker.join(1500))}) was satisfied even when the blocker was still parked.</li>
+ * <li>Uses {@code assertTimeoutPreemptively} so the test framework
forcibly interrupts a hanging test rather than hanging the JVM.</li>
+ * <li>Waits for the blocker to actually reach {@code
Thread.State.WAITING} before calling {@code shutdown()}, removing the {@code
Thread.sleep(100)}
+ * race.</li>
+ * </ul>
+ */
+ @Test
+ public void testShutdownWakesBlockedAcquireThreads() {
+ assertTimeoutPreemptively(Duration.ofSeconds(10), () -> {
+ // Period of 60s ensures endOfPeriod() does NOT fire during the
test
+ // window. The only way the blocker can wake is via shutdown()
calling
+ // notifyAll().
+ final TimedSemaphore sem =
TimedSemaphore.builder().setPeriod(60).setTimeUnit(TimeUnit.SECONDS).setLimit(1).get();
+ sem.acquire(); // consume the only permit for this period.
+ final Thread blocker = new Thread(() -> {
+ try {
+ sem.acquire(); // limit=1 already taken => blocks in
wait().
+ } catch (final InterruptedException e) {
+ Thread.currentThread().interrupt();
+ } catch (final IllegalStateException e) {
+ // Acceptable post-patch outcome: re-check of shutdown
flag throws.
+ }
+ }, "testShutdownWakesBlockedAcquireThreads");
+ blocker.setDaemon(true);
+ blocker.start();
+ // Wait until blocker is parked in Object.wait() inside acquire().
+ final long parkDeadline = System.nanoTime() +
Duration.ofSeconds(2).toNanos();
+ while (System.nanoTime() < parkDeadline && blocker.getState() !=
Thread.State.WAITING) {
+ Thread.sleep(10);
+ }
+ sem.shutdown();
+ // At HEAD (patched): blocker wakes from notifyAll(), re-checks
flag,
+ // throws ISE, and terminates within milliseconds.
+ // At baseline: blocker stays in WAITING for 60s — well past this
join.
+ blocker.join(5000);
+ assertFalse(blocker.isAlive(), "TimedSemaphore.shutdown() failed
to wake thread blocked in acquire(): blocker still alive in state="
+ + blocker.getState() + " 5s after shutdown(). Bug present
(shutdown() does not call notifyAll()).");
+ });
+ }
+
/**
* Tests starting the timer.
*
