From 42e67c6c0c55604c4bcd559d3858c7bac6dee558 Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Thu, 18 Jun 2026 10:34:14 +0000 Subject: [PATCH 1/2] 8386085: Livelock in AbstractQueuedSyncronizer.cleanQueue() when multiple threads do tryAcquire() with a short timeout and no permits available Reviewed-by: dl, alanb --- .../locks/AbstractQueuedLongSynchronizer.java | 3 +- .../locks/AbstractQueuedSynchronizer.java | 3 +- .../util/concurrent/locks/StampedLock.java | 3 +- .../util/concurrent/tck/SemaphoreTest.java | 39 +++++++++++++++++++ .../util/concurrent/tck/StampedLockTest.java | 37 +++++++++++++++++- 5 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java b/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java index 83de94cd612..3f2d6771d67 100644 --- a/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java +++ b/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedLongSynchronizer.java @@ -372,7 +372,8 @@ private void cleanQueue() { if (q.status < 0) { // cancelled if ((s == null ? casTail(q, p) : s.casPrev(q, p)) && q.prev == p) { - p.casNext(q, s); // OK if fails + if (s != null) + p.casNext(q, s); // OK if fails if (p.prev == null) signalNext(p); } diff --git a/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java b/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java index 7a320b6f432..27043aa0495 100644 --- a/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java +++ b/src/java.base/share/classes/java/util/concurrent/locks/AbstractQueuedSynchronizer.java @@ -740,7 +740,8 @@ private void cleanQueue() { if (q.status < 0) { // cancelled if ((s == null ? casTail(q, p) : s.casPrev(q, p)) && q.prev == p) { - p.casNext(q, s); // OK if fails + if (s != null) + p.casNext(q, s); // OK if fails if (p.prev == null) signalNext(p); } diff --git a/src/java.base/share/classes/java/util/concurrent/locks/StampedLock.java b/src/java.base/share/classes/java/util/concurrent/locks/StampedLock.java index b9f0e9340b7..8e6fcaae22a 100644 --- a/src/java.base/share/classes/java/util/concurrent/locks/StampedLock.java +++ b/src/java.base/share/classes/java/util/concurrent/locks/StampedLock.java @@ -1405,7 +1405,8 @@ private void cleanQueue() { if (q.status < 0) { // cancelled if ((s == null ? casTail(q, p) : s.casPrev(q, p)) && q.prev == p) { - p.casNext(q, s); // OK if fails + if (s != null) + p.casNext(q, s); // OK if fails if (p.prev == null) signalNext(p); } diff --git a/test/jdk/java/util/concurrent/tck/SemaphoreTest.java b/test/jdk/java/util/concurrent/tck/SemaphoreTest.java index 5d45dc21a8e..38ba54645b9 100644 --- a/test/jdk/java/util/concurrent/tck/SemaphoreTest.java +++ b/test/jdk/java/util/concurrent/tck/SemaphoreTest.java @@ -33,11 +33,15 @@ * Pat Fisher, Mike Judd. */ +import static java.util.concurrent.TimeUnit.MICROSECONDS; import static java.util.concurrent.TimeUnit.MILLISECONDS; import java.util.Collection; +import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicBoolean; import junit.framework.Test; import junit.framework.TestSuite; @@ -672,4 +676,39 @@ public void testToString(boolean fair) { assertTrue(s.toString().contains("Permits = -2")); } + /** + * Test scenario for JDK-8386085 + * When investigating failures of this test, it is important to know that AbstractQueuedSynchronizer, + * AbstractQueuedLongSynchronizer, and StampedLock all share similar code which was fixed for JDK-8386085 + */ + public void testShortTimeoutAcquisition() throws InterruptedException { + final int width = Runtime.getRuntime().availableProcessors(); + try (var pool = Executors.newFixedThreadPool(width)) { + // Setup + final AtomicBoolean done = new AtomicBoolean(false); + final CountDownLatch waitingToRun = new CountDownLatch(width); + final Semaphore s = new Semaphore(0); + final Callable c = () -> { + waitingToRun.countDown(); + do { + s.tryAcquire(1, MICROSECONDS); // acquisition storm + } while (!done.get()); + return null; + }; + + // Task creation + for(int i = 0; i < width; ++i) + pool.submit(c); + + waitingToRun.await(); // Wait for all tasks to start + Thread.sleep(3000); // Wait a while for acquisitions + s.release(width); // Hand out permits + Thread.sleep(1000); // Wait a while for permit acquisitions + + final int permitsAvailable = s.availablePermits(); + done.set(true); // Ensure that tasks can exit + assertTrue(permitsAvailable < width); // Some permits should've been taken + } + } + } diff --git a/test/jdk/java/util/concurrent/tck/StampedLockTest.java b/test/jdk/java/util/concurrent/tck/StampedLockTest.java index 673047ccf12..a6b68357712 100644 --- a/test/jdk/java/util/concurrent/tck/StampedLockTest.java +++ b/test/jdk/java/util/concurrent/tck/StampedLockTest.java @@ -33,8 +33,8 @@ */ import static java.util.concurrent.TimeUnit.DAYS; +import static java.util.concurrent.TimeUnit.MICROSECONDS; import static java.util.concurrent.TimeUnit.MILLISECONDS; - import static java.util.concurrent.locks.StampedLock.isLockStamp; import static java.util.concurrent.locks.StampedLock.isOptimisticReadStamp; import static java.util.concurrent.locks.StampedLock.isReadLockStamp; @@ -45,6 +45,7 @@ import java.util.concurrent.Callable; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.TimeUnit; @@ -1527,4 +1528,38 @@ public void testConcurrentAccess() throws Exception { checkTimedGet(future, null); } + /** + * test scenario for JDK-8386085 + */ + public void testShortTimeoutAcquisition() throws InterruptedException { + final int width = Runtime.getRuntime().availableProcessors(); + final StampedLock s = new StampedLock(); + try (var pool = Executors.newFixedThreadPool(width)) { + // Setup + final AtomicBoolean done = new AtomicBoolean(false); + final CountDownLatch waitingToRun = new CountDownLatch(width); + final long stamp = s.writeLock(); + assertTrue(s.validate(stamp)); + final Callable c = () -> { + waitingToRun.countDown(); + do { + long lock = s.tryWriteLock(1, MICROSECONDS); // acquisition storm + if (s.validate(lock)) + s.unlockWrite(lock); + } while (!done.get()); + return null; + }; + + // Task creation + for(int i = 0; i < width; ++i) + pool.submit(c); + + waitingToRun.await(); // Wait for all tasks to start + Thread.sleep(3000); // Wait a while for acquisitions + s.unlockWrite(stamp); // Hand out permits + Thread.sleep(1000); // Wait a while for permit acquisitions + done.set(true); // Ensure that tasks can exit + } + assertTrue(s.validate(s.writeLockInterruptibly())); // Should succeed + } } From 2b9f2c2bdb1f65b52b13f490997467dbe102f438 Mon Sep 17 00:00:00 2001 From: Kirill Shirokov Date: Fri, 19 Jun 2026 05:14:51 +0000 Subject: [PATCH 2/2] Fix new tests for JDK 17 where ExecutorService does not implement AutoCloseable --- test/jdk/java/util/concurrent/tck/SemaphoreTest.java | 5 ++++- test/jdk/java/util/concurrent/tck/StampedLockTest.java | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/test/jdk/java/util/concurrent/tck/SemaphoreTest.java b/test/jdk/java/util/concurrent/tck/SemaphoreTest.java index 38ba54645b9..451d060175d 100644 --- a/test/jdk/java/util/concurrent/tck/SemaphoreTest.java +++ b/test/jdk/java/util/concurrent/tck/SemaphoreTest.java @@ -683,7 +683,8 @@ public void testToString(boolean fair) { */ public void testShortTimeoutAcquisition() throws InterruptedException { final int width = Runtime.getRuntime().availableProcessors(); - try (var pool = Executors.newFixedThreadPool(width)) { + final var pool = Executors.newFixedThreadPool(width); + try { // Setup final AtomicBoolean done = new AtomicBoolean(false); final CountDownLatch waitingToRun = new CountDownLatch(width); @@ -708,6 +709,8 @@ public void testShortTimeoutAcquisition() throws InterruptedException { final int permitsAvailable = s.availablePermits(); done.set(true); // Ensure that tasks can exit assertTrue(permitsAvailable < width); // Some permits should've been taken + } finally { + pool.shutdown(); } } diff --git a/test/jdk/java/util/concurrent/tck/StampedLockTest.java b/test/jdk/java/util/concurrent/tck/StampedLockTest.java index a6b68357712..241a73b4218 100644 --- a/test/jdk/java/util/concurrent/tck/StampedLockTest.java +++ b/test/jdk/java/util/concurrent/tck/StampedLockTest.java @@ -1534,7 +1534,8 @@ public void testConcurrentAccess() throws Exception { public void testShortTimeoutAcquisition() throws InterruptedException { final int width = Runtime.getRuntime().availableProcessors(); final StampedLock s = new StampedLock(); - try (var pool = Executors.newFixedThreadPool(width)) { + final var pool = Executors.newFixedThreadPool(width); + try { // Setup final AtomicBoolean done = new AtomicBoolean(false); final CountDownLatch waitingToRun = new CountDownLatch(width); @@ -1559,6 +1560,8 @@ public void testShortTimeoutAcquisition() throws InterruptedException { s.unlockWrite(stamp); // Hand out permits Thread.sleep(1000); // Wait a while for permit acquisitions done.set(true); // Ensure that tasks can exit + } finally { + pool.shutdown(); } assertTrue(s.validate(s.writeLockInterruptibly())); // Should succeed }