Skip to content

Commit

Permalink
8308609: java/lang/ScopedValue/StressStackOverflow.java fails with "-…
Browse files Browse the repository at this point in the history
…XX:-VMContinuations"

Reviewed-by: alanb
  • Loading branch information
Andrew Haley committed Jun 16, 2023
1 parent b412fc7 commit 44a8aa0
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 69 deletions.
5 changes: 2 additions & 3 deletions src/hotspot/share/prims/jvm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
|| holder == vmClasses::VirtualThread_klass()
|| holder == vmClasses::Thread_klass())) {
if (holder == vmClasses::Thread_klass()
|| holder == resolver.Carrier_klass) {
loc = 1;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/java.base/share/classes/java/lang/Thread.java
Original file line number Diff line number Diff line change
Expand Up @@ -1578,7 +1578,7 @@ public void run() {
*/
@Hidden
@ForceInline
private void runWith(Object bindings, Runnable op) {
final void runWith(Object bindings, Runnable op) {
ensureMaterializedForStackWalk(bindings);
op.run();
Reference.reachabilityFence(bindings);
Expand Down
3 changes: 2 additions & 1 deletion src/java.base/share/classes/java/lang/ThreadBuilders.java
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,8 @@ public void run() {
// 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);
}
}

Expand Down
12 changes: 1 addition & 11 deletions src/java.base/share/classes/java/lang/VirtualThread.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
*/
package java.lang;

import java.lang.ref.Reference;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Locale;
Expand Down Expand Up @@ -54,7 +53,6 @@
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;
Expand Down Expand Up @@ -306,7 +304,7 @@ private void run(Runnable task) {
event.commit();
}

Object bindings = scopedValueBindings();
Object bindings = Thread.scopedValueBindings();
try {
runWith(bindings, task);
} catch (Throwable exc) {
Expand Down Expand Up @@ -334,14 +332,6 @@ private void run(Runnable task) {
}
}

@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.
Expand Down
2 changes: 0 additions & 2 deletions test/jdk/ProblemList-Virtual.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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

##########
Expand Down
2 changes: 0 additions & 2 deletions test/jdk/ProblemList.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
124 changes: 75 additions & 49 deletions test/jdk/java/lang/ScopedValue/StressStackOverflow.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,23 @@
* questions.
*/

/**
* @test
/*
* @test id=default
* @summary StressStackOverflow the recovery path for ScopedValue
* @enablePreview
* @run main/othervm/timeout=300 -XX:-TieredCompilation StressStackOverflow
* @run main/othervm/timeout=300 -XX:TieredStopAtLevel=1 StressStackOverflow
* @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;
Expand All @@ -42,21 +48,22 @@ 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;

static class TestFailureException extends RuntimeException {
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)];
}
Expand All @@ -70,38 +77,40 @@ public DeepRecursion(Behaviour behaviour) {

public void run() {
final var last = el.get();
ITERS--;
var nextRandomFloat = tlr.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));
while (ITERS-- > 0) {
if (System.nanoTime() - startTime > 3 * MINUTES) { // 3 minutes is long enough
return;
}
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;

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();
Thread.yield();
}
}

public Object get() {
Expand All @@ -114,13 +123,10 @@ public Object call() {
}
}

static final Runnable nop = new Runnable() {
public void run() { }
};
static final Runnable nop = () -> {};

// Consume some stack.
//

// The double recursion used here prevents an optimizing JIT from
// inlining all the recursive calls, which would make it
// ineffective.
Expand All @@ -137,7 +143,7 @@ private long fibonacci_pad1(int n, Runnable op) {
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;
Expand All @@ -152,14 +158,16 @@ long fibonacci_pad(int n, Runnable op) {
// 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();
try (var scope = new StructuredTaskScope<Object>("", threadFactory)) {
= (ThreadLocalRandom.current().nextBoolean() ? Thread.ofPlatform() : Thread.ofVirtual()).factory();
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);
}
Expand All @@ -168,12 +176,12 @@ void runInNewThread(Runnable op) {
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
Expand Down Expand Up @@ -204,21 +212,39 @@ public void run() {
} 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) {
torture.run();
while (torture.ITERS > 0
&& 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");
}
Expand Down

3 comments on commit 44a8aa0

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RealCLanger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk21u

@openjdk
Copy link

@openjdk openjdk bot commented on 44a8aa0 Jul 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RealCLanger the backport was successfully created on the branch RealCLanger-backport-44a8aa06 in my personal fork of openjdk/jdk21u. To create a pull request with this backport targeting openjdk/jdk21u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 44a8aa06 from the openjdk/jdk repository.

The commit being backported was authored by Andrew Haley on 16 Jun 2023 and was reviewed by Alan Bateman.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21u:

$ git fetch https://github.com/openjdk-bots/jdk21u.git RealCLanger-backport-44a8aa06:RealCLanger-backport-44a8aa06
$ git checkout RealCLanger-backport-44a8aa06
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk21u.git RealCLanger-backport-44a8aa06

Please sign in to comment.