From e0c46d589b12aa644e12e4a4c9e84e035f7cf98d Mon Sep 17 00:00:00 2001 From: Viktor Klang Date: Tue, 3 Sep 2024 12:55:23 +0000 Subject: [PATCH] 8325397: sun/java2d/Disposer/TestDisposerRace.java fails in linux-aarch64 Reviewed-by: alanb --- .../locks/AbstractQueuedLongSynchronizer.java | 44 +++++++++++-- .../locks/AbstractQueuedSynchronizer.java | 44 +++++++++++-- .../sun/java2d/Disposer/TestDisposerRace.java | 61 +++++++++++++++---- 3 files changed, 126 insertions(+), 23 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 2de1d17e7c1..95ac1b2e46f 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 @@ -277,6 +277,40 @@ public abstract class AbstractQueuedLongSynchronizer } } + /** + * Repeatedly invokes acquire, if its execution throws an Error or a Runtime Exception, + * using an Unsafe.park-based backoff + * @param node which to reacquire + * @param arg the acquire argument + */ + private final void reacquire(Node node, long arg) { + try { + acquire(node, arg, false, false, false, 0L); + } catch (Error | RuntimeException firstEx) { + // While we currently do not emit an JFR events in this situation, mainly + // because the conditions under which this happens are such that it + // cannot be presumed to be possible to actually allocate an event, and + // using a preconstructed one would have limited value in serviceability. + // Having said that, the following place would be the more appropriate + // place to put such logic: + // emit JFR event + + for (long nanos = 1L;;) { + U.park(false, nanos); // must use Unsafe park to sleep + if (nanos < 1L << 30) // max about 1 second + nanos <<= 1; + + try { + acquire(node, arg, false, false, false, 0L); + } catch (Error | RuntimeException ignored) { + continue; + } + + throw firstEx; + } + } + } + /** * Main acquire method, invoked by all exported acquire methods. * @@ -1299,7 +1333,7 @@ public abstract class AbstractQueuedLongSynchronizer } LockSupport.setCurrentBlocker(null); node.clearStatus(); - acquire(node, savedState, false, false, false, 0L); + reacquire(node, savedState); if (interrupted) Thread.currentThread().interrupt(); } @@ -1346,7 +1380,7 @@ public abstract class AbstractQueuedLongSynchronizer } LockSupport.setCurrentBlocker(null); node.clearStatus(); - acquire(node, savedState, false, false, false, 0L); + reacquire(node, savedState); if (interrupted) { if (cancelled) { unlinkCancelledWaiters(node); @@ -1389,7 +1423,7 @@ public abstract class AbstractQueuedLongSynchronizer LockSupport.parkNanos(this, nanos); } node.clearStatus(); - acquire(node, savedState, false, false, false, 0L); + reacquire(node, savedState); if (cancelled) { unlinkCancelledWaiters(node); if (interrupted) @@ -1433,7 +1467,7 @@ public abstract class AbstractQueuedLongSynchronizer LockSupport.parkUntil(this, abstime); } node.clearStatus(); - acquire(node, savedState, false, false, false, 0L); + reacquire(node, savedState); if (cancelled) { unlinkCancelledWaiters(node); if (interrupted) @@ -1478,7 +1512,7 @@ public abstract class AbstractQueuedLongSynchronizer LockSupport.parkNanos(this, nanos); } node.clearStatus(); - acquire(node, savedState, false, false, false, 0L); + reacquire(node, savedState); if (cancelled) { unlinkCancelledWaiters(node); if (interrupted) 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 2ea0d3ed3b0..6794bab3110 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 @@ -656,6 +656,40 @@ public abstract class AbstractQueuedSynchronizer } } + /** + * Repeatedly invokes acquire, if its execution throws an Error or a Runtime Exception, + * using an Unsafe.park-based backoff + * @param node which to reacquire + * @param arg the acquire argument + */ + private final void reacquire(Node node, int arg) { + try { + acquire(node, arg, false, false, false, 0L); + } catch (Error | RuntimeException firstEx) { + // While we currently do not emit an JFR events in this situation, mainly + // because the conditions under which this happens are such that it + // cannot be presumed to be possible to actually allocate an event, and + // using a preconstructed one would have limited value in serviceability. + // Having said that, the following place would be the more appropriate + // place to put such logic: + // emit JFR event + + for (long nanos = 1L;;) { + U.park(false, nanos); // must use Unsafe park to sleep + if (nanos < 1L << 30) // max about 1 second + nanos <<= 1; + + try { + acquire(node, arg, false, false, false, 0L); + } catch (Error | RuntimeException ignored) { + continue; + } + + throw firstEx; + } + } + } + /** * Main acquire method, invoked by all exported acquire methods. * @@ -1678,7 +1712,7 @@ public abstract class AbstractQueuedSynchronizer } LockSupport.setCurrentBlocker(null); node.clearStatus(); - acquire(node, savedState, false, false, false, 0L); + reacquire(node, savedState); if (interrupted) Thread.currentThread().interrupt(); } @@ -1725,7 +1759,7 @@ public abstract class AbstractQueuedSynchronizer } LockSupport.setCurrentBlocker(null); node.clearStatus(); - acquire(node, savedState, false, false, false, 0L); + reacquire(node, savedState); if (interrupted) { if (cancelled) { unlinkCancelledWaiters(node); @@ -1768,7 +1802,7 @@ public abstract class AbstractQueuedSynchronizer LockSupport.parkNanos(this, nanos); } node.clearStatus(); - acquire(node, savedState, false, false, false, 0L); + reacquire(node, savedState); if (cancelled) { unlinkCancelledWaiters(node); if (interrupted) @@ -1812,7 +1846,7 @@ public abstract class AbstractQueuedSynchronizer LockSupport.parkUntil(this, abstime); } node.clearStatus(); - acquire(node, savedState, false, false, false, 0L); + reacquire(node, savedState); if (cancelled) { unlinkCancelledWaiters(node); if (interrupted) @@ -1857,7 +1891,7 @@ public abstract class AbstractQueuedSynchronizer LockSupport.parkNanos(this, nanos); } node.clearStatus(); - acquire(node, savedState, false, false, false, 0L); + reacquire(node, savedState); if (cancelled) { unlinkCancelledWaiters(node); if (interrupted) diff --git a/test/jdk/sun/java2d/Disposer/TestDisposerRace.java b/test/jdk/sun/java2d/Disposer/TestDisposerRace.java index e22e5979951..b78966f1200 100644 --- a/test/jdk/sun/java2d/Disposer/TestDisposerRace.java +++ b/test/jdk/sun/java2d/Disposer/TestDisposerRace.java @@ -23,6 +23,7 @@ import java.util.LinkedList; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Supplier; import javax.swing.SwingUtilities; import sun.java2d.Disposer; @@ -39,16 +40,23 @@ public final class TestDisposerRace { private static final AtomicInteger recordsCount = new AtomicInteger(); private static volatile boolean disposerDone = false; + private static final String KO_OVERFLOW = "Some records have not been disposed!"; + private static final String KO_UNDERFLOW = "Disposed more records than were added!"; + public static void main(String[] args) throws Exception { - TestDisposerRace test = new TestDisposerRace(); - test.run(); + new TestDisposerRace().run(); checkRecordsCountIsSane(); if (recordsCount.get() > 0) { + System.err.println(KO_OVERFLOW); // In case the next line fails to allocate due to OOME throw new RuntimeException("Some records (" + recordsCount + ") have not been disposed"); } } + interface ThrowingRunnable { + void run() throws E; + } + TestDisposerRace() { addRecordsToDisposer(30_000); } @@ -56,14 +64,14 @@ public final class TestDisposerRace { void run() throws Exception { generateOOME(); for (int i = 0; i < 1000; ++i) { - SwingUtilities.invokeAndWait(Disposer::pollRemove); - if (i % 10 == 0) { - // Adding records will race with the diposer trying to remove them + retryOnOOME(() -> SwingUtilities.invokeAndWait(Disposer::pollRemove)); + + // Adding records will race with the diposer trying to remove them + if (i % 10 == 0) addRecordsToDisposer(1000); - } } - Disposer.addObjectRecord(new Object(), new FinalDisposerRecord()); + retryOnOOME(() -> Disposer.addObjectRecord(new Object(), new FinalDisposerRecord())); while (!disposerDone) { generateOOME(); @@ -72,18 +80,45 @@ public final class TestDisposerRace { private static void checkRecordsCountIsSane() { if (recordsCount.get() < 0) { - throw new RuntimeException("Disposed more records than were added"); + throw new RuntimeException(KO_UNDERFLOW); + } + } + + private static T retryOnOOME(Supplier allocator) { + for(;;) { + try { + return allocator.get(); + } catch (OutOfMemoryError ignored1) { + try { + Thread.sleep(1); // Give GC a little chance to run + } catch (InterruptedException ignored2) {} + } + } + } + + private static void retryOnOOME(ThrowingRunnable tr) throws E { + for(;;) { + try { + tr.run(); + break; + } catch (OutOfMemoryError ignored1) { + try { + Thread.sleep(1); // Give GC a little chance to run + } catch (InterruptedException ignored2) {} + } } } private void addRecordsToDisposer(int count) { checkRecordsCountIsSane(); - recordsCount.addAndGet(count); + MyDisposerRecord disposerRecord = retryOnOOME(MyDisposerRecord::new); - MyDisposerRecord disposerRecord = new MyDisposerRecord(); - for (int i = 0; i < count; i++) { - Disposer.addObjectRecord(new Object(), disposerRecord); + while(count > 0) { + recordsCount.incrementAndGet(); // pre-add to make sure it doesn't go negative + var o = retryOnOOME(Object::new); + retryOnOOME(() -> Disposer.addObjectRecord(o, disposerRecord)); + --count; } } @@ -106,8 +141,8 @@ public final class TestDisposerRace { } private static void generateOOME() throws Exception { - final List leak = new LinkedList<>(); try { + final List leak = new LinkedList<>(); while (true) { leak.add(new byte[1024 * 1024]); }