From 291172a8190b29cdf1775c42e376bf2235ac89d3 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Mon, 16 Oct 2023 13:38:04 +0200 Subject: [PATCH] Verify that recurring callbacks are allocation free. --- .../com/oracle/svm/core/jfr/SubstrateJVM.java | 11 --- .../com/oracle/svm/core/thread/Safepoint.java | 75 +++++++-------- .../svm/core/thread/ThreadingSupportImpl.java | 91 ++++++++++++++----- .../polyglot/nativeapi/PolyglotNativeAPI.java | 23 ++--- 4 files changed, 107 insertions(+), 93 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java index d09667b187fe..49cbe53fa308 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jfr/SubstrateJVM.java @@ -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; @@ -186,16 +185,6 @@ public static JfrLogging getJfrLogging() { return get().jfrLogging; } - public static Object getHandler(Class 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; diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/Safepoint.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/Safepoint.java index 154ede835acb..ca8a8b2bdbe2 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/Safepoint.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/Safepoint.java @@ -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; @@ -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. @@ -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); } } @@ -200,7 +208,7 @@ static class SafepointException extends RuntimeException { * **/ @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(); @@ -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); } /** @@ -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"); diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/ThreadingSupportImpl.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/ThreadingSupportImpl.java index f85267b4e573..77d1ec1ffe7d 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/ThreadingSupportImpl.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/thread/ThreadingSupportImpl.java @@ -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; @@ -42,7 +42,6 @@ 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; @@ -50,6 +49,9 @@ 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 { @@ -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 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 @@ -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(); @@ -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 activeTimer = FastThreadLocalFactory.createObject(RecurringCallbackTimer.class, "ThreadingSupportImpl.activeTimer"); @@ -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) { @@ -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()); } } @@ -407,7 +437,18 @@ public static boolean isRecurringCallbackSupported() { } @SuppressWarnings("unchecked") + @Uninterruptible(reason = "Called from uninterruptible code.", mayBeInlined = true) private static 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() { + } + } } diff --git a/substratevm/src/org.graalvm.polyglot.nativeapi/src/org/graalvm/polyglot/nativeapi/PolyglotNativeAPI.java b/substratevm/src/org.graalvm.polyglot.nativeapi/src/org/graalvm/polyglot/nativeapi/PolyglotNativeAPI.java index e67db6e0463f..4e96c57d976a 100644 --- a/substratevm/src/org.graalvm.polyglot.nativeapi/src/org/graalvm/polyglot/nativeapi/PolyglotNativeAPI.java +++ b/substratevm/src/org.graalvm.polyglot.nativeapi/src/org/graalvm/polyglot/nativeapi/PolyglotNativeAPI.java @@ -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 {