8308609: java/lang/ScopedValue/StressStackOverflow.java fails with "-XX:-VMContinuations"

Reviewed-by: alanb

src/hotspot/share/prims/jvm.cpp

@@ -1385,9 +1385,8 @@ JVM_ENTRY(jobject, JVM_FindScopedValueBindings(JNIEnv *env, jclass cls))
 
     InstanceKlass* holder = method->method_holder();
     if (name == vmSymbols::runWith_method_name()) {
-      if ((holder == resolver.Carrier_klass
+      if (holder == vmClasses::Thread_klass()
-           || holder == vmClasses::VirtualThread_klass()
+          || holder == resolver.Carrier_klass) {
-           || holder == vmClasses::Thread_klass())) {
         loc = 1;
       }
     }

src/java.base/share/classes/java/lang/Thread.java

@@ -1578,7 +1578,7 @@ public class Thread implements Runnable {
      */
     @Hidden
     @ForceInline
-    private void runWith(Object bindings, Runnable op) {
+    final void runWith(Object bindings, Runnable op) {
         ensureMaterializedForStackWalk(bindings);
         op.run();
         Reference.reachabilityFence(bindings);

src/java.base/share/classes/java/lang/ThreadBuilders.java

@@ -433,7 +433,8 @@ class ThreadBuilders {
             // run is specified to do nothing when Thread is a virtual thread
             if (Thread.currentThread() == this && !runInvoked) {
                 runInvoked = true;
-                task.run();
+                Object bindings = Thread.scopedValueBindings();
+                runWith(bindings, task);
             }
         }
 

src/java.base/share/classes/java/lang/VirtualThread.java

@@ -24,7 +24,6 @@
  */
 package java.lang;
 
-import java.lang.ref.Reference;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.util.Locale;
@@ -54,7 +53,6 @@ import jdk.internal.vm.StackableScope;
 import jdk.internal.vm.ThreadContainer;
 import jdk.internal.vm.ThreadContainers;
 import jdk.internal.vm.annotation.ChangesCurrentThread;
-import jdk.internal.vm.annotation.ForceInline;
 import jdk.internal.vm.annotation.Hidden;
 import jdk.internal.vm.annotation.IntrinsicCandidate;
 import jdk.internal.vm.annotation.JvmtiMountTransition;
@@ -306,7 +304,7 @@ final class VirtualThread extends BaseVirtualThread {
             event.commit();
         }
 
-        Object bindings = scopedValueBindings();
+        Object bindings = Thread.scopedValueBindings();
         try {
             runWith(bindings, task);
         } catch (Throwable exc) {
@@ -334,14 +332,6 @@ final class VirtualThread extends BaseVirtualThread {
         }
     }
 
-    @Hidden
-    @ForceInline
-    private void runWith(Object bindings, Runnable op) {
-        ensureMaterializedForStackWalk(bindings);
-        op.run();
-        Reference.reachabilityFence(bindings);
-    }
-
     /**
      * Mounts this virtual thread onto the current platform thread. On
      * return, the current thread is the virtual thread.

test/jdk/ProblemList-Virtual.txt

@@ -42,8 +42,6 @@ javax/management/remote/mandatory/loading/MissingClassTest.java 8145413 windows-
 
 javax/management/remote/mandatory/loading/RMIDownloadTest.java 8308366 windows-x64
 
-java/lang/ScopedValue/StressStackOverflow.java 8309646 generic-all
-
 java/lang/instrument/NativeMethodPrefixAgent.java 8307169 generic-all
 
 ##########

test/jdk/ProblemList.txt

@@ -487,8 +487,6 @@ java/lang/invoke/LFCaching/LFGarbageCollectedTest.java          8078602 generic-
 java/lang/invoke/lambda/LambdaFileEncodingSerialization.java    8249079 linux-x64
 java/lang/invoke/RicochetTest.java                              8251969 generic-all
 
-java/lang/ScopedValue/StressStackOverflow.java                  8303498 linux-s390x
-
 ############################################################################
 
 # jdk_instrument

test/jdk/java/lang/ScopedValue/StressStackOverflow.java

@@ -21,8 +21,8 @@
  * questions.
  */
 
-/**
+/*
- * @test
+ * @test id=default
  * @summary StressStackOverflow the recovery path for ScopedValue
  * @enablePreview
  * @run main/othervm/timeout=300 -XX:-TieredCompilation StressStackOverflow
@@ -30,8 +30,14 @@
  * @run main/othervm/timeout=300 StressStackOverflow
  */
 
+/*
+ * @test id=no-vmcontinuations
+ * @requires vm.continuations
+ * @enablePreview
+ * @run main/othervm/timeout=300 -XX:+UnlockExperimentalVMOptions -XX:-VMContinuations StressStackOverflow
+ */
+
 import java.util.concurrent.Callable;
-import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.ThreadLocalRandom;
 import java.util.concurrent.StructureViolationException;
 import java.util.concurrent.StructuredTaskScope;
@@ -42,7 +48,6 @@ public class StressStackOverflow {
 
     public static final ScopedValue<Integer> inheritedValue = ScopedValue.newInstance();
 
-    final ThreadLocalRandom tlr = ThreadLocalRandom.current();
     static final TestFailureException testFailureException = new TestFailureException("Unexpected value for ScopedValue");
     int ITERS = 1_000_000;
 
@@ -50,13 +55,15 @@ public class StressStackOverflow {
         TestFailureException(String s) { super(s); }
     }
 
+    static final long MINUTES = 60 * 1_000_000_000L; // 60 * 10**9 ns
+
     // Test the ScopedValue recovery mechanism for stack overflows. We implement both Callable
     // and Runnable interfaces. Which one gets tested depends on the constructor argument.
-    class DeepRecursion implements Callable, Supplier, Runnable {
+    class DeepRecursion implements Callable<Object>, Supplier<Object>, Runnable {
 
-        static enum Behaviour {
+        enum Behaviour {
             CALL, GET, RUN;
-            private static Behaviour[] values = values();
+            private static final Behaviour[] values = values();
             public static Behaviour choose(ThreadLocalRandom tlr) {
                 return values[tlr.nextInt(3)];
             }
@@ -70,38 +77,40 @@ public class StressStackOverflow {
 
         public void run() {
             final var last = el.get();
-            ITERS--;
+            while (ITERS-- > 0) {
-            var nextRandomFloat = tlr.nextFloat();
+                if (System.nanoTime() - startTime > 3 * MINUTES) { // 3 minutes is long enough
-            try {
+                    return;
-                switch (behaviour) {
-                    case CALL ->
-                        ScopedValue.where(el, el.get() + 1).call(() -> fibonacci_pad(20, this));
-                    case GET ->
-                        ScopedValue.where(el, el.get() + 1).get(() -> fibonacci_pad(20, this));
-                    case RUN ->
-                        ScopedValue.where(el, el.get() + 1).run(() -> fibonacci_pad(20, this));
                 }
-                if (!last.equals(el.get())) {
-                    throw testFailureException;
-                }
-            } catch (StackOverflowError e) {
-                if (nextRandomFloat <= 0.1) {
-                    ScopedValue.where(el, el.get() + 1).run(this);
-                }
-            } catch (TestFailureException e) {
-                throw e;
-            } catch (Throwable throwable) {
-                // StackOverflowErrors cause many different failures. These include
-                // StructureViolationExceptions and InvocationTargetExceptions. This test
-                // checks that, no matter what the failure mode, scoped values are handled
-                // correctly.
-            } finally {
-                if (!last.equals(el.get())) {
-                    throw testFailureException;
-                }
-            }
 
-            Thread.yield();
+                var nextRandomFloat = ThreadLocalRandom.current().nextFloat();
+                try {
+                    switch (behaviour) {
+                        case CALL -> ScopedValue.where(el, el.get() + 1).call(() -> fibonacci_pad(20, this));
+                        case GET -> ScopedValue.where(el, el.get() + 1).get(() -> fibonacci_pad(20, this));
+                        case RUN -> ScopedValue.where(el, el.get() + 1).run(() -> fibonacci_pad(20, this));
+                    }
+                    if (!last.equals(el.get())) {
+                        throw testFailureException;
+                    }
+                } catch (StackOverflowError e) {
+                    if (nextRandomFloat <= 0.1) {
+                        ScopedValue.where(el, el.get() + 1).run(this);
+                    }
+                } catch (TestFailureException e) {
+                    throw e;
+                } catch (Throwable throwable) {
+                    // StackOverflowErrors cause many different failures. These include
+                    // StructureViolationExceptions and InvocationTargetExceptions. This test
+                    // checks that, no matter what the failure mode, scoped values are handled
+                    // correctly.
+                } finally {
+                    if (!last.equals(el.get())) {
+                        throw testFailureException;
+                    }
+                }
+
+                Thread.yield();
+            }
         }
 
         public Object get() {
@@ -114,13 +123,10 @@ public class StressStackOverflow {
         }
     }
 
-    static final Runnable nop = new Runnable() {
+    static final Runnable nop = () -> {};
-        public void run() { }
-    };
 
     // Consume some stack.
     //
-
     // The double recursion used here prevents an optimizing JIT from
     // inlining all the recursive calls, which would make it
     // ineffective.
@@ -137,7 +143,7 @@ public class StressStackOverflow {
     long fibonacci_pad(int n, Runnable op) {
         final var last = el.get();
         try {
-            return fibonacci_pad1(tlr.nextInt(n), op);
+            return fibonacci_pad1(ThreadLocalRandom.current().nextInt(n), op);
         } catch (StackOverflowError err) {
             if (!inheritedValue.get().equals(I_42)) {
                 throw testFailureException;
@@ -152,14 +158,16 @@ public class StressStackOverflow {
     // Run op in a new thread. Platform or virtual threads are chosen at random.
     void runInNewThread(Runnable op) {
         var threadFactory
-                = (tlr.nextBoolean() ? Thread.ofPlatform() : Thread.ofVirtual()).factory();
+                = (ThreadLocalRandom.current().nextBoolean() ? Thread.ofPlatform() : Thread.ofVirtual()).factory();
-        try (var scope = new StructuredTaskScope<Object>("", threadFactory)) {
+        try (var scope = new StructuredTaskScope<>("", threadFactory)) {
             var handle = scope.fork(() -> {
                 op.run();
                 return null;
             });
             scope.join();
             handle.get();
+        } catch (TestFailureException e) {
+            throw e;
         } catch (Exception e) {
             throw new RuntimeException(e);
         }
@@ -168,12 +176,12 @@ public class StressStackOverflow {
     public void run() {
         try {
             ScopedValue.where(inheritedValue, 42).where(el, 0).run(() -> {
-                try (var scope = new StructuredTaskScope<Object>()) {
+                try (var scope = new StructuredTaskScope<>()) {
                     try {
-                        if (tlr.nextBoolean()) {
+                        if (ThreadLocalRandom.current().nextBoolean()) {
                             // Repeatedly test Scoped Values set by ScopedValue::call(), get(), and run()
                             final var deepRecursion
-                                = new DeepRecursion(DeepRecursion.Behaviour.choose(tlr));
+                                = new DeepRecursion(DeepRecursion.Behaviour.choose(ThreadLocalRandom.current()));
                             deepRecursion.run();
                         } else {
                             // Recursively run ourself until we get a stack overflow
@@ -204,21 +212,39 @@ public class StressStackOverflow {
                     } catch (StructureViolationException structureViolationException) {
                         // Can happen if a stack overflow prevented a StackableScope from
                         // being removed. We can continue.
+                    } catch (TestFailureException e) {
+                        throw e;
                     } catch (Exception e) {
                         throw new RuntimeException(e);
                     }
                 }
             });
-        } catch (StructureViolationException structureViolationException) {
+        } catch (TestFailureException e) {
+            throw e;
+        } catch (Exception e) {
             // Can happen if a stack overflow prevented a StackableScope from
             // being removed. We can continue.
         }
     }
 
+    static long startTime = System.nanoTime();
+
     public static void main(String[] args) {
         var torture = new StressStackOverflow();
-        while (torture.ITERS > 0) {
+        while (torture.ITERS > 0
-            torture.run();
+                && System.nanoTime() - startTime <= 3 * MINUTES) { // 3 minutes is long enough
+            try {
+                torture.run();
+                if (inheritedValue.isBound()) {
+                    throw new TestFailureException("Should not be bound here");
+                }
+            } catch (TestFailureException e) {
+                throw e;
+            } catch (Exception e) {
+                // ScopedValueContainer and StructuredTaskScope can
+                // throw many exceptions on stack overflow. Ignore
+                // them all.
+            }
         }
         System.out.println("OK");
     }