Skip to content

[GR-43125] Verify that recurring callbacks don't allocate Java heap memory. #7787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 9, 2023
Merged

[GR-43125] Verify that recurring callbacks don't allocate Java heap memory. #7787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 0 additions & 11 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
*/
package com.oracle.svm.core.jfr;

import java.lang.reflect.Field;
import java.util.List;

import jdk.graal.compiler.api.replacements.Fold;
Expand Down Expand Up @@ -186,16 +185,6 @@ public static JfrLogging getJfrLogging() {
return get().jfrLogging;
}

public static Object getHandler(Class<? extends jdk.internal.event.Event> eventClass) {
try {
Field f = eventClass.getDeclaredField("eventHandler");
f.setAccessible(true);
return f.get(null);
} catch (NoSuchFieldException | IllegalArgumentException | IllegalAccessException e) {
throw new InternalError("Could not access event handler");
}
}

@Uninterruptible(reason = "Prevent races with VM operations that start/stop recording.", callerMustBe = true)
protected boolean isRecording() {
return recording;
Expand Down
75 changes: 31 additions & 44 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/Safepoint.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,6 @@

import java.util.concurrent.atomic.AtomicInteger;

import jdk.graal.compiler.api.replacements.Fold;
import jdk.graal.compiler.core.common.spi.ForeignCallDescriptor;
import jdk.graal.compiler.graph.Node.ConstantNodeParameter;
import jdk.graal.compiler.graph.Node.NodeIntrinsic;
import jdk.graal.compiler.nodes.PauseNode;
import jdk.graal.compiler.nodes.extended.BranchProbabilityNode;
import jdk.graal.compiler.nodes.extended.ForeignCallNode;
import jdk.graal.compiler.nodes.extended.MembarNode;
import jdk.graal.compiler.options.Option;
import org.graalvm.nativeimage.CurrentIsolate;
import org.graalvm.nativeimage.ImageSingletons;
import org.graalvm.nativeimage.IsolateThread;
Expand Down Expand Up @@ -80,6 +71,16 @@
import com.oracle.svm.core.util.TimeUtils;
import com.oracle.svm.core.util.VMError;

import jdk.graal.compiler.api.replacements.Fold;
import jdk.graal.compiler.core.common.spi.ForeignCallDescriptor;
import jdk.graal.compiler.graph.Node.ConstantNodeParameter;
import jdk.graal.compiler.graph.Node.NodeIntrinsic;
import jdk.graal.compiler.nodes.PauseNode;
import jdk.graal.compiler.nodes.extended.BranchProbabilityNode;
import jdk.graal.compiler.nodes.extended.ForeignCallNode;
import jdk.graal.compiler.nodes.extended.MembarNode;
import jdk.graal.compiler.options.Option;

/**
* Support for initiating safepoints, which are a global state in which all threads are paused so
* that an invasive operation (such as garbage collection) can execute without interferences.
Expand Down Expand Up @@ -178,16 +179,23 @@ private static long getSafepointPromptnessFailureNanos() {
return Options.SafepointPromptnessFailureNanos.getValue().longValue();
}

/**
* Used to wrap exceptions that are explicitly thrown by recurring callbacks.
*/
static class SafepointException extends RuntimeException {
private static final long serialVersionUID = 1L;

final Throwable inner;

SafepointException(Throwable inner) {
this.inner = inner;
@Uninterruptible(reason = "Must not contain safepoint checks.")
private static void slowPathSafepointCheck(int newStatus, boolean callerHasJavaFrameAnchor, boolean popFrameAnchor) throws Throwable {
try {
slowPathSafepointCheck0(newStatus, callerHasJavaFrameAnchor, popFrameAnchor);
} catch (ThreadingSupportImpl.SafepointException e) {
/* This exception is intended to be thrown from safepoint checks, at one's own risk */
throw ThreadingSupportImpl.RecurringCallbackTimer.getAndClearPendingException();
} catch (Throwable ex) {
/*
* The foreign call from snippets to this method does not have an exception edge. So we
* could miss an exception handler if we unwind an exception from this method.
*
* Any exception coming out of a safepoint would be surprising to users. There is a good
* reason why Thread.stop() has been deprecated a long time ago (we do not support it on
* Substrate VM).
*/
VMError.shouldNotReachHere(ex);
}
}

Expand All @@ -200,7 +208,7 @@ static class SafepointException extends RuntimeException {
* </ul>
**/
@Uninterruptible(reason = "Must not contain safepoint checks.")
private static void slowPathSafepointCheck(int newStatus, boolean callerHasJavaFrameAnchor, boolean popFrameAnchor) {
private static void slowPathSafepointCheck0(int newStatus, boolean callerHasJavaFrameAnchor, boolean popFrameAnchor) {
final IsolateThread myself = CurrentIsolate.getCurrentThread();

SafepointListenerSupport.singleton().beforeSlowpathSafepointCheck();
Expand Down Expand Up @@ -410,30 +418,9 @@ public static void slowPathSafepointCheck() throws Throwable {
Safepoint.setSafepointRequested(THREAD_REQUEST_RESET);
return;
}
VMError.guarantee(StatusSupport.isStatusJava(), "Attempting to do a safepoint check when not in Java mode");

try {
/*
* Block on mutex held by thread that requested safepoint, i.e., transition to native
* code.
*/
slowPathSafepointCheck(StatusSupport.STATUS_IN_JAVA, false, false);

} catch (SafepointException se) {
/* This exception is intended to be thrown from safepoint checks, at one's own risk */
throw se.inner;

} catch (Throwable ex) {
/*
* The foreign call from snippets to this method does not have an exception edge. So we
* could miss an exception handler if we unwind an exception from this method.
*
* Any exception coming out of a safepoint would be surprising to users. There is a good
* reason why Thread.stop() has been deprecated a long time ago (we do not support it on
* Substrate VM).
*/
VMError.shouldNotReachHere(ex);
}
VMError.guarantee(StatusSupport.isStatusJava(), "Attempting to do a safepoint check when not in Java mode");
slowPathSafepointCheck(StatusSupport.STATUS_IN_JAVA, false, false);
}

/**
Expand Down Expand Up @@ -528,7 +515,7 @@ public static void transitionVMToNative() {
*/
@SubstrateForeignCallTarget(stubCallingConvention = false)
@Uninterruptible(reason = "Must not contain safepoint checks")
private static void enterSlowPathTransitionFromNativeToNewStatus(int newStatus, boolean popFrameAnchor) {
private static void enterSlowPathTransitionFromNativeToNewStatus(int newStatus, boolean popFrameAnchor) throws Throwable {
VMError.guarantee(StatusSupport.isStatusNativeOrSafepoint(), "Must either be at a safepoint or in native mode");
VMError.guarantee(!SafepointBehavior.ignoresSafepoints(),
"The safepoint handling doesn't change the status of threads that ignore safepoints. So, the fast path transition must succeed and this slow path must not be called");
Expand Down
91 changes: 66 additions & 25 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/ThreadingSupportImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@
package com.oracle.svm.core.thread;

import static com.oracle.svm.core.SubstrateOptions.MultiThreaded;
import static com.oracle.svm.core.heap.RestrictHeapAccess.Access.NO_ALLOCATION;
import static com.oracle.svm.core.thread.ThreadingSupportImpl.Options.SupportRecurringCallback;

import java.io.Serial;
import java.util.concurrent.TimeUnit;

import jdk.graal.compiler.api.replacements.Fold;
import jdk.graal.compiler.options.Option;
import org.graalvm.nativeimage.CurrentIsolate;
import org.graalvm.nativeimage.IsolateThread;
import org.graalvm.nativeimage.Threading.RecurringCallback;
Expand All @@ -42,14 +42,16 @@
import com.oracle.svm.core.heap.RestrictHeapAccess;
import com.oracle.svm.core.option.HostedOptionKey;
import com.oracle.svm.core.option.SubstrateOptionsParser;
import com.oracle.svm.core.thread.Safepoint.SafepointException;
import com.oracle.svm.core.thread.VMThreads.ActionOnTransitionToJavaSupport;
import com.oracle.svm.core.thread.VMThreads.StatusSupport;
import com.oracle.svm.core.threadlocal.FastThreadLocalFactory;
import com.oracle.svm.core.threadlocal.FastThreadLocalInt;
import com.oracle.svm.core.threadlocal.FastThreadLocalObject;
import com.oracle.svm.core.util.VMError;

import jdk.graal.compiler.api.replacements.Fold;
import jdk.graal.compiler.options.Option;

@AutomaticallyRegisteredImageSingleton(ThreadingSupport.class)
public class ThreadingSupportImpl implements ThreadingSupport {
public static class Options {
Expand All @@ -69,12 +71,8 @@ public static class Options {
* adapt to a changing frequency of safepoint checks in the code that the thread executes.
*/
public static class RecurringCallbackTimer {
private static final RecurringCallbackAccess CALLBACK_ACCESS = new RecurringCallbackAccess() {
@Override
public void throwException(Throwable t) {
throw new SafepointException(t);
}
};
private static final FastThreadLocalObject<Throwable> EXCEPTION_TL = FastThreadLocalFactory.createObject(Throwable.class, "RecurringCallbackTimer.exception");
private static final RecurringCallbackAccess CALLBACK_ACCESS = new RecurringCallbackAccessImpl();

/**
* Weight of the newest sample in {@link #ewmaChecksPerNano}. Older samples have a total
Expand Down Expand Up @@ -107,6 +105,14 @@ public void throwException(Throwable t) {
this.requestedChecks = INITIAL_CHECKS;
}

@Uninterruptible(reason = "Prevent recurring callback execution.", callerMustBe = true)
public static Throwable getAndClearPendingException() {
Throwable t = EXCEPTION_TL.get();
VMError.guarantee(t != null, "There must be a recurring callback exception pending.");
EXCEPTION_TL.set(null);
return t;
}

@Uninterruptible(reason = "Must not contain safepoint checks.")
void evaluate() {
updateStatistics();
Expand Down Expand Up @@ -208,24 +214,42 @@ private boolean isCallbackDisabled() {
}

/**
* Separate method to invoke {@link #callback} so that {@link #evaluate()} can be strictly
* {@link Uninterruptible} and allocation-free.
* Recurring callbacks may be executed in any method that contains a safepoint check. This
* includes methods that need to be allocation free. Therefore, recurring callbacks must not
* allocate any Java heap memory.
*/
@Uninterruptible(reason = "Required by caller, but does not apply to callee.", calleeMustBe = false)
@RestrictHeapAccess(reason = "Callee may allocate", access = RestrictHeapAccess.Access.UNRESTRICTED)
@RestrictHeapAccess(reason = "Recurring callbacks must not allocate.", access = NO_ALLOCATION)
private void invokeCallback() {
try {
callback.run(CALLBACK_ACCESS);
} catch (SafepointException se) {
throw se;
} catch (SafepointException e) {
throw e;
} catch (Throwable t) {
/*
* Recurring callbacks are specified to ignore all exceptions. We cannot even log
* the exception because that could lead to a StackOverflowError (especially when
* the recurring callback failed with a StackOverflowError).
* Recurring callbacks are specified to ignore exceptions (except if the exception
* is thrown via RecurringCallbackAccess.throwException(), which is handled above).
* We cannot even log the exception because that could lead to a StackOverflowError
* (especially when the recurring callback failed with a StackOverflowError).
*/
}
}

/**
* We need to distinguish between arbitrary exceptions (must be swallowed) and exceptions
* that are thrown via {@link RecurringCallbackAccess#throwException} (must be forwarded to
* the application). When a recurring callback uses
* {@link RecurringCallbackAccess#throwException}, we store the exception in a thread local
* (to avoid allocations) and throw a pre-allocated marker exception instead. We catch the
* marker exception internally, accesses the thread local, and rethrow that exception.
*/
private static final class RecurringCallbackAccessImpl implements RecurringCallbackAccess {
@Override
public void throwException(Throwable t) {
EXCEPTION_TL.set(t);
throw SafepointException.SINGLETON;
}
}
}

private static final FastThreadLocalObject<RecurringCallbackTimer> activeTimer = FastThreadLocalFactory.createObject(RecurringCallbackTimer.class, "ThreadingSupportImpl.activeTimer");
Expand Down Expand Up @@ -332,9 +356,9 @@ static boolean needsNativeToJavaSlowpath() {
}

/**
* Recurring callbacks execute arbitrary code and can throw {@link SafepointException}s. In some
* code parts (e.g., when executing VM operations), we can't deal with arbitrary code execution
* and therefore need to pause the execution of recurring callbacks.
* Recurring callbacks execute arbitrary code and may throw exceptions. In some code parts
* (e.g., when executing VM operations), we can't deal with arbitrary code execution and
* therefore need to pause the execution of recurring callbacks.
*/
@Uninterruptible(reason = "Must not contain safepoint checks.")
public static void pauseRecurringCallback(@SuppressWarnings("unused") String reason) {
Expand Down Expand Up @@ -370,11 +394,17 @@ public static void resumeRecurringCallbackAtNextSafepoint() {
*/
public static void resumeRecurringCallback() {
if (resumeCallbackExecution()) {
try {
onSafepointCheckSlowpath();
} catch (SafepointException e) {
throwUnchecked(e.inner); // needed: callers cannot declare `throws Throwable`
}
maybeExecuteRecurringCallback();
}
}

@Uninterruptible(reason = "Prevent safepoints.")
private static void maybeExecuteRecurringCallback() {
try {
onSafepointCheckSlowpath();
} catch (SafepointException e) {
/* Callers cannot declare `throws Throwable`. */
throwUnchecked(RecurringCallbackTimer.getAndClearPendingException());
}
}

Expand Down Expand Up @@ -407,7 +437,18 @@ public static boolean isRecurringCallbackSupported() {
}

@SuppressWarnings("unchecked")
@Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true)
private static <T extends Throwable> void throwUnchecked(Throwable exception) throws T {
throw (T) exception; // T is inferred as RuntimeException, but doesn't have to be
}

static final class SafepointException extends RuntimeException {
public static final SafepointException SINGLETON = new SafepointException();

@Serial //
private static final long serialVersionUID = 1L;

private SafepointException() {
}
}
}
23 changes: 10 additions & 13 deletions .../org.graalvm.polyglot.nativeapi/src/org/graalvm/polyglot/nativeapi/PolyglotNativeAPI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2403,20 +2403,17 @@ private static PolyglotStatus doRegisterRecurringCallback0(long intervalNanos, P
PolyglotCallbackInfoInternal info = new PolyglotCallbackInfoInternal(new ObjectHandle[0], data);
var infoHandle = (PolyglotCallbackInfo) objectHandles.create(info);
var handles = getHandles();
RecurringCallback recurringCallback = new RecurringCallback() {
@Override
public void run(Threading.RecurringCallbackAccess access) {
handles.freezeHandleCreation();
try {
callback.invoke((PolyglotIsolateThread) CurrentIsolate.getCurrentThread(), infoHandle);
CallbackException ce = threadLocals.get().callbackException;
if (ce != null) {
access.throwException(ce);
}
} finally {
threadLocals.get().callbackException = null;
handles.unfreezeHandleCreation();
RecurringCallback recurringCallback = access -> {
handles.freezeHandleCreation();
try {
callback.invoke((PolyglotIsolateThread) CurrentIsolate.getCurrentThread(), infoHandle);
CallbackException ce = threadLocals.get().callbackException;
if (ce != null) {
access.throwException(ce);
}
} finally {
threadLocals.get().callbackException = null;
handles.unfreezeHandleCreation();
}
};
try {
Expand Down