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..451d060175d 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,42 @@ 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(); + final var pool = Executors.newFixedThreadPool(width); + try { + // 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 + } finally { + pool.shutdown(); + } + } + } diff --git a/test/jdk/java/util/concurrent/tck/StampedLockTest.java b/test/jdk/java/util/concurrent/tck/StampedLockTest.java index 673047ccf12..241a73b4218 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,41 @@ 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(); + final var pool = Executors.newFixedThreadPool(width); + try { + // 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 + } finally { + pool.shutdown(); + } + assertTrue(s.validate(s.writeLockInterruptibly())); // Should succeed + } }