From af285a0890e084da6746d6db7561b56144959b17 Mon Sep 17 00:00:00 2001 From: Tom Shull Date: Wed, 16 Jun 2021 11:46:38 +0200 Subject: [PATCH 01/18] Fix EnsureClassInitializedNode state. --- .../EnsureClassInitializedNode.java | 11 ++++++++--- .../svm/hosted/code/CompilationInfoSupport.java | 7 +++++-- .../phases/SubstrateClassInitializationPlugin.java | 7 ++++--- .../oracle/svm/hosted/snippets/ReflectionPlugins.java | 2 +- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/classinitialization/EnsureClassInitializedNode.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/classinitialization/EnsureClassInitializedNode.java index 4bb61d14aa59..fb13149b2713 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/classinitialization/EnsureClassInitializedNode.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/classinitialization/EnsureClassInitializedNode.java @@ -27,8 +27,6 @@ import org.graalvm.compiler.core.common.type.StampFactory; import org.graalvm.compiler.graph.Node.NodeIntrinsicFactory; import org.graalvm.compiler.graph.NodeClass; -import org.graalvm.compiler.nodes.spi.Simplifiable; -import org.graalvm.compiler.nodes.spi.SimplifierTool; import org.graalvm.compiler.nodeinfo.InputType; import org.graalvm.compiler.nodeinfo.NodeCycles; import org.graalvm.compiler.nodeinfo.NodeInfo; @@ -40,6 +38,8 @@ import org.graalvm.compiler.nodes.graphbuilderconf.GraphBuilderContext; import org.graalvm.compiler.nodes.memory.SingleMemoryKill; import org.graalvm.compiler.nodes.spi.Lowerable; +import org.graalvm.compiler.nodes.spi.Simplifiable; +import org.graalvm.compiler.nodes.spi.SimplifierTool; import org.graalvm.compiler.nodes.type.StampTool; import org.graalvm.word.LocationIdentity; @@ -59,10 +59,15 @@ public static boolean intrinsify(GraphBuilderContext b, ValueNode hub) { return true; } - public EnsureClassInitializedNode(ValueNode hub) { + public EnsureClassInitializedNode(ValueNode hub, FrameState stateAfter) { super(TYPE, StampFactory.forVoid()); this.hub = hub; assert StampTool.isPointerNonNull(hub) : "Hub must already be null-checked"; + this.stateAfter = stateAfter; + } + + public EnsureClassInitializedNode(ValueNode hub) { + this(hub, null); } public ValueNode getHub() { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompilationInfoSupport.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompilationInfoSupport.java index ac19b48e4a70..81ab4a691f29 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompilationInfoSupport.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompilationInfoSupport.java @@ -31,6 +31,7 @@ import java.util.Set; import java.util.stream.Collectors; +import org.graalvm.compiler.nodeinfo.Verbosity; import org.graalvm.compiler.nodes.FrameState; import org.graalvm.compiler.nodes.ValueNode; import org.graalvm.nativeimage.ImageSingletons; @@ -95,10 +96,12 @@ public DeoptSourceFrameInfo mergeStateInfo(FrameState state) { expectedKinds.size() == otherKinds.size(); if (!matchingSizes) { StringBuilder errorMessage = new StringBuilder(); - errorMessage.append("Unexpected number of values in state to merge.\n"); + errorMessage.append("Unexpected number of values in state to merge. Please report this problem.\n"); + errorMessage.append(String.format("****Merge FrameState****\n%s************************\n", state.toString(Verbosity.Debugger))); + errorMessage.append(String.format("bci: %d, duringCall: %b, rethrowException: %b\n", state.bci, state.duringCall(), state.rethrowException())); errorMessage.append(String.format("DeoptSourceFrameInfo: locals-%d, stack-%d, locks-%d.\n", numLocals, numStack, numLocks)); errorMessage.append(String.format("Merge FrameState: locals-%d, stack-%d, locks-%d.\n", state.localsSize(), state.stackSize(), state.locksSize())); - VMError.shouldNotReachHere(errorMessage.toString()); + throw VMError.shouldNotReachHere(errorMessage.toString()); } for (int i = 0; i < expectedKinds.size(); i++) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SubstrateClassInitializationPlugin.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SubstrateClassInitializationPlugin.java index 34213d398a5c..60a0359d0340 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SubstrateClassInitializationPlugin.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/phases/SubstrateClassInitializationPlugin.java @@ -61,7 +61,7 @@ public void loadReferencedType(GraphBuilderContext builder, ConstantPool constan @Override public boolean apply(GraphBuilderContext builder, ResolvedJavaType type, Supplier frameState, ValueNode[] classInit) { if (needsRuntimeInitialization(builder.getMethod().getDeclaringClass(), type)) { - emitEnsureClassInitialized(builder, SubstrateObjectConstant.forObject(host.dynamicHub(type))); + emitEnsureClassInitialized(builder, SubstrateObjectConstant.forObject(host.dynamicHub(type)), frameState.get()); /* * The classInit value is only registered with Invoke nodes. Since we do not need that, * we ensure it is null. @@ -74,9 +74,10 @@ public boolean apply(GraphBuilderContext builder, ResolvedJavaType type, Supplie return false; } - public static void emitEnsureClassInitialized(GraphBuilderContext builder, JavaConstant hubConstant) { + private static void emitEnsureClassInitialized(GraphBuilderContext builder, JavaConstant hubConstant, FrameState frameState) { ValueNode hub = ConstantNode.forConstant(hubConstant, builder.getMetaAccess(), builder.getGraph()); - builder.add(new EnsureClassInitializedNode(hub)); + EnsureClassInitializedNode node = new EnsureClassInitializedNode(hub, frameState); + builder.add(node); } /** diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java index b8ac30b0355b..d84ff23e6525 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/snippets/ReflectionPlugins.java @@ -293,7 +293,7 @@ private boolean processClassForName(GraphBuilderContext b, ResolvedJavaMethod ta } if (initialize) { - classInitializationPlugin.apply(b, b.getMetaAccess().lookupJavaType(clazz), null, null); + classInitializationPlugin.apply(b, b.getMetaAccess().lookupJavaType(clazz), () -> null, null); } return true; } From 1b5c48c618f74d76c939abec30aae75bdc5dc28b Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Thu, 17 Jun 2021 22:17:57 +0200 Subject: [PATCH 02/18] Update truffleruby import --- vm/mx.vm/suite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vm/mx.vm/suite.py b/vm/mx.vm/suite.py index 5f07a9f1e6f0..8b8d21dbefe0 100644 --- a/vm/mx.vm/suite.py +++ b/vm/mx.vm/suite.py @@ -57,7 +57,7 @@ }, { "name": "truffleruby", - "version": "139963a0c2c49d2669275dd19d48a0c1ac45b660", + "version": "970501c92a3e18deff2597e10061e0184ec6236b", "dynamic": True, "urls": [ {"url": "https://github.com/oracle/truffleruby.git", "kind": "git"}, From dc52498789b3014441b8004864b81b1ae01b22c5 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Fri, 18 Jun 2021 13:36:56 +0200 Subject: [PATCH 03/18] [GR-32150] Require at least macOS Sierra (10.12) * OS X El Capitan (10.11) is no longer supported by GraalVM. --- espresso/ci_common/common.jsonnet | 4 ++-- sulong/mx.sulong/suite.py | 6 +++--- vm/ci_common/common.hocon | 3 ++- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/espresso/ci_common/common.jsonnet b/espresso/ci_common/common.jsonnet index bcecaadeef95..84eaae390228 100644 --- a/espresso/ci_common/common.jsonnet +++ b/espresso/ci_common/common.jsonnet @@ -50,8 +50,8 @@ local benchmark_suites = ['dacapo', 'renaissance', 'scala-dacapo']; darwin: self.common + { environment+: { - // for compatibility with macOS El Capitan - MACOSX_DEPLOYMENT_TARGET: '10.11', + // for compatibility with macOS Sierra + MACOSX_DEPLOYMENT_TARGET: '10.12', }, capabilities: ['darwin', 'amd64'], }, diff --git a/sulong/mx.sulong/suite.py b/sulong/mx.sulong/suite.py index a1dcbb7feaf4..8c19e12acd8d 100644 --- a/sulong/mx.sulong/suite.py +++ b/sulong/mx.sulong/suite.py @@ -654,7 +654,7 @@ "SULONG_BOOTSTRAP_TOOLCHAIN_NO_HOME", ], "cmakeConfig" : { - "CMAKE_OSX_DEPLOYMENT_TARGET" : "10.11", + "CMAKE_OSX_DEPLOYMENT_TARGET" : "10.12", "CMAKE_C_COMPILER" : "/bin/", "CMAKE_CXX_COMPILER" : "/bin/", "GRAALVM_LLVM_INCLUDE_DIR" : "/include", @@ -705,7 +705,7 @@ "com.oracle.truffle.llvm.libraries.graalvm.llvm", ], "cmakeConfig" : { - "CMAKE_OSX_DEPLOYMENT_TARGET" : "10.11", + "CMAKE_OSX_DEPLOYMENT_TARGET" : "10.12", "CMAKE_C_COMPILER" : "/bin/", "GRAALVM_LLVM_INCLUDE_DIR" : "/include", "LLVM_LINK" : "/bin/", @@ -730,7 +730,7 @@ "sdk:LLVM_TOOLCHAIN", ], "cmakeConfig" : { - "CMAKE_OSX_DEPLOYMENT_TARGET" : "10.11", + "CMAKE_OSX_DEPLOYMENT_TARGET" : "10.12", "CMAKE_C_COMPILER" : "/bin/", "TRUFFLE_NFI_NATIVE_INCLUDE" : "/include", "CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS" : "YES", diff --git a/vm/ci_common/common.hocon b/vm/ci_common/common.hocon index 800c44b49692..1723326ed44b 100644 --- a/vm/ci_common/common.hocon +++ b/vm/ci_common/common.hocon @@ -17,7 +17,8 @@ common_vm_darwin: ${common_vm} ${darwin} { } environment: { LANG: en_US.UTF-8 - MACOSX_DEPLOYMENT_TARGET: "10.11" + # for compatibility with macOS Sierra + MACOSX_DEPLOYMENT_TARGET: "10.12" } setup: ${common_vm.setup} } From 68827119c19e0b8e82d9800bb0ba9ca543a8437c Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Tue, 15 Jun 2021 10:01:33 -0700 Subject: [PATCH 04/18] Improve performance of graph verification --- .../src/org/graalvm/compiler/graph/Graph.java | 12 +++++++++++- .../src/org/graalvm/compiler/graph/Node.java | 5 ++--- .../org/graalvm/compiler/nodes/util/GraphUtil.java | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/compiler/src/org.graalvm.compiler.graph/src/org/graalvm/compiler/graph/Graph.java b/compiler/src/org.graalvm.compiler.graph/src/org/graalvm/compiler/graph/Graph.java index 0030fe9f9f93..41d8465a6757 100644 --- a/compiler/src/org.graalvm.compiler.graph/src/org/graalvm/compiler/graph/Graph.java +++ b/compiler/src/org.graalvm.compiler.graph/src/org/graalvm/compiler/graph/Graph.java @@ -75,6 +75,13 @@ private enum FreezeState { public final String name; + /** + * Cached actual value of the {@link Options} to avoid expensive map lookup for every time a + * node / graph is verified. + */ + public final boolean verifyGraphs; + public final boolean verifyGraphEdges; + /** * The set of nodes in the graph, ordered by {@linkplain #register(Node) registration} time. */ @@ -287,6 +294,9 @@ public Graph(String name, OptionValues options, DebugContext debug, boolean trac nodeModCounts = new int[INITIAL_NODES_SIZE]; nodeUsageModCounts = new int[INITIAL_NODES_SIZE]; } + + verifyGraphs = Options.VerifyGraalGraphs.getValue(options); + verifyGraphEdges = Options.VerifyGraalGraphEdges.getValue(options); } int extractOriginalNodeId(Node node) { @@ -1166,7 +1176,7 @@ void unregister(Node node) { } public boolean verify() { - if (Options.VerifyGraalGraphs.getValue(options)) { + if (verifyGraphs) { for (Node node : getNodes()) { try { try { diff --git a/compiler/src/org.graalvm.compiler.graph/src/org/graalvm/compiler/graph/Node.java b/compiler/src/org.graalvm.compiler.graph/src/org/graalvm/compiler/graph/Node.java index 784a459522ed..b1e358b7a060 100644 --- a/compiler/src/org.graalvm.compiler.graph/src/org/graalvm/compiler/graph/Node.java +++ b/compiler/src/org.graalvm.compiler.graph/src/org/graalvm/compiler/graph/Node.java @@ -49,7 +49,6 @@ import org.graalvm.compiler.debug.DebugCloseable; import org.graalvm.compiler.debug.DebugContext; import org.graalvm.compiler.graph.Graph.NodeEventListener; -import org.graalvm.compiler.graph.Graph.Options; import org.graalvm.compiler.graph.iterators.NodeIterable; import org.graalvm.compiler.graph.iterators.NodePredicate; import org.graalvm.compiler.nodeinfo.InputType; @@ -1154,10 +1153,10 @@ protected boolean verifyInputs() { } public boolean verify() { - assertTrue(isAlive(), "cannot verify inactive nodes (id=%d)", id); + assertTrue(isAlive(), "cannot verify inactive node %s", this); assertTrue(graph() != null, "null graph"); verifyInputs(); - if (Options.VerifyGraalGraphEdges.getValue(getOptions())) { + if (graph.verifyGraphEdges) { verifyEdges(); } return true; diff --git a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/util/GraphUtil.java b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/util/GraphUtil.java index 05e64a72abc4..5d31507dc3ff 100644 --- a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/util/GraphUtil.java +++ b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/util/GraphUtil.java @@ -271,7 +271,7 @@ public static void killCFG(FixedNode node) { EconomicSet unsafeNodes = null; Graph.NodeEventScope nodeEventScope = null; OptionValues options = node.getOptions(); - boolean verifyGraalGraphEdges = Graph.Options.VerifyGraalGraphEdges.getValue(options); + boolean verifyGraalGraphEdges = node.graph().verifyGraphEdges; boolean verifyKillCFGUnusedNodes = GraphUtil.Options.VerifyKillCFGUnusedNodes.getValue(options); if (verifyGraalGraphEdges) { unsafeNodes = collectUnsafeNodes(node.graph()); From 66ecdc585ef8da27c613f5833db22d8465df6758 Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Tue, 15 Jun 2021 10:12:38 -0700 Subject: [PATCH 05/18] Improve image build time --- .../pointsto/util/CompletionExecutor.java | 7 ++++- .../hosted/ImageSingletonsSupportImpl.java | 26 +++++++++---------- .../svm/hosted/NativeImageGenerator.java | 10 +------ .../hosted/NativeImageGeneratorRunner.java | 2 +- .../src/com/oracle/svm/hosted/SVMHost.java | 4 +-- 5 files changed, 21 insertions(+), 28 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/CompletionExecutor.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/CompletionExecutor.java index 3f2c073adb30..5db7c5e63f9c 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/CompletionExecutor.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/CompletionExecutor.java @@ -235,7 +235,12 @@ public long complete() throws InterruptedException { while (true) { assert state.get() == State.STARTED; - boolean quiescent = executorService.awaitTermination(100, TimeUnit.MILLISECONDS); + boolean quiescent; + if (executorService instanceof ForkJoinPool) { + quiescent = ((ForkJoinPool) executorService).awaitQuiescence(100, TimeUnit.MILLISECONDS); + } else { + quiescent = executorService.awaitTermination(100, TimeUnit.MILLISECONDS); + } if (timing != null && !quiescent) { long curTime = System.nanoTime(); if (curTime - lastPrint > timing.getPrintIntervalNanos()) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageSingletonsSupportImpl.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageSingletonsSupportImpl.java index 36a91c544f9a..8ea257dcb2ce 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageSingletonsSupportImpl.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ImageSingletonsSupportImpl.java @@ -64,14 +64,13 @@ public static final class HostedManagement { } /** - * Multiple image generators can run simultaneously in one hosting VM. Therefore, the - * {@link ImageSingletons} is not a singleton during image generation, i.e., we cannot store - * the {@link ImageSingletons} in a static field. Instead, we store it in a thread local - * variable. The image generator is responsible to {@link #installInThread install} the same - * configuration in all worker threads, and {@link #clearInThread clear} it when it is no - * longer needed. + * The {@link ImageSingletons} removes static state from the image generator, and in theory + * would allow multiple image builds to run at the same time in the same HotSpot VM. But in + * practice, this is not possible because JDK state would leak between image builds. So it + * is OK (and better for performance) to store all the {@link ImageSingletons} in a static + * field here. */ - private static final ThreadLocal hostedVMConfig = new ThreadLocal<>(); + private static HostedManagement singletonDuringImageBuild; public static HostedManagement getAndAssertExists() { HostedManagement result = get(); @@ -80,17 +79,16 @@ public static HostedManagement getAndAssertExists() { } public static HostedManagement get() { - return hostedVMConfig.get(); + return singletonDuringImageBuild; } - public static void installInThread(HostedManagement vmConfig) { - assert hostedVMConfig.get() == null; - hostedVMConfig.set(vmConfig); + public static void install(HostedManagement vmConfig) { + UserError.guarantee(singletonDuringImageBuild == null, "Only one native image build can run at a time"); + singletonDuringImageBuild = vmConfig; } - public static void clearInThread() { - assert hostedVMConfig.get() != null; - hostedVMConfig.set(null); + public static void clear() { + singletonDuringImageBuild = null; } private final Map, Object> configObjects; diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java index f0fc50fb414f..e3f9704d9d87 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java @@ -483,7 +483,7 @@ public void run(Map entryPoints, setSystemPropertiesForImageLate(k); - ImageSingletonsSupportImpl.HostedManagement.installInThread(new ImageSingletonsSupportImpl.HostedManagement()); + ImageSingletonsSupportImpl.HostedManagement.install(new ImageSingletonsSupportImpl.HostedManagement()); ForkJoinPool buildExecutor = executor = createForkJoinPool(compilationExecutor.getParallelism()); buildExecutor.submit(() -> { @@ -536,22 +536,14 @@ protected static void clearSystemPropertiesForImage() { } protected ForkJoinPool createForkJoinPool(int maxConcurrentThreads) { - ImageSingletonsSupportImpl.HostedManagement vmConfig = ImageSingletonsSupportImpl.HostedManagement.getAndAssertExists(); return new ForkJoinPool( maxConcurrentThreads, pool -> new ForkJoinWorkerThread(pool) { @Override protected void onStart() { super.onStart(); - ImageSingletonsSupportImpl.HostedManagement.installInThread(vmConfig); assert loader.getClassLoader().equals(getContextClassLoader()); } - - @Override - protected void onTermination(Throwable exception) { - ImageSingletonsSupportImpl.HostedManagement.clearInThread(); - super.onTermination(exception); - } }, Thread.getDefaultUncaughtExceptionHandler(), false); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGeneratorRunner.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGeneratorRunner.java index 6bbc89c115e1..ebf47a5dd526 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGeneratorRunner.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGeneratorRunner.java @@ -439,7 +439,7 @@ private int buildImage(String[] arguments, ImageClassLoader classLoader) { generator.reportBuildArtifacts(imageName); } NativeImageGenerator.clearSystemPropertiesForImage(); - ImageSingletonsSupportImpl.HostedManagement.clearInThread(); + ImageSingletonsSupportImpl.HostedManagement.clear(); } return 0; } diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java index 0fdb9a0e9ceb..122edfbf7000 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/SVMHost.java @@ -242,14 +242,12 @@ public boolean isRelocatedPointer(Object originalObject) { @Override public void clearInThread() { - Thread.currentThread().setContextClassLoader(SVMHost.class.getClassLoader()); - ImageSingletonsSupportImpl.HostedManagement.clearInThread(); } @Override public void installInThread(Object vmConfig) { Thread.currentThread().setContextClassLoader(classLoader); - ImageSingletonsSupportImpl.HostedManagement.installInThread((ImageSingletonsSupportImpl.HostedManagement) vmConfig); + assert vmConfig == ImageSingletonsSupportImpl.HostedManagement.get(); } @Override From 6541625f20c610c73dab255ac3d5e032cbd8bc9b Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Tue, 15 Jun 2021 12:02:14 -0700 Subject: [PATCH 06/18] Remove unnecessary ForkJoinPool --- .../pointsto/util/CompletionExecutor.java | 2 - .../svm/hosted/NativeImageGenerator.java | 96 ++++++------------- 2 files changed, 29 insertions(+), 69 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/CompletionExecutor.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/CompletionExecutor.java index 5db7c5e63f9c..01d9cda71f3a 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/CompletionExecutor.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/util/CompletionExecutor.java @@ -98,8 +98,6 @@ public void init() { } public void init(Timing newTiming) { - assert isSequential() || !(executorService instanceof ForkJoinPool) || !((ForkJoinPool) executorService).hasQueuedSubmissions(); - timing = newTiming; setState(State.BEFORE_START); postedOperations.reset(); diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java index e3f9704d9d87..d3703587ca16 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java @@ -49,10 +49,7 @@ import java.util.Map; import java.util.ServiceLoader; import java.util.Set; -import java.util.concurrent.CancellationException; -import java.util.concurrent.ExecutionException; import java.util.concurrent.ForkJoinPool; -import java.util.concurrent.ForkJoinWorkerThread; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Consumer; import java.util.stream.Collectors; @@ -464,59 +461,38 @@ public void run(Map entryPoints, SubstitutionProcessor harnessSubstitutions, ForkJoinPool compilationExecutor, ForkJoinPool analysisExecutor, EconomicSet allOptionNames) { - ForkJoinPool executor = null; + if (!buildStarted.compareAndSet(false, true)) { + throw UserError.abort("An image build has already been performed with this generator."); + } + try { - if (!buildStarted.compareAndSet(false, true)) { - throw UserError.abort("An image build has already been performed with this generator."); - } + /* + * JVMCI 20.2-b01 introduced new methods for linking and querying whether an interface + * has default methods. Fail early if these methods are missing. + */ + ResolvedJavaType.class.getDeclaredMethod("link"); + } catch (ReflectiveOperationException ex) { + throw UserError.abort("JVMCI version provided %s is missing the 'ResolvedJavaType.link()' method added in jvmci-20.2-b01. " + + "Please use the latest JVMCI JDK from %s or %s.", System.getProperty("java.home"), JVMCI8_RELEASES_URL, JVMCI11_RELEASES_URL); + } - try { - /* - * JVMCI 20.2-b01 introduced new methods for linking and querying whether an - * interface has default methods. Fail early if these methods are missing. - */ - ResolvedJavaType.class.getDeclaredMethod("link"); - } catch (ReflectiveOperationException ex) { - throw UserError.abort("JVMCI version provided %s is missing the 'ResolvedJavaType.link()' method added in jvmci-20.2-b01. " + - "Please use the latest JVMCI JDK from %s or %s.", System.getProperty("java.home"), JVMCI8_RELEASES_URL, JVMCI11_RELEASES_URL); - } + setSystemPropertiesForImageLate(k); - setSystemPropertiesForImageLate(k); - - ImageSingletonsSupportImpl.HostedManagement.install(new ImageSingletonsSupportImpl.HostedManagement()); - ForkJoinPool buildExecutor = executor = createForkJoinPool(compilationExecutor.getParallelism()); - - buildExecutor.submit(() -> { - ImageSingletons.add(BuildArtifacts.class, (type, artifact) -> buildArtifacts.computeIfAbsent(type, t -> new ArrayList<>()).add(artifact)); - ImageSingletons.add(ClassLoaderQuery.class, new ClassLoaderQueryImpl(loader.getClassLoader())); - ImageSingletons.add(HostedOptionValues.class, new HostedOptionValues(optionProvider.getHostedValues())); - ImageSingletons.add(RuntimeOptionValues.class, new RuntimeOptionValues(optionProvider.getRuntimeValues(), allOptionNames)); - watchdog = new DeadlockWatchdog(); - try (TemporaryBuildDirectoryProviderImpl tempDirectoryProvider = new TemporaryBuildDirectoryProviderImpl()) { - ImageSingletons.add(TemporaryBuildDirectoryProvider.class, tempDirectoryProvider); - doRun(entryPoints, javaMainSupport, imageName, k, harnessSubstitutions, compilationExecutor, analysisExecutor, buildExecutor); - } finally { - watchdog.close(); - } - }).get(); - } catch (InterruptedException | CancellationException e) { - System.out.println("Interrupted!"); - throw new InterruptImageBuilding(e); - } catch (ExecutionException e) { - rethrow(e.getCause()); + ImageSingletonsSupportImpl.HostedManagement.install(new ImageSingletonsSupportImpl.HostedManagement()); + + ImageSingletons.add(BuildArtifacts.class, (type, artifact) -> buildArtifacts.computeIfAbsent(type, t -> new ArrayList<>()).add(artifact)); + ImageSingletons.add(ClassLoaderQuery.class, new ClassLoaderQueryImpl(loader.getClassLoader())); + ImageSingletons.add(HostedOptionValues.class, new HostedOptionValues(optionProvider.getHostedValues())); + ImageSingletons.add(RuntimeOptionValues.class, new RuntimeOptionValues(optionProvider.getRuntimeValues(), allOptionNames)); + watchdog = new DeadlockWatchdog(); + try (TemporaryBuildDirectoryProviderImpl tempDirectoryProvider = new TemporaryBuildDirectoryProviderImpl()) { + ImageSingletons.add(TemporaryBuildDirectoryProvider.class, tempDirectoryProvider); + doRun(entryPoints, javaMainSupport, imageName, k, harnessSubstitutions, compilationExecutor, analysisExecutor); } finally { - if (executor != null) { - executor.shutdownNow(); - } + watchdog.close(); } } - /** A version of "sneaky throw" to relay exceptions. */ - @SuppressWarnings("unchecked") - private static void rethrow(Throwable t) throws T { - throw (T) t; - } - protected static void setSystemPropertiesForImageEarly() { System.setProperty(ImageInfo.PROPERTY_IMAGE_CODE_KEY, ImageInfo.PROPERTY_IMAGE_CODE_VALUE_BUILDTIME); } @@ -535,32 +511,18 @@ protected static void clearSystemPropertiesForImage() { System.clearProperty(ImageInfo.PROPERTY_IMAGE_KIND_KEY); } - protected ForkJoinPool createForkJoinPool(int maxConcurrentThreads) { - return new ForkJoinPool( - maxConcurrentThreads, - pool -> new ForkJoinWorkerThread(pool) { - @Override - protected void onStart() { - super.onStart(); - assert loader.getClassLoader().equals(getContextClassLoader()); - } - }, - Thread.getDefaultUncaughtExceptionHandler(), - false); - } - @SuppressWarnings("try") private void doRun(Map entryPoints, JavaMainSupport javaMainSupport, String imageName, NativeImageKind k, SubstitutionProcessor harnessSubstitutions, - ForkJoinPool compilationExecutor, ForkJoinPool analysisExecutor, ForkJoinPool buildExecutor) { + ForkJoinPool compilationExecutor, ForkJoinPool analysisExecutor) { List hostedEntryPoints = new ArrayList<>(); OptionValues options = HostedOptionValues.singleton(); SnippetReflectionProvider originalSnippetReflection = GraalAccess.getOriginalSnippetReflection(); try (DebugContext debug = new Builder(options, new GraalDebugHandlersFactory(originalSnippetReflection)).build(); DebugCloseable featureCleanup = () -> featureHandler.forEachFeature(Feature::cleanup)) { - setupNativeImage(imageName, options, entryPoints, javaMainSupport, harnessSubstitutions, analysisExecutor, buildExecutor, originalSnippetReflection, debug); + setupNativeImage(imageName, options, entryPoints, javaMainSupport, harnessSubstitutions, analysisExecutor, originalSnippetReflection, debug); boolean returnAfterAnalysis = runPointsToAnalysis(imageName, options, debug); if (returnAfterAnalysis) { @@ -852,7 +814,7 @@ private boolean runPointsToAnalysis(String imageName, OptionValues options, Debu @SuppressWarnings("try") private void setupNativeImage(String imageName, OptionValues options, Map entryPoints, JavaMainSupport javaMainSupport, SubstitutionProcessor harnessSubstitutions, - ForkJoinPool analysisExecutor, ForkJoinPool buildExecutor, SnippetReflectionProvider originalSnippetReflection, DebugContext debug) { + ForkJoinPool analysisExecutor, SnippetReflectionProvider originalSnippetReflection, DebugContext debug) { try (Indent ignored = debug.logAndIndent("setup native-image builder")) { try (StopTimer ignored1 = new Timer(imageName, "setup").start()) { SubstrateTargetDescription target = createTarget(loader.platform); @@ -892,7 +854,7 @@ private void setupNativeImage(String imageName, OptionValues options, Map Date: Thu, 17 Jun 2021 23:02:46 -0700 Subject: [PATCH 07/18] Compute assignableTypes for all types --- .../flow/AllInstantiatedTypeFlow.java | 5 ++++ .../graal/pointsto/meta/AnalysisType.java | 30 +++++++++++-------- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/AllInstantiatedTypeFlow.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/AllInstantiatedTypeFlow.java index 5bd5f38b278c..92ca5d765b06 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/AllInstantiatedTypeFlow.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/flow/AllInstantiatedTypeFlow.java @@ -26,6 +26,7 @@ import com.oracle.graal.pointsto.BigBang; import com.oracle.graal.pointsto.meta.AnalysisType; +import com.oracle.graal.pointsto.typestate.TypeState; public final class AllInstantiatedTypeFlow extends TypeFlow { @@ -33,6 +34,10 @@ public AllInstantiatedTypeFlow(AnalysisType declaredType) { super(declaredType, declaredType); } + public AllInstantiatedTypeFlow(AnalysisType declaredType, TypeState state) { + super(declaredType, declaredType, state); + } + @Override public TypeFlow copy(BigBang bb, MethodFlowsGraph methodFlows) { return this; diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java index f5c4b7546c6a..19b473e5cef8 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java @@ -455,20 +455,26 @@ public static void updateAssignableTypes(BigBang bb) { } } for (AnalysisType type : allTypes) { - if (type.assignableTypes != null) { - TypeState assignableTypeState = TypeState.forNull(); - if (newAssignableTypes.get(type.getId()) != null) { - BitSet assignableTypes = newAssignableTypes.get(type.getId()); - if (type.assignableTypes.getState().hasExactTypes(assignableTypes)) { - /* Avoid creation of the expensive type state. */ - continue; - } - assignableTypeState = TypeState.forExactTypes(bb, newAssignableTypes.get(type.getId()), true); + if (type.assignableTypes == null) { + /* + * Computing assignable types in bulk here is much cheaper than doing it + * individually when needed in updateTypeFlows. + */ + type.assignableTypes = new AllInstantiatedTypeFlow(type, TypeState.forNull()); + type.assignableTypesNonNull = new AllInstantiatedTypeFlow(type, TypeState.forEmpty()); + } + TypeState assignableTypeState = TypeState.forNull(); + if (newAssignableTypes.get(type.getId()) != null) { + BitSet assignableTypes = newAssignableTypes.get(type.getId()); + if (type.assignableTypes.getState().hasExactTypes(assignableTypes)) { + /* Avoid creation of the expensive type state. */ + continue; } - - updateFlow(bb, type.assignableTypes, assignableTypeState, changedFlows); - updateFlow(bb, type.assignableTypesNonNull, assignableTypeState.forNonNull(bb), changedFlows); + assignableTypeState = TypeState.forExactTypes(bb, newAssignableTypes.get(type.getId()), true); } + + updateFlow(bb, type.assignableTypes, assignableTypeState, changedFlows); + updateFlow(bb, type.assignableTypesNonNull, assignableTypeState.forNonNull(bb), changedFlows); } for (TypeFlow changedFlow : changedFlows) { From e41a4777e9e04f85612bdd02ae93e3923912caf4 Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Thu, 17 Jun 2021 23:03:36 -0700 Subject: [PATCH 08/18] Scan reflection-related objects only once --- .../hosted/ReflectionObjectReplacer.java | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionObjectReplacer.java b/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionObjectReplacer.java index d39fe52f3d99..69e70085aae8 100644 --- a/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionObjectReplacer.java +++ b/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionObjectReplacer.java @@ -31,6 +31,8 @@ import java.lang.reflect.Field; import java.lang.reflect.Method; import java.lang.reflect.Parameter; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import org.graalvm.nativeimage.ImageSingletons; @@ -42,6 +44,7 @@ import com.oracle.graal.pointsto.meta.AnalysisUniverse; import com.oracle.svm.core.annotate.Delete; +import sun.reflect.generics.repository.AbstractRepository; import sun.reflect.generics.repository.ClassRepository; import sun.reflect.generics.repository.ConstructorRepository; import sun.reflect.generics.repository.FieldRepository; @@ -55,9 +58,37 @@ public ReflectionObjectReplacer(AnalysisMetaAccess metaAccess) { this.metaAccess = metaAccess; } + static class Identity { + private final Object wrapped; + + Identity(Object wrapped) { + this.wrapped = wrapped; + } + + @Override + public int hashCode() { + return System.identityHashCode(wrapped); + } + + @Override + public boolean equals(Object obj) { + return ((Identity) obj).wrapped == wrapped; + } + } + + private final Set scanned = ConcurrentHashMap.newKeySet(); + @Override public Object apply(Object original) { + if (original instanceof AccessibleObject || original instanceof Parameter || original instanceof AbstractRepository) { + if (scanned.add(new Identity(original))) { + scan(original); + } + } + return original; + } + private void scan(Object original) { /* * Reflection accessors use Unsafe, so ensure that all reflectively accessible fields are * registered as unsafe-accessible, whether they have been explicitly registered or their @@ -142,7 +173,5 @@ public Object apply(Object original) { classRepository.getSuperclass(); classRepository.getSuperInterfaces(); } - - return original; } } From 37c51f39c13125b00df4f9588db9318253c182d6 Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Thu, 17 Jun 2021 23:20:50 -0700 Subject: [PATCH 09/18] Track individual classes that need to be processed for reflection --- .../reflect/hosted/ReflectionDataBuilder.java | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionDataBuilder.java b/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionDataBuilder.java index ef6f471601fa..2ca61a147026 100644 --- a/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionDataBuilder.java +++ b/substratevm/src/com.oracle.svm.reflect/src/com/oracle/svm/reflect/hosted/ReflectionDataBuilder.java @@ -32,7 +32,6 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -62,7 +61,7 @@ public class ReflectionDataBuilder implements RuntimeReflectionSupport { public static final Constructor[] EMPTY_CONSTRUCTORS = new Constructor[0]; public static final Class[] EMPTY_CLASSES = new Class[0]; - private boolean modified; + private final Set> modifiedClasses = ConcurrentHashMap.newKeySet(); private boolean sealed; private final DynamicHub.ReflectionData arrayReflectionData; @@ -110,16 +109,20 @@ private static DynamicHub.ReflectionData getArrayReflectionData() { @Override public void register(Class... classes) { checkNotSealed(); - if (reflectionClasses.addAll(Arrays.asList(classes))) { - modified = true; + for (Class clazz : classes) { + if (reflectionClasses.add(clazz)) { + modifiedClasses.add(clazz); + } } } @Override public void register(Executable... methods) { checkNotSealed(); - if (reflectionMethods.addAll(Arrays.asList(methods))) { - modified = true; + for (Executable method : methods) { + if (reflectionMethods.add(method)) { + modifiedClasses.add(method.getDeclaringClass()); + } } } @@ -127,8 +130,10 @@ public void register(Executable... methods) { public void register(boolean finalIsWritable, Field... fields) { checkNotSealed(); // Unsafe and write accesses are always enabled for fields because accessors use Unsafe. - if (reflectionFields.addAll(Arrays.asList(fields))) { - modified = true; + for (Field field : fields) { + if (reflectionFields.add(field)) { + modifiedClasses.add(field.getDeclaringClass()); + } } } @@ -187,17 +192,15 @@ private void processReachableTypes(DuringAnalysisAccessImpl access) { } private void processRegisteredElements(DuringAnalysisAccessImpl access) { - if (!modified) { + if (modifiedClasses.isEmpty()) { return; } - modified = false; access.requireAnalysisIteration(); - Set> allClasses = new HashSet<>(reflectionClasses); - reflectionMethods.stream().map(Executable::getDeclaringClass).forEach(allClasses::add); - reflectionFields.stream().map(Field::getDeclaringClass).forEach(allClasses::add); - - allClasses.forEach(clazz -> processClass(access, clazz)); + for (Class clazz : modifiedClasses) { + processClass(access, clazz); + } + modifiedClasses.clear(); } private void processClass(DuringAnalysisAccessImpl access, Class clazz) { @@ -324,7 +327,7 @@ private static void reportLinkingError(Class clazz, Throwable e) { protected void afterAnalysis() { sealed = true; - if (modified) { + if (!modifiedClasses.isEmpty()) { throw UserError.abort("Registration of classes, methods, and fields for reflective access during analysis must set DuringAnalysisAccess.requireAnalysisIteration()."); } } From 465b30f051522049aa07317d01b1a129fb8e3728 Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Fri, 18 Jun 2021 10:35:39 -0700 Subject: [PATCH 10/18] Discard Graal graphs used for static analysis when no longer necessary --- .../src/com/oracle/svm/hosted/code/CompileQueue.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java index 1045110811f1..e84e7dbbbdae 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java @@ -744,6 +744,13 @@ private StructuredGraph transplantGraph(DebugContext debug, HostedMethod hMethod if (aGraph == null) { throw VMError.shouldNotReachHere("Method not parsed during static analysis: " + aMethod.format("%r %H.%n(%p)") + ". Reachable from: " + reason); } + + /* + * The graph in the analysis universe is no longer necessary once it is transplanted into + * the hosted universe. + */ + aMethod.setAnalyzedGraph(null); + StructuredGraph graph = aGraph.copy(universe.lookup(aGraph.method()), getCustomizedOptions(debug), debug); IdentityHashMap replacements = new IdentityHashMap<>(); From a17fd40eeb06e053c0e5fac370e0f7c66560f57a Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Fri, 18 Jun 2021 16:10:32 -0700 Subject: [PATCH 11/18] Allow to disable verifyAssignableTypes using an option --- .../graal/pointsto/meta/AnalysisType.java | 5 ++++- .../svm/hosted/NativeImageGenerator.java | 21 +++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java index 19b473e5cef8..12e05d99a6b0 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisType.java @@ -427,7 +427,10 @@ public static boolean verifyAssignableTypes(BigBang bb) { } } } - return pass; + if (!pass) { + throw new AssertionError("Verification of all-instantiated type flows failed"); + } + return true; } public static void updateAssignableTypes(BigBang bb) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java index d3703587ca16..0de49eff4398 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java @@ -734,12 +734,7 @@ private boolean runPointsToAnalysis(String imageName, OptionValues options, Debu } } } - - /* - * This verification has quadratic complexity, so do it only once after the static - * analysis has finished. - */ - assert AnalysisType.verifyAssignableTypes(bigbang) : "Verification of all-instantiated type flows failed"; + assert verifyAssignableTypes(imageName); /* * Libraries defined via @CLibrary annotations are added at the end of the list of @@ -812,6 +807,20 @@ private boolean runPointsToAnalysis(String imageName, OptionValues options, Debu return false; } + @SuppressWarnings("try") + private boolean verifyAssignableTypes(String imageName) { + /* + * This verification has quadratic complexity, so do it only once after the static analysis + * has finished, and can be disabled with an option. + */ + if (SubstrateOptions.DisableTypeIdResultVerification.getValue()) { + return true; + } + try (StopTimer t = new Timer(imageName, "(verifyAssignableTypes)").start()) { + return AnalysisType.verifyAssignableTypes(bigbang); + } + } + @SuppressWarnings("try") private void setupNativeImage(String imageName, OptionValues options, Map entryPoints, JavaMainSupport javaMainSupport, SubstitutionProcessor harnessSubstitutions, ForkJoinPool analysisExecutor, SnippetReflectionProvider originalSnippetReflection, DebugContext debug) { From 9d9b21cc8e3c5c00fd55351161590449056d089f Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Fri, 18 Jun 2021 10:43:19 -0700 Subject: [PATCH 12/18] Clear AnalysisParsedGraph when no longer needed --- .../src/com/oracle/graal/pointsto/meta/AnalysisMethod.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java index d82ed0c73ffb..1a84e36cb217 100644 --- a/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java +++ b/substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/meta/AnalysisMethod.java @@ -95,6 +95,7 @@ public class AnalysisMethod implements WrappedJavaMethod, GraphProvider, Origina private final AtomicReference parsedGraphCacheState = new AtomicReference<>(GRAPH_CACHE_UNPARSED); private static final Object GRAPH_CACHE_UNPARSED = "unparsed"; + private static final Object GRAPH_CACHE_CLEARED = "cleared by cleanupAfterAnalysis"; private StructuredGraph analyzedGraph; @@ -179,6 +180,9 @@ public void cleanupAfterAnalysis() { typeFlow = null; invokedBy = null; implementationInvokedBy = null; + if (parsedGraphCacheState.get() instanceof AnalysisParsedGraph) { + parsedGraphCacheState.set(GRAPH_CACHE_CLEARED); + } } public void startTrackInvocations() { @@ -634,6 +638,9 @@ public AnalysisParsedGraph ensureGraphParsed(BigBang bb) { } else if (curState instanceof Throwable) { throw AnalysisError.shouldNotReachHere("parsing had failed in another thread", (Throwable) curState); + } else if (curState == GRAPH_CACHE_CLEARED) { + return null; + } else { throw AnalysisError.shouldNotReachHere("Unknown state: " + curState); } From 54119c5d0b98ee2dbcc3730595a91e6db810c354 Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Fri, 18 Jun 2021 19:41:17 -0700 Subject: [PATCH 13/18] Shut down all ForkJoinPool --- .../svm/hosted/NativeImageGenerator.java | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java index 0de49eff4398..edb16b5b2275 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java @@ -461,35 +461,40 @@ public void run(Map entryPoints, SubstitutionProcessor harnessSubstitutions, ForkJoinPool compilationExecutor, ForkJoinPool analysisExecutor, EconomicSet allOptionNames) { - if (!buildStarted.compareAndSet(false, true)) { - throw UserError.abort("An image build has already been performed with this generator."); - } - try { - /* - * JVMCI 20.2-b01 introduced new methods for linking and querying whether an interface - * has default methods. Fail early if these methods are missing. - */ - ResolvedJavaType.class.getDeclaredMethod("link"); - } catch (ReflectiveOperationException ex) { - throw UserError.abort("JVMCI version provided %s is missing the 'ResolvedJavaType.link()' method added in jvmci-20.2-b01. " + - "Please use the latest JVMCI JDK from %s or %s.", System.getProperty("java.home"), JVMCI8_RELEASES_URL, JVMCI11_RELEASES_URL); - } + if (!buildStarted.compareAndSet(false, true)) { + throw UserError.abort("An image build has already been performed with this generator."); + } - setSystemPropertiesForImageLate(k); + try { + /* + * JVMCI 20.2-b01 introduced new methods for linking and querying whether an + * interface has default methods. Fail early if these methods are missing. + */ + ResolvedJavaType.class.getDeclaredMethod("link"); + } catch (ReflectiveOperationException ex) { + throw UserError.abort("JVMCI version provided %s is missing the 'ResolvedJavaType.link()' method added in jvmci-20.2-b01. " + + "Please use the latest JVMCI JDK from %s or %s.", System.getProperty("java.home"), JVMCI8_RELEASES_URL, JVMCI11_RELEASES_URL); + } + + setSystemPropertiesForImageLate(k); - ImageSingletonsSupportImpl.HostedManagement.install(new ImageSingletonsSupportImpl.HostedManagement()); + ImageSingletonsSupportImpl.HostedManagement.install(new ImageSingletonsSupportImpl.HostedManagement()); - ImageSingletons.add(BuildArtifacts.class, (type, artifact) -> buildArtifacts.computeIfAbsent(type, t -> new ArrayList<>()).add(artifact)); - ImageSingletons.add(ClassLoaderQuery.class, new ClassLoaderQueryImpl(loader.getClassLoader())); - ImageSingletons.add(HostedOptionValues.class, new HostedOptionValues(optionProvider.getHostedValues())); - ImageSingletons.add(RuntimeOptionValues.class, new RuntimeOptionValues(optionProvider.getRuntimeValues(), allOptionNames)); - watchdog = new DeadlockWatchdog(); - try (TemporaryBuildDirectoryProviderImpl tempDirectoryProvider = new TemporaryBuildDirectoryProviderImpl()) { - ImageSingletons.add(TemporaryBuildDirectoryProvider.class, tempDirectoryProvider); - doRun(entryPoints, javaMainSupport, imageName, k, harnessSubstitutions, compilationExecutor, analysisExecutor); + ImageSingletons.add(BuildArtifacts.class, (type, artifact) -> buildArtifacts.computeIfAbsent(type, t -> new ArrayList<>()).add(artifact)); + ImageSingletons.add(ClassLoaderQuery.class, new ClassLoaderQueryImpl(loader.getClassLoader())); + ImageSingletons.add(HostedOptionValues.class, new HostedOptionValues(optionProvider.getHostedValues())); + ImageSingletons.add(RuntimeOptionValues.class, new RuntimeOptionValues(optionProvider.getRuntimeValues(), allOptionNames)); + watchdog = new DeadlockWatchdog(); + try (TemporaryBuildDirectoryProviderImpl tempDirectoryProvider = new TemporaryBuildDirectoryProviderImpl()) { + ImageSingletons.add(TemporaryBuildDirectoryProvider.class, tempDirectoryProvider); + doRun(entryPoints, javaMainSupport, imageName, k, harnessSubstitutions, compilationExecutor, analysisExecutor); + } finally { + watchdog.close(); + } } finally { - watchdog.close(); + analysisExecutor.shutdownNow(); + compilationExecutor.shutdownNow(); } } From 73751ed1f89db7c10135b31ba0813c10f55416e6 Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Fri, 18 Jun 2021 21:07:06 -0700 Subject: [PATCH 14/18] Remove unused MustNotSynchronize annotation and checker --- .../svm/core/annotate/MustNotSynchronize.java | 50 ---- .../oracle/svm/hosted/code/CompileQueue.java | 3 - .../MustNotSynchronizeAnnotationChecker.java | 220 ------------------ 3 files changed, 273 deletions(-) delete mode 100644 substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/annotate/MustNotSynchronize.java delete mode 100644 substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/MustNotSynchronizeAnnotationChecker.java diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/annotate/MustNotSynchronize.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/annotate/MustNotSynchronize.java deleted file mode 100644 index 9e21b9f168c3..000000000000 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/annotate/MustNotSynchronize.java +++ /dev/null @@ -1,50 +0,0 @@ -/* - * Copyright (c) 2017, 2017, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ -package com.oracle.svm.core.annotate; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** - * Methods annotated with @MustNotSynchronize(MustNotSynchronize.BLACKLIST) may not - * synchronize, nor may any of their transitive callees unless that callee is annotated with - * @MustNotSynchronize(MustNotSynchronize.WHITELIST). - */ -@Retention(RetentionPolicy.RUNTIME) -@Target({ElementType.METHOD, ElementType.CONSTRUCTOR}) -public @interface MustNotSynchronize { - - /** Constants for use in annotations. */ - boolean WHITELIST = false; - boolean BLACKLIST = true; - - /** Whether the method is on the blacklist or the whitelist. */ - boolean list() default BLACKLIST; - - /** Why the method is annotated. */ - String reason(); -} diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java index e84e7dbbbdae..9501ecf079b0 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java @@ -370,9 +370,6 @@ public void finish(DebugContext debug) { // Checking @RestrictHeapAccess annotations does not take long enough to justify a // timer. RestrictHeapAccessAnnotationChecker.check(debug, universe, universe.getMethods()); - // Checking @MustNotSynchronize annotations does not take long enough to justify a - // timer. - MustNotSynchronizeAnnotationChecker.check(debug, universe.getMethods()); if (SubstrateOptions.AOTInline.getValue() && SubstrateOptions.AOTTrivialInline.getValue()) { try (StopTimer ignored = new Timer(imageName, "(inline)").start()) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/MustNotSynchronizeAnnotationChecker.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/MustNotSynchronizeAnnotationChecker.java deleted file mode 100644 index 06fc7d5c8131..000000000000 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/MustNotSynchronizeAnnotationChecker.java +++ /dev/null @@ -1,220 +0,0 @@ -/* - * Copyright (c) 2017, 2017, Oracle and/or its affiliates. All rights reserved. - * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. - * - * This code is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License version 2 only, as - * published by the Free Software Foundation. Oracle designates this - * particular file as subject to the "Classpath" exception as provided - * by Oracle in the LICENSE file that accompanied this code. - * - * This code is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License - * version 2 for more details (a copy is included in the LICENSE file that - * accompanied this code). - * - * You should have received a copy of the GNU General Public License version - * 2 along with this work; if not, write to the Free Software Foundation, - * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. - * - * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA - * or visit www.oracle.com if you need additional information or have any - * questions. - */ -package com.oracle.svm.hosted.code; - -import java.util.ArrayDeque; -import java.util.Collection; -import java.util.Deque; -import java.util.Iterator; - -import org.graalvm.compiler.debug.DebugContext; -import org.graalvm.compiler.graph.Node; -import org.graalvm.compiler.nodes.Invoke; -import org.graalvm.compiler.nodes.StructuredGraph; -import org.graalvm.compiler.nodes.java.MonitorEnterNode; -import org.graalvm.compiler.options.Option; - -import com.oracle.svm.core.annotate.MustNotSynchronize; -import com.oracle.svm.core.option.HostedOptionKey; -import com.oracle.svm.hosted.meta.HostedMethod; - -public final class MustNotSynchronizeAnnotationChecker { - - /* - * Command line options so errors are not fatal to the build. - */ - public static class Options { - @Option(help = "Print warnings for @MustNotSynchronize annotations.")// - public static final HostedOptionKey PrintMustNotSynchronizeWarnings = new HostedOptionKey<>(true); - - @Option(help = "Print path for @MustNotSynchronize warnings.")// - public static final HostedOptionKey PrintMustNotSynchronizePath = new HostedOptionKey<>(true); - - @Option(help = "Warnings for @MustNotSynchronize annotations are fatal.")// - public static final HostedOptionKey MustNotSynchronizeWarningsAreFatal = new HostedOptionKey<>(true); - } - - /** A collection of methods from the universe. */ - private final Collection methods; - - /** - * Stacks of methods and their implementations that are currently being examined, to detect - * cycles in the call graph. - */ - private final Deque methodPath; - private final Deque methodImplPath; - - /** Private constructor for {@link #check}. */ - private MustNotSynchronizeAnnotationChecker(Collection methods) { - this.methods = methods; - this.methodPath = new ArrayDeque<>(); - this.methodImplPath = new ArrayDeque<>(); - } - - /** Entry point method. */ - public static void check(DebugContext debug, Collection methods) { - final MustNotSynchronizeAnnotationChecker checker = new MustNotSynchronizeAnnotationChecker(methods); - checker.checkMethods(debug); - } - - /** Check methods with the {@link MustNotSynchronize} annotation. */ - @SuppressWarnings("try") - public void checkMethods(DebugContext debug) { - for (HostedMethod method : methods) { - try (DebugContext.Scope s = debug.scope("MustNotSynchronizeAnnotationChecker", method.compilationInfo.graph, method, this)) { - MustNotSynchronize annotation = method.getAnnotation(MustNotSynchronize.class); - if ((annotation != null) && (annotation.list() == MustNotSynchronize.BLACKLIST)) { - methodPath.clear(); - methodImplPath.clear(); - try { - checkMethod(method, method); - } catch (WarningException we) { - // Clean up the recursive stack trace for Debug.scope. - throw new WarningException(we.getMessage()); - } - } - } catch (Throwable t) { - throw debug.handle(t); - } - } - } - - /** Check this method for direct synchronizations or calls to methods that synchronize. */ - protected boolean checkMethod(HostedMethod method, HostedMethod methodImpl) throws WarningException { - if (methodImplPath.contains(methodImpl)) { - // If the method is already on the path then avoid recursion. - return false; - } - MustNotSynchronize annotation = methodImpl.getAnnotation(MustNotSynchronize.class); - if ((annotation != null) && (annotation.list() == MustNotSynchronize.WHITELIST)) { - // The method is on the whitelist, so I do not care if it synchronizes. - return false; - } - methodPath.push(method); - methodImplPath.push(methodImpl); - try { - // Check for direct synchronizations. - if (synchronizesDirectly(methodImpl)) { - return true; - } - if (synchronizesIndirectly(methodImpl)) { - return true; - } - return false; - } finally { - methodPath.pop(); - methodImplPath.pop(); - } - } - - /** Does this method synchronize directly? */ - protected boolean synchronizesDirectly(HostedMethod methodImpl) throws WarningException { - final StructuredGraph graph = methodImpl.compilationInfo.getGraph(); - if (graph != null) { - for (Node node : graph.getNodes()) { - if (node instanceof MonitorEnterNode) { - postMustNotSynchronizeWarning(); - return true; - } - } - } - return false; - } - - /** Does this method call a method that synchronizes? */ - protected boolean synchronizesIndirectly(HostedMethod methodImpl) throws WarningException { - boolean result = false; - final StructuredGraph graph = methodImpl.compilationInfo.getGraph(); - if (graph != null) { - for (Invoke invoke : graph.getInvokes()) { - final HostedMethod callee = (HostedMethod) invoke.callTarget().targetMethod(); - if (invoke.callTarget().invokeKind().isDirect()) { - result |= checkMethod(callee, callee); - if (result) { - return result; - } - } else { - for (HostedMethod calleeImpl : callee.getImplementations()) { - result |= checkMethod(callee, calleeImpl); - /* One violation is too many. */ - if (result) { - return result; - } - } - } - } - } - return result; - } - - private void postMustNotSynchronizeWarning() throws WarningException { - final HostedMethod blacklistMethod = methodPath.getLast(); - String message = "@MustNotSynchronize warning: "; - if (methodPath.size() == 1) { - message += "Blacklisted method: " + blacklistMethod.format("%h.%n(%p)") + " synchronizes."; - } else { - final HostedMethod witness = methodPath.getFirst(); - message += "Blacklisted method: " + blacklistMethod.format("%h.%n(%p)") + " calls " + witness.format("%h.%n(%p)") + " that synchronizes."; - } - if (Options.PrintMustNotSynchronizeWarnings.getValue()) { - System.err.println(message); - if (Options.PrintMustNotSynchronizePath.getValue() && (1 < methodPath.size())) { - printPath(); - } - } - if (Options.MustNotSynchronizeWarningsAreFatal.getValue()) { - throw new WarningException(message); - } - } - - private void printPath() { - System.out.print(" [Path: "); - final Iterator methodIterator = methodPath.iterator(); - final Iterator methodImplIterator = methodImplPath.iterator(); - while (methodIterator.hasNext()) { - final HostedMethod method = methodIterator.next(); - final HostedMethod methodImpl = methodImplIterator.next(); - System.err.println(); - if (method.equals(methodImpl)) { - /* If the method and the implementation are the same, give a short message. */ - System.err.print(" " + method.format("%h.%n(%p)")); - } else { - /* Else give a longer message to help people follow virtual calls. */ - System.err.print(" " + method.format("%f %h.%n(%p)") + " implemented by " + methodImpl.format("%h.%n(%p)")); - } - } - System.err.println("]"); - } - - public static class WarningException extends Exception { - - public WarningException(String message) { - super(message); - } - - /** Every exception needs a generated serialVersionUID. */ - private static final long serialVersionUID = 5793144021924912791L; - } -} From 20c7be2a5edbb88df07d739d5672d2a421087405 Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Fri, 18 Jun 2021 22:02:38 -0700 Subject: [PATCH 15/18] Only use NodeSourcePosition when explicitly enabled --- .../graalvm/compiler/nodes/StructuredGraph.java | 16 ++++++++-------- .../com/oracle/svm/hosted/code/CompileQueue.java | 15 +++++++++++++-- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/StructuredGraph.java b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/StructuredGraph.java index 500bd0c228fb..100cae11097a 100644 --- a/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/StructuredGraph.java +++ b/compiler/src/org.graalvm.compiler.nodes/src/org/graalvm/compiler/nodes/StructuredGraph.java @@ -679,7 +679,7 @@ public void logInliningTree() { */ @Override protected Graph copy(String newName, Consumer> duplicationMapCallback, DebugContext debugForCopy) { - return copy(newName, rootMethod, getOptions(), duplicationMapCallback, compilationId, debugForCopy); + return copy(newName, rootMethod, getOptions(), duplicationMapCallback, compilationId, debugForCopy, trackNodeSourcePosition); } /** @@ -695,12 +695,12 @@ protected Graph copy(String newName, Consumer> duplicationMapCallback, DebugContext debugForCopy, OptionValues options) { - return copy(newName, rootMethod, options, duplicationMapCallback, compilationId, debugForCopy); + return copy(newName, rootMethod, options, duplicationMapCallback, compilationId, debugForCopy, trackNodeSourcePosition); } @SuppressWarnings("try") private StructuredGraph copy(String newName, ResolvedJavaMethod rootMethodForCopy, OptionValues optionsForCopy, Consumer> duplicationMapCallback, - CompilationIdentifier newCompilationId, DebugContext debugForCopy) { + CompilationIdentifier newCompilationId, DebugContext debugForCopy, boolean trackNodeSourcePositionForCopy) { AllowAssumptions allowAssumptions = allowAssumptions(); StructuredGraph copy = new StructuredGraph(newName, rootMethodForCopy, @@ -710,7 +710,7 @@ private StructuredGraph copy(String newName, ResolvedJavaMethod rootMethodForCop useProfilingInfo, isSubstitution, methods != null ? new ArrayList<>(methods) : null, - trackNodeSourcePosition, + trackNodeSourcePositionForCopy, newCompilationId, optionsForCopy, debugForCopy, null, callerContext); if (allowAssumptions == AllowAssumptions.YES && assumptions != null) { @@ -719,7 +719,7 @@ private StructuredGraph copy(String newName, ResolvedJavaMethod rootMethodForCop copy.hasUnsafeAccess = hasUnsafeAccess; copy.setGuardsStage(getGuardsStage()); copy.stageFlags = EnumSet.copyOf(stageFlags); - copy.trackNodeSourcePosition = trackNodeSourcePosition; + copy.trackNodeSourcePosition = trackNodeSourcePositionForCopy; if (fields != null) { copy.fields = createFieldSet(fields); } @@ -744,11 +744,11 @@ private StructuredGraph copy(String newName, ResolvedJavaMethod rootMethodForCop * accessed by multiple threads). */ public StructuredGraph copyWithIdentifier(CompilationIdentifier newCompilationId, DebugContext debugForCopy) { - return copy(name, rootMethod, getOptions(), null, newCompilationId, debugForCopy); + return copy(name, rootMethod, getOptions(), null, newCompilationId, debugForCopy, trackNodeSourcePosition); } - public StructuredGraph copy(ResolvedJavaMethod rootMethodForCopy, OptionValues optionsForCopy, DebugContext debugForCopy) { - return copy(name, rootMethodForCopy, optionsForCopy, null, compilationId, debugForCopy); + public StructuredGraph copy(ResolvedJavaMethod rootMethodForCopy, OptionValues optionsForCopy, DebugContext debugForCopy, boolean trackNodeSourcePositionForCopy) { + return copy(name, rootMethodForCopy, optionsForCopy, null, compilationId, debugForCopy, trackNodeSourcePositionForCopy); } public ParameterNode getParameter(int index) { diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java index 9501ecf079b0..b96f3be1c73d 100644 --- a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java +++ b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/code/CompileQueue.java @@ -53,6 +53,7 @@ import org.graalvm.compiler.core.GraalCompiler; import org.graalvm.compiler.core.common.CompilationIdentifier; import org.graalvm.compiler.core.common.CompilationIdentifier.Verbosity; +import org.graalvm.compiler.core.common.GraalOptions; import org.graalvm.compiler.core.common.spi.CodeGenProviders; import org.graalvm.compiler.core.common.type.AbstractObjectStamp; import org.graalvm.compiler.core.common.type.ObjectStamp; @@ -748,7 +749,13 @@ private StructuredGraph transplantGraph(DebugContext debug, HostedMethod hMethod */ aMethod.setAnalyzedGraph(null); - StructuredGraph graph = aGraph.copy(universe.lookup(aGraph.method()), getCustomizedOptions(debug), debug); + OptionValues options = getCustomizedOptions(debug); + /* + * The static analysis always needs NodeSourcePosition. But for AOT compilation, we only + * need to preserve them when explicitly enabled, to reduce memory pressure. + */ + boolean trackNodeSourcePosition = GraalOptions.TrackNodeSourcePosition.getValue(options); + StructuredGraph graph = aGraph.copy(universe.lookup(aGraph.method()), options, debug, trackNodeSourcePosition); IdentityHashMap replacements = new IdentityHashMap<>(); for (Node node : graph.getNodes()) { @@ -765,7 +772,11 @@ private StructuredGraph transplantGraph(DebugContext debug, HostedMethod hMethod * The NodeSourcePosition is not part of the regular "data" fields, so we need to * process it manually. */ - node.setNodeSourcePosition((NodeSourcePosition) replaceAnalysisObjects(node.getNodeSourcePosition(), node, replacements, universe)); + if (trackNodeSourcePosition) { + node.setNodeSourcePosition((NodeSourcePosition) replaceAnalysisObjects(node.getNodeSourcePosition(), node, replacements, universe)); + } else { + node.clearNodeSourcePosition(); + } } return graph; From 11926c17aa5cfff511e819fe993bc9897913ce21 Mon Sep 17 00:00:00 2001 From: Christian Wimmer Date: Fri, 18 Jun 2021 23:25:49 -0700 Subject: [PATCH 16/18] Disable assignable-type verification by default --- .../src/com/oracle/svm/core/SubstrateOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java index b9b51ad3b6c2..ef323be6f15a 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/SubstrateOptions.java @@ -542,7 +542,7 @@ public Boolean getValue(OptionValues values) { public static final RuntimeOptionKey ActiveProcessorCount = new RuntimeOptionKey<>(-1); @Option(help = "For internal purposes only. Disables type id result verification even when running with assertions enabled.", stability = OptionStability.EXPERIMENTAL, type = Debug)// - public static final HostedOptionKey DisableTypeIdResultVerification = new HostedOptionKey<>(false); + public static final HostedOptionKey DisableTypeIdResultVerification = new HostedOptionKey<>(true); public static boolean areMethodHandlesSupported() { return JavaVersionUtil.JAVA_SPEC >= 11; From 514fc26c7d7ae4872d51c964a1c196ca88c436d5 Mon Sep 17 00:00:00 2001 From: Jakub Chaloupka Date: Tue, 2 Mar 2021 15:44:35 +0100 Subject: [PATCH 17/18] Polyglot Context States. --- .../truffle/runtime/OptimizedCallTarget.java | 5 +- truffle/CHANGELOG.md | 1 + .../ContextInterruptCloseOrCancelTest.java | 259 ++++ .../ContextInterruptParameterizedTest.java | 3 - .../test/ContextInterruptStandaloneTest.java | 15 +- ...InnerAndOuterContextCancellationTest.java} | 11 +- .../test/InstrumentationTestLanguage.java | 63 +- .../test/MultiThreadingTest.java | 137 ++ ...tingAndPolyglotThreadCancellationTest.java | 278 ++++ .../test/ResourceLimitsTest.java | 487 ++++--- .../test/TruffleContextTest.java | 1 + .../LanguageFinalizationFailureTest.java | 151 +++ .../api/test/polyglot/LanguageSPITest.java | 102 +- .../oracle/truffle/api/TruffleContext.java | 10 +- .../oracle/truffle/api/TruffleLanguage.java | 16 +- .../truffle/api/impl/DefaultCallTarget.java | 10 +- .../api/impl/ThreadLocalHandshake.java | 13 +- .../truffle/host/GuestToHostRootNode.java | 1 - .../truffle/polyglot/EngineAccessor.java | 31 +- .../truffle/polyglot/HostToGuestRootNode.java | 4 +- .../polyglot/PauseThreadLocalAction.java | 3 +- .../truffle/polyglot/PolyglotContextImpl.java | 1165 +++++++++++------ .../polyglot/PolyglotEngineException.java | 19 +- .../truffle/polyglot/PolyglotEngineImpl.java | 167 ++- .../polyglot/PolyglotExceptionImpl.java | 15 +- .../oracle/truffle/polyglot/PolyglotImpl.java | 11 +- .../polyglot/PolyglotLanguageContext.java | 72 +- .../truffle/polyglot/PolyglotLimits.java | 16 +- .../PolyglotStackFramesRetriever.java | 2 +- .../truffle/polyglot/PolyglotThread.java | 10 +- .../truffle/polyglot/PolyglotThreadInfo.java | 4 +- .../polyglot/PolyglotThreadLocalActions.java | 18 +- .../truffle/polyglot/PolyglotWrapper.java | 16 +- 33 files changed, 2276 insertions(+), 840 deletions(-) create mode 100644 truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptCloseOrCancelTest.java rename truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/{CancellationTest.java => InnerAndOuterContextCancellationTest.java} (97%) create mode 100644 truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/MultiThreadingTest.java create mode 100644 truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/NestingAndPolyglotThreadCancellationTest.java create mode 100644 truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/LanguageFinalizationFailureTest.java diff --git a/compiler/src/org.graalvm.compiler.truffle.runtime/src/org/graalvm/compiler/truffle/runtime/OptimizedCallTarget.java b/compiler/src/org.graalvm.compiler.truffle.runtime/src/org/graalvm/compiler/truffle/runtime/OptimizedCallTarget.java index 1eaa07ddfeda..03f0b2b7f38d 100644 --- a/compiler/src/org.graalvm.compiler.truffle.runtime/src/org/graalvm/compiler/truffle/runtime/OptimizedCallTarget.java +++ b/compiler/src/org.graalvm.compiler.truffle.runtime/src/org/graalvm/compiler/truffle/runtime/OptimizedCallTarget.java @@ -625,7 +625,9 @@ private boolean lastTierCompile() { private Object executeRootNode(VirtualFrame frame) { final boolean inCompiled = CompilerDirectives.inCompilationRoot(); try { - return rootNode.execute(frame); + Object toRet = rootNode.execute(frame); + TruffleSafepoint.poll(rootNode); + return toRet; } catch (ControlFlowException t) { throw rethrow(profileExceptionType(t)); } catch (Throwable t) { @@ -636,7 +638,6 @@ private Object executeRootNode(VirtualFrame frame) { if (CompilerDirectives.inInterpreter() && inCompiled) { notifyDeoptimized(frame); } - TruffleSafepoint.poll(rootNode); // this assertion is needed to keep the values from being cleared as non-live locals assert frame != null && this != null; } diff --git a/truffle/CHANGELOG.md b/truffle/CHANGELOG.md index 5849ec706a20..e8e63fd8811f 100644 --- a/truffle/CHANGELOG.md +++ b/truffle/CHANGELOG.md @@ -22,6 +22,7 @@ This changelog summarizes major changes between Truffle versions relevant to lan * Added support for iterators and hash maps to `DebugValue`. The added methods wraps the respective methods of `InteropLibrary`. * Added support for Truffle libraries to be prepared for AOT. See `ExportLibrary.useForAOT` or the `AOTTutorial` java class for further details. * The Specialization DSL now generates code to throw an `AssertionError` if a `@Shared` and `@Cached` parameter returns a non-null value and is used in a guard. The `null` state is reserved for the uninitialized state. +* Changed `TruffleLanguage.disposeContext`. In case the underlying polyglot context is being cancelled, `TruffleLanguage.disposeContext` is called even if `TruffleLanguage.finalizeContext` throws a TruffleException or a ThreadDeath exception. ## Version 21.1.0 * Added methods into `Instrumenter` that create bindings to be attached later on. Added `EventBinding.attach()` method. diff --git a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptCloseOrCancelTest.java b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptCloseOrCancelTest.java new file mode 100644 index 000000000000..cd90fa70e562 --- /dev/null +++ b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptCloseOrCancelTest.java @@ -0,0 +1,259 @@ +/* + * Copyright (c) 2021, 2021, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or + * data (collectively the "Software"), free of charge and under any and all + * copyright rights in the Software, and any and all patent rights owned or + * freely licensable by each licensor hereunder covering either (i) the + * unmodified Software as contributed to or provided by such licensor, or (ii) + * the Larger Works (as defined below), to deal in both + * + * (a) the Software, and + * + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * + * The above copyright notice and either this complete permission notice or at a + * minimum a reference to the UPL must be included in all copies or substantial + * portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package com.oracle.truffle.api.instrumentation.test; + +import java.io.IOException; +import java.lang.reflect.Field; +import java.time.Duration; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; + +import org.graalvm.polyglot.Context; +import org.graalvm.polyglot.Engine; +import org.graalvm.polyglot.Source; +import org.junit.After; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TestName; + +import com.oracle.truffle.api.CompilerDirectives; +import com.oracle.truffle.api.TruffleContext; +import com.oracle.truffle.api.frame.VirtualFrame; +import com.oracle.truffle.api.instrumentation.ContextsListener; +import com.oracle.truffle.api.instrumentation.EventContext; +import com.oracle.truffle.api.instrumentation.ExecutionEventListener; +import com.oracle.truffle.api.instrumentation.SourceSectionFilter; +import com.oracle.truffle.api.instrumentation.TruffleInstrument; +import com.oracle.truffle.api.nodes.LanguageInfo; +import com.oracle.truffle.api.test.polyglot.AbstractPolyglotTest; +import com.oracle.truffle.api.test.polyglot.ProxyLanguage; + +public class ContextInterruptCloseOrCancelTest extends AbstractPolyglotTest { + private void runOnContextClose(TruffleContext triggerCtx, Runnable r) { + instrumentEnv.getInstrumenter().attachContextsListener(new ContextsListener() { + @Override + public void onContextCreated(TruffleContext ctx) { + + } + + @Override + public void onLanguageContextCreated(TruffleContext ctx, LanguageInfo lang) { + + } + + @Override + public void onLanguageContextInitialized(TruffleContext ctx, LanguageInfo lang) { + + } + + @Override + public void onLanguageContextFinalized(TruffleContext ctx, LanguageInfo lang) { + + } + + @Override + public void onLanguageContextDisposed(TruffleContext ctx, LanguageInfo lang) { + + } + + @Override + public void onContextClosed(TruffleContext ctx) { + if (ctx == triggerCtx) { + r.run(); + } + } + }, false); + } + + @Rule public TestName testNameRule = new TestName(); + + @After + public void checkInterrupted() { + Assert.assertFalse("Interrupted flag was left set by test: " + testNameRule.getMethodName(), Thread.interrupted()); + } + + @Test + public void testInterruptPolyglotThreadWhileClosing() throws ExecutionException, InterruptedException, IOException { + enterContext = false; + setupEnv(Context.newBuilder().allowCreateThread(true), new ProxyLanguage() { + @Override + protected boolean isThreadAccessAllowed(Thread thread, boolean singleThreaded) { + return true; + } + }); + ExecutorService executorService = Executors.newSingleThreadExecutor(); + try { + CountDownLatch interruptLatch = new CountDownLatch(1); + Source source = Source.newBuilder(InstrumentationTestLanguage.ID, "ROOT(DEFINE(foo,CONSTANT(42),LOOP(infinity,STATEMENT)),SPAWN(foo))", "InfiniteLoop").build(); + attachListener(interruptLatch::countDown, instrumentEnv); + Future future = executorService.submit(() -> { + context.eval(source); + }); + interruptLatch.await(); + future.get(); + TruffleContext innerContext = languageEnv.newContextBuilder().build(); + AtomicReference> interruptFutureReference = new AtomicReference<>(); + runOnContextClose(innerContext, () -> { + /* + * When inner context is closed we know that the parent context is being closed. + * Parent context is entered in the current thread, so we must interrupt in a + * different thread. + */ + Future interruptFuture = executorService.submit(() -> { + try { + context.interrupt(Duration.ofSeconds(5)); + } catch (TimeoutException te) { + throw new AssertionError(te); + } + }); + interruptFutureReference.set(interruptFuture); + boolean interruptedExceptionSwallowed = false; + try { + /* + * The parent context is entered in the current thread, so the interrupt will + * try to interrupt it. Therefore, we have to ignore the InterruptedException in + * order for the close operation to be able to successfully continue. We cannot + * wait for the interrupt future here though, because it waits also for the + * current entered thread. + */ + interruptFuture.get(); + } catch (ExecutionException e) { + throw new AssertionError(e); + } catch (InterruptedException ie) { + interruptedExceptionSwallowed = true; + } + Assert.assertTrue(interruptedExceptionSwallowed); + Assert.assertFalse(interruptFuture.isDone()); + }); + context.close(); + assertContextState("CLOSED"); + Assert.assertNotNull(interruptFutureReference.get()); + interruptFutureReference.get().get(); + } finally { + executorService.shutdownNow(); + executorService.awaitTermination(100, TimeUnit.SECONDS); + } + } + + @Test + public void testCancelPolyglotThreadWhileClosing() throws ExecutionException, InterruptedException, IOException { + enterContext = false; + setupEnv(Context.newBuilder().allowCreateThread(true), new ProxyLanguage() { + @Override + protected boolean isThreadAccessAllowed(Thread thread, boolean singleThreaded) { + return true; + } + }); + ExecutorService executorService = Executors.newSingleThreadExecutor(); + try { + CountDownLatch cancelLatch = new CountDownLatch(1); + Source source = Source.newBuilder(InstrumentationTestLanguage.ID, "ROOT(DEFINE(foo,CONSTANT(42),LOOP(infinity,STATEMENT)),SPAWN(foo))", "InfiniteLoop").build(); + attachListener(cancelLatch::countDown, instrumentEnv); + Future future = executorService.submit(() -> { + context.eval(source); + }); + cancelLatch.await(); + future.get(); + TruffleContext innerContext = languageEnv.newContextBuilder().build(); + runOnContextClose(innerContext, () -> { + /* + * Cancel must work even when entered. + */ + context.close(true); + }); + context.close(); + assertContextState("CLOSED_CANCELLED"); + } finally { + executorService.shutdownNow(); + executorService.awaitTermination(100, TimeUnit.SECONDS); + } + } + + private void assertContextState(String expectedState) { + Object contextState; + try { + Field implField = context.getClass().getDeclaredField("receiver"); + implField.setAccessible(true); + Object contextImpl = implField.get(context); + Field stateField = contextImpl.getClass().getDeclaredField("state"); + stateField.setAccessible(true); + contextState = stateField.get(contextImpl); + } catch (NoSuchFieldException | IllegalAccessException e) { + throw new RuntimeException(e); + } + Assert.assertEquals(expectedState, ((Enum) contextState).name()); + } + + private static void attachListener(Runnable runnable, TruffleInstrument.Env instrumentEnv) { + instrumentEnv.getInstrumenter().attachExecutionEventListener(SourceSectionFilter.newBuilder().tagIs(InstrumentationTestLanguage.ConstantTag.class).build(), new ExecutionEventListener() { + @Override + public void onEnter(EventContext c, VirtualFrame frame) { + executeSideEffect(); + } + + @CompilerDirectives.TruffleBoundary + void executeSideEffect() { + runnable.run(); + } + + @Override + public void onReturnValue(EventContext c, VirtualFrame frame, Object result) { + + } + + @Override + public void onReturnExceptional(EventContext c, VirtualFrame frame, Throwable exception) { + + } + }); + } + + static TruffleInstrument.Env getInstrumentEnv(Engine engine) { + return engine.getInstruments().get("InstrumentationUpdateInstrument").lookup(TruffleInstrument.Env.class); + } +} diff --git a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptParameterizedTest.java b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptParameterizedTest.java index baa2e733beef..eeea2e04a84c 100644 --- a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptParameterizedTest.java +++ b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptParameterizedTest.java @@ -246,9 +246,6 @@ public void testInterrupt() throws InterruptedException, IOException, ExecutionE } } } - } catch (Exception e) { - e.printStackTrace(); - throw e; } finally { for (int i = 0; i < nThreads; i++) { if (multiContext || i == 0) { diff --git a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptStandaloneTest.java b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptStandaloneTest.java index 05e005da8c62..a459a72784ea 100644 --- a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptStandaloneTest.java +++ b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ContextInterruptStandaloneTest.java @@ -98,6 +98,7 @@ public void checkInterrupted() { @Test public void testCancelDuringHostSleep() throws ExecutionException, InterruptedException { CountDownLatch beforeSleep = new CountDownLatch(1); + enterContext = false; setupEnv(Context.newBuilder(ProxyLanguage.ID).allowHostClassLookup((s) -> true).allowHostAccess(HostAccess.ALL), new ProxyLanguage() { @Override @@ -111,7 +112,7 @@ public Object execute(VirtualFrame frame) { try { InteropLibrary.getUncached().invokeMember(javaThread, "sleep", 10000); } catch (UnsupportedMessageException | ArityException | UnknownIdentifierException | UnsupportedTypeException e) { - throw new RuntimeException(e); + throw new AssertionError(e); } return 0; } @@ -160,7 +161,7 @@ public void run() { Thread.sleep(10000); Assert.fail(); } catch (InterruptedException ie) { - throw new RuntimeException(ie); + throw new AssertionError(ie); } } }, instrEnv); @@ -169,7 +170,7 @@ public void run() { ctx.eval(source); Assert.fail(); } catch (PolyglotException pe) { - if (!pe.isCancelled() || pe.isInterrupted()) { + if (!pe.isCancelled()) { throw pe; } } @@ -215,7 +216,7 @@ public void testParallelCloseAndInterrupt() throws InterruptedException, IOExcep try { ctx.interrupt(Duration.ofSeconds(50)); } catch (TimeoutException te) { - throw new RuntimeException(te); + throw new AssertionError(te); } } })); @@ -261,7 +262,7 @@ public void apply(CountDownLatch arg) throws InterruptedException { throw ie; } } catch (UnsupportedMessageException ume) { - throw new RuntimeException(ume); + throw new AssertionError(ume); } } } @@ -298,7 +299,7 @@ public void testInterruptCurrentThreadEntered() throws IOException { try { ctx[0].interrupt(Duration.ofSeconds(100)); } catch (TimeoutException te) { - throw new RuntimeException(te); + throw new AssertionError(te); } }, getInstrumentEnv(ctx[0].getEngine())); ctx[0].initialize(InstrumentationTestLanguage.ID); @@ -323,7 +324,7 @@ public void testInterruptCurrentThreadEnteredByChild() { ctx[0].interrupt(Duration.ofSeconds(100)); } catch (TimeoutException te) { polyglotThreadException[0] = te; - throw new RuntimeException(te); + throw new AssertionError(te); } catch (IllegalStateException e) { polyglotThreadException[0] = e; if (!"Cannot interrupt context from a thread where its child context is active.".equals(e.getMessage())) { diff --git a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/CancellationTest.java b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/InnerAndOuterContextCancellationTest.java similarity index 97% rename from truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/CancellationTest.java rename to truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/InnerAndOuterContextCancellationTest.java index 423b7b26f94c..48c7278dc65f 100644 --- a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/CancellationTest.java +++ b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/InnerAndOuterContextCancellationTest.java @@ -67,6 +67,7 @@ import org.junit.Test; import org.junit.rules.TestName; +import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.TruffleContext; import com.oracle.truffle.api.TruffleLanguage; import com.oracle.truffle.api.frame.VirtualFrame; @@ -76,7 +77,7 @@ import com.oracle.truffle.api.instrumentation.StandardTags; import com.oracle.truffle.api.instrumentation.TruffleInstrument; -public class CancellationTest { +public class InnerAndOuterContextCancellationTest { private Engine engine; private Context context; @@ -234,6 +235,11 @@ private void captureInnerContext(CountDownLatch cancelLatch, AtomicReference> materializedTags) } } + private static Thread.UncaughtExceptionHandler getPolyglotThreadUncaughtExceptionHandler(InstrumentContext context) { + return (t, e) -> { + InteropLibrary interop = InteropLibrary.getUncached(); + boolean interrupted; + boolean cancelled = false; + if (interop.isException(e)) { + try { + ExceptionType exceptionType = interop.getExceptionType(e); + interrupted = exceptionType == ExceptionType.INTERRUPT; + } catch (UnsupportedMessageException ume) { + throw CompilerDirectives.shouldNotReachHere(ume); + } + } else { + interrupted = e != null && e.getCause() instanceof InterruptedException; + cancelled = e instanceof ThreadDeath; + } + if (e != null && !interrupted && !cancelled) { + Env currentEnv = context.env; + try { + e.printStackTrace(new PrintStream(currentEnv.err())); + } catch (Throwable exc) { + // Still show the original error if printing on Env.err() fails for some + // reason + e.printStackTrace(); + } + } + }; + } + private static final class AsyncStackInfo { private final List callNodes = new ArrayList<>(); @@ -3198,39 +3228,6 @@ public String fileExtension() { protected void finalizeContext(InstrumentContext context) { joinSpawnedThreads(context, true); } - - @Override - protected void initializeThread(InstrumentContext context, Thread thread) { - Thread.UncaughtExceptionHandler currentHandler = thread.getUncaughtExceptionHandler(); - if (currentHandler != null && "com.oracle.truffle.polyglot.PolyglotLanguageContext$PolyglotUncaughtExceptionHandler".equals(currentHandler.getClass().getName())) { - thread.setUncaughtExceptionHandler((t, e) -> { - InteropLibrary interop = InteropLibrary.getUncached(); - boolean interrupted; - boolean cancelled = false; - if (interop.isException(e)) { - try { - ExceptionType exceptionType = interop.getExceptionType(e); - interrupted = exceptionType == ExceptionType.INTERRUPT; - } catch (UnsupportedMessageException ume) { - throw CompilerDirectives.shouldNotReachHere(ume); - } - } else { - interrupted = e != null && e.getCause() instanceof InterruptedException; - cancelled = e instanceof ThreadDeath; - } - if (!interrupted && !cancelled) { - Env currentEnv = context.env; - try { - e.printStackTrace(new PrintStream(currentEnv.err())); - } catch (Throwable exc) { - // Still show the original error if printing on Env.err() fails for some - // reason - e.printStackTrace(); - } - } - }); - } - } } class InstrumentContext { diff --git a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/MultiThreadingTest.java b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/MultiThreadingTest.java new file mode 100644 index 000000000000..ceb6fa0d8655 --- /dev/null +++ b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/MultiThreadingTest.java @@ -0,0 +1,137 @@ +/* + * Copyright (c) 2021, 2021, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or + * data (collectively the "Software"), free of charge and under any and all + * copyright rights in the Software, and any and all patent rights owned or + * freely licensable by each licensor hereunder covering either (i) the + * unmodified Software as contributed to or provided by such licensor, or (ii) + * the Larger Works (as defined below), to deal in both + * + * (a) the Software, and + * + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * + * The above copyright notice and either this complete permission notice or at a + * minimum a reference to the UPL must be included in all copies or substantial + * portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package com.oracle.truffle.api.instrumentation.test; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; + +import org.graalvm.polyglot.Context; +import org.junit.Assert; +import org.junit.Test; + +import com.oracle.truffle.api.TruffleContext; +import com.oracle.truffle.api.nodes.Node; +import com.oracle.truffle.api.test.polyglot.AbstractPolyglotTest; +import com.oracle.truffle.api.test.polyglot.ProxyLanguage; + +public class MultiThreadingTest extends AbstractPolyglotTest { + private static final Node INVALID_NODE = new Node() { + }; + + @Test + public void testInitMultiThreading() throws ExecutionException, InterruptedException { + ExecutorService executorService = Executors.newFixedThreadPool(2); + try { + int multiThreadingInits = 0; + int lastIterationNo = 0; + AtomicBoolean multiThreadingInitNotExecuted = new AtomicBoolean(); + for (int itNo = 0; itNo < 10000 && !multiThreadingInitNotExecuted.get(); itNo++) { + lastIterationNo = itNo; + CountDownLatch enterLeaveLoopLatch = new CountDownLatch(1); + enterContext = false; + AtomicBoolean testEnd = new AtomicBoolean(); + AtomicBoolean multiThreadingInitialized = new AtomicBoolean(); + setupEnv(Context.newBuilder(), new ProxyLanguage() { + @Override + protected boolean isThreadAccessAllowed(Thread thread, boolean singleThreaded) { + return true; + } + + @Override + protected void initializeMultiThreading(LanguageContext c) { + multiThreadingInitialized.set(true); + } + }); + AtomicInteger enteredCount = new AtomicInteger(); + Future future1 = executorService.submit(() -> { + TruffleContext truffleContext = languageEnv.getContext(); + Object prev = truffleContext.enter(INVALID_NODE); + truffleContext.leave(INVALID_NODE, prev); + enterLeaveLoopLatch.countDown(); + while (!testEnd.get()) { + prev = truffleContext.enter(INVALID_NODE); + int localEnteredCount = enteredCount.incrementAndGet(); + if (localEnteredCount > 1 && !multiThreadingInitialized.get()) { + multiThreadingInitNotExecuted.set(true); + } + enteredCount.decrementAndGet(); + truffleContext.leave(INVALID_NODE, prev); + } + }); + Future future2 = executorService.submit(() -> { + TruffleContext truffleContext = languageEnv.getContext(); + try { + enterLeaveLoopLatch.await(); + } catch (InterruptedException ie) { + throw new AssertionError(ie); + } + Object prev = truffleContext.enter(INVALID_NODE); + int localEnteredCount = enteredCount.incrementAndGet(); + if (localEnteredCount > 1 && !multiThreadingInitialized.get()) { + multiThreadingInitNotExecuted.set(true); + } + enteredCount.decrementAndGet(); + truffleContext.leave(INVALID_NODE, prev); + testEnd.set(true); + }); + enterLeaveLoopLatch.await(); + future1.get(); + future2.get(); + context.close(); + if (multiThreadingInitialized.get()) { + multiThreadingInits++; + } + } + Assert.assertTrue("Multi-threading was never initialized!", multiThreadingInits > 0); + Assert.assertFalse("Multi-threading was not initialized even though more than one thread was entered at the same time! lastIterationNo = " + lastIterationNo, + multiThreadingInitNotExecuted.get()); + } finally { + executorService.shutdownNow(); + executorService.awaitTermination(100, TimeUnit.SECONDS); + } + } +} diff --git a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/NestingAndPolyglotThreadCancellationTest.java b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/NestingAndPolyglotThreadCancellationTest.java new file mode 100644 index 000000000000..426f2464f758 --- /dev/null +++ b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/NestingAndPolyglotThreadCancellationTest.java @@ -0,0 +1,278 @@ +/* + * Copyright (c) 2021, 2021, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or + * data (collectively the "Software"), free of charge and under any and all + * copyright rights in the Software, and any and all patent rights owned or + * freely licensable by each licensor hereunder covering either (i) the + * unmodified Software as contributed to or provided by such licensor, or (ii) + * the Larger Works (as defined below), to deal in both + * + * (a) the Software, and + * + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * + * The above copyright notice and either this complete permission notice or at a + * minimum a reference to the UPL must be included in all copies or substantial + * portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package com.oracle.truffle.api.instrumentation.test; + +import static org.junit.Assert.fail; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.Phaser; +import java.util.concurrent.TimeUnit; + +import org.graalvm.polyglot.Context; +import org.graalvm.polyglot.Engine; +import org.graalvm.polyglot.PolyglotException; +import org.graalvm.polyglot.Source; +import org.junit.After; +import org.junit.Assert; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TestName; + +import com.oracle.truffle.api.CompilerDirectives; +import com.oracle.truffle.api.frame.VirtualFrame; +import com.oracle.truffle.api.instrumentation.EventContext; +import com.oracle.truffle.api.instrumentation.ExecutionEventListener; +import com.oracle.truffle.api.instrumentation.SourceSectionFilter; +import com.oracle.truffle.api.instrumentation.StandardTags; +import com.oracle.truffle.api.instrumentation.TruffleInstrument; + +public class NestingAndPolyglotThreadCancellationTest { + @Rule public TestName testNameRule = new TestName(); + + @After + public void checkInterrupted() { + Assert.assertFalse("Interrupted flag was left set by test: " + testNameRule.getMethodName(), Thread.interrupted()); + } + + @Test + public void testCancelSpawnedThread() throws Exception { + try (Context context = Context.newBuilder().allowCreateThread(true).build()) { + context.initialize(InstrumentationTestLanguage.ID); + Source source = Source.newBuilder(InstrumentationTestLanguage.ID, "ROOT(DEFINE(foo,LOOP(infinity,STATEMENT)),SPAWN(foo))", "CancelSpawnedThread").build(); + context.eval(source); + context.close(true); + } + } + + @Test + public void testCancelSpawnedThreadFromAnotherThread() throws Exception { + /* + * This test verifies that cancelling of context from another thread does not cancel the + * main thread too soon when the polyglot thread is still running which would cause the + * implicit close method to throw illegal state exception. + */ + ExecutorService executorService = Executors.newFixedThreadPool(1); + try { + Future future; + try (Context context = Context.newBuilder().allowCreateThread(true).build()) { + context.initialize(InstrumentationTestLanguage.ID); + Source source = Source.newBuilder(InstrumentationTestLanguage.ID, "ROOT(DEFINE(foo,LOOP(infinity,BLOCK(SLEEP(100),STATEMENT))),SPAWN(foo),LOOP(infinity,STATEMENT),JOIN())", + "CancelSpawnedThread").build(); + TruffleInstrument.Env instrumentEnv = context.getEngine().getInstruments().get("InstrumentationUpdateInstrument").lookup(TruffleInstrument.Env.class); + CountDownLatch cancelLatch = new CountDownLatch(1); + instrumentEnv.getInstrumenter().attachExecutionEventListener(SourceSectionFilter.newBuilder().tagIs(StandardTags.StatementTag.class).build(), new ExecutionEventListener() { + @Override + public void onEnter(EventContext ctx, VirtualFrame frame) { + onEnterBoundary(); + } + + @CompilerDirectives.TruffleBoundary + private void onEnterBoundary() { + cancelLatch.countDown(); + } + + @Override + public void onReturnValue(EventContext ctx, VirtualFrame frame, Object result) { + + } + + @Override + public void onReturnExceptional(EventContext ctx, VirtualFrame frame, Throwable exception) { + + } + }); + future = executorService.submit(() -> { + try { + cancelLatch.await(); + } catch (InterruptedException ie) { + } + context.close(true); + }); + try { + context.eval(source); + fail(); + } catch (PolyglotException pe) { + /* + * We need to catch the cancelled exception here, if we did that in the main + * try-with-resources block, then the potential IllegalStateException caused by + * not all polyglot threads being finished would be suppressed by the cancelled + * exception. + */ + if (!pe.isCancelled()) { + throw pe; + } + } + } + future.get(); + } finally { + executorService.shutdownNow(); + executorService.awaitTermination(100, TimeUnit.SECONDS); + } + } + + @Test + public void testCancelNestedContext() throws ExecutionException, InterruptedException { + ExecutorService executorService = Executors.newFixedThreadPool(1); + try { + Future future; + CountDownLatch mainThreadLatch = new CountDownLatch(2); + Context[] context = new Context[1]; + context[0] = Context.newBuilder().allowExperimentalOptions(true).build(); + TruffleInstrument.Env instrumentEnv = context[0].getEngine().getInstruments().get("InstrumentationUpdateInstrument").lookup(TruffleInstrument.Env.class); + instrumentEnv.getInstrumenter().attachExecutionEventListener(SourceSectionFilter.newBuilder().tagIs(InstrumentationTestLanguage.ConstantTag.class).build(), new ExecutionEventListener() { + @Override + public void onEnter(EventContext c, VirtualFrame frame) { + } + + @Override + public void onReturnValue(EventContext c, VirtualFrame frame, Object result) { + onReturnValueBoundary(result); + } + + @CompilerDirectives.TruffleBoundary + private void onReturnValueBoundary(Object result) { + mainThreadLatch.countDown(); + if (Integer.valueOf(42).equals(result)) { + context[0].enter(); + try { + /* + * Eval does not enter the polyglot thread for the second time, the + * explicit enter does. In any case, here the wrapped ExitException can + * be caught, or actually must be caught, because PolyglotException must + * not be thrown from inside guest language code. If the host does not + * allow the exit to continue, then we have a problem, but is this + * really a problem? In any case we need some place to catch the + * ExitException in the main thread (as opposed to PolyglotThread) and + * cancel the other threads if context is not entered in the main + * thread. We should probably set exiting only just before we call the + * close. + */ + context[0].eval(InstrumentationTestLanguage.ID, "ROOT(CONSTANT(1),LOOP(infinity, STATEMENT))"); + Assert.fail(); + } catch (PolyglotException pe) { + if (!pe.isCancelled()) { + throw pe; + } + } finally { + context[0].leave(); + } + } + } + + @Override + public void onReturnExceptional(EventContext c, VirtualFrame frame, Throwable exception) { + } + }); + try { + context[0].initialize(InstrumentationTestLanguage.ID); + future = executorService.submit(() -> { + try { + context[0].eval(InstrumentationTestLanguage.ID, "CONSTANT(42)"); + Assert.fail(); + } catch (PolyglotException pe) { + if (!pe.isCancelled()) { + throw pe; + } + } + }); + mainThreadLatch.await(); + context[0].close(true); + future.get(); + } finally { + context[0].close(); + } + } finally { + executorService.shutdownNow(); + executorService.awaitTermination(100, TimeUnit.SECONDS); + } + } + + @Test + public void testMultipleContextDeadlock() throws InterruptedException, ExecutionException { + ExecutorService executorService = Executors.newFixedThreadPool(2); + try (Engine engine = Engine.create()) { + try (Context context1 = Context.newBuilder().engine(engine).allowCreateThread(true).build(); Context context2 = Context.newBuilder().engine(engine).allowCreateThread(true).build()) { + Phaser cancelSignal = new Phaser(2); + Future future1 = executorService.submit(() -> { + context1.enter(); + try { + cancelSignal.arriveAndAwaitAdvance(); + context1.close(true); + cancelSignal.awaitAdvance(cancelSignal.arriveAndDeregister()); + context2.enter(); + context2.leave(); + } catch (PolyglotException pe) { + if (!pe.isCancelled()) { + throw pe; + } + } finally { + context1.leave(); + } + }); + Future future2 = executorService.submit(() -> { + context1.enter(); + try { + cancelSignal.arriveAndAwaitAdvance(); + cancelSignal.awaitAdvance(cancelSignal.arriveAndDeregister()); + context2.enter(); + context2.leave(); + } catch (PolyglotException pe) { + if (!pe.isCancelled()) { + throw pe; + } + } finally { + context1.leave(); + } + + }); + future1.get(); + future2.get(); + } finally { + executorService.shutdownNow(); + executorService.awaitTermination(100, TimeUnit.SECONDS); + } + } + } +} diff --git a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ResourceLimitsTest.java b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ResourceLimitsTest.java index 5992f08055fc..c6ed7abe9a02 100644 --- a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ResourceLimitsTest.java +++ b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/ResourceLimitsTest.java @@ -132,17 +132,18 @@ public void testStatementInvalidLimitFilter() { Context.newBuilder().resourceLimits(limits0).build().close(); Context.newBuilder().resourceLimits(limits1).build().close(); - Engine engine = Engine.create(); - Context.newBuilder().engine(engine).resourceLimits(limits0).build().close(); - Context.Builder builder = Context.newBuilder().engine(engine).resourceLimits(limits1); - try { - builder.build(); - fail(); - } catch (IllegalArgumentException e) { - assertEquals("Using multiple source predicates per engine is not supported. " + - "The same statement limit source predicate must be used for all polyglot contexts that are assigned to the same engine. " + - "Resolve this by using the same predicate instance when constructing the limits object with ResourceLimits.Builder.statementLimit(long, Predicate).", - e.getMessage()); + try (Engine engine = Engine.create()) { + Context.newBuilder().engine(engine).resourceLimits(limits0).build().close(); + Context.Builder builder = Context.newBuilder().engine(engine).resourceLimits(limits1); + try { + builder.build(); + fail(); + } catch (IllegalArgumentException e) { + assertEquals("Using multiple source predicates per engine is not supported. " + + "The same statement limit source predicate must be used for all polyglot contexts that are assigned to the same engine. " + + "Resolve this by using the same predicate instance when constructing the limits object with ResourceLimits.Builder.statementLimit(long, Predicate).", + e.getMessage()); + } } } @@ -197,58 +198,8 @@ public void testStatementLimitDifferentPerContext() { statementLimit(2, null).// build(); Source source = statements(1); - Engine engine = Engine.create(); - - for (int i = 0; i < 10; i++) { - // test no limit - try (Context context = Context.newBuilder().engine(engine).build()) { - context.eval(source); - context.eval(source); - context.eval(source); - } - - // test with limit - try (Context context = Context.newBuilder().engine(engine).resourceLimits(limits1).build()) { - context.eval(source); - try { - context.eval(source); - fail(); - } catch (PolyglotException ex) { - assertStatementCountLimit(context, ex, 1); - } - } - - // test with different limit - try (Context context = Context.newBuilder().engine(engine).resourceLimits(limits2).build()) { - context.eval(source); - context.eval(source); - try { - context.eval(source); - fail(); - } catch (PolyglotException ex) { - assertStatementCountLimit(context, ex, 2); - } - } - } - } - - @Test - public void testStatementLimitDifferentPerContextParallel() throws InterruptedException { - ResourceLimits limits1 = ResourceLimits.newBuilder().// - statementLimit(1, null).// - build(); - ResourceLimits limits2 = ResourceLimits.newBuilder().// - statementLimit(2, null).// - build(); - Source source = statements(1); - Engine engine = Engine.create(); - - ExecutorService executorService = Executors.newFixedThreadPool(20); - List> futures = new ArrayList<>(); - - for (int i = 0; i < 10; i++) { - futures.add(executorService.submit(() -> { - + try (Engine engine = Engine.create()) { + for (int i = 0; i < 10; i++) { // test no limit try (Context context = Context.newBuilder().engine(engine).build()) { context.eval(source); @@ -278,11 +229,66 @@ public void testStatementLimitDifferentPerContextParallel() throws InterruptedEx assertStatementCountLimit(context, ex, 2); } } - })); + } } + } - executorService.shutdown(); - executorService.awaitTermination(100, TimeUnit.SECONDS); + @Test + public void testStatementLimitDifferentPerContextParallel() throws ExecutionException, InterruptedException { + ResourceLimits limits1 = ResourceLimits.newBuilder().// + statementLimit(1, null).// + build(); + ResourceLimits limits2 = ResourceLimits.newBuilder().// + statementLimit(2, null).// + build(); + Source source = statements(1); + + try (Engine engine = Engine.create()) { + ExecutorService executorService = Executors.newFixedThreadPool(20); + List> futures = new ArrayList<>(); + + for (int i = 0; i < 10; i++) { + futures.add(executorService.submit(() -> { + + // test no limit + try (Context context = Context.newBuilder().engine(engine).build()) { + context.eval(source); + context.eval(source); + context.eval(source); + } + + // test with limit + try (Context context = Context.newBuilder().engine(engine).resourceLimits(limits1).build()) { + context.eval(source); + try { + context.eval(source); + fail(); + } catch (PolyglotException ex) { + assertStatementCountLimit(context, ex, 1); + } + } + + // test with different limit + try (Context context = Context.newBuilder().engine(engine).resourceLimits(limits2).build()) { + context.eval(source); + context.eval(source); + try { + context.eval(source); + fail(); + } catch (PolyglotException ex) { + assertStatementCountLimit(context, ex, 2); + } + } + })); + } + + for (Future future : futures) { + future.get(); + } + + executorService.shutdown(); + executorService.awaitTermination(100, TimeUnit.SECONDS); + } } private static Source statements(int count) { @@ -300,66 +306,68 @@ public void testSharedContextStatementLimitSynchronous() { statementLimit(50, null).// onLimit((e) -> events.add(e)).// build(); - Engine engine = Engine.create(); - for (int i = 0; i < 10; i++) { - try (Context c = Context.newBuilder().engine(engine).resourceLimits(limits).build()) { - c.eval(statements(50)); - try { - c.eval(statements(1)); - fail(); - } catch (PolyglotException e) { - assertStatementCountLimit(c, e, 50); - assertEquals(1, events.size()); - assertSame(c, events.iterator().next().getContext()); - assertNotNull(events.iterator().next().toString()); + try (Engine engine = Engine.create()) { + for (int i = 0; i < 10; i++) { + try (Context c = Context.newBuilder().engine(engine).resourceLimits(limits).build()) { + c.eval(statements(50)); + try { + c.eval(statements(1)); + fail(); + } catch (PolyglotException e) { + assertStatementCountLimit(c, e, 50); + assertEquals(1, events.size()); + assertSame(c, events.iterator().next().getContext()); + assertNotNull(events.iterator().next().toString()); + } } + events.clear(); } - events.clear(); } } @Test public void testSharedContextStatementLimitParallel() throws InterruptedException, ExecutionException { - Engine engine = Engine.create(); - Map events = new HashMap<>(); - final int limit = 50; - ResourceLimits limits = ResourceLimits.newBuilder().// - statementLimit(limit, null).// - onLimit((e) -> { + try (Engine engine = Engine.create()) { + Map events = new HashMap<>(); + final int limit = 50; + ResourceLimits limits = ResourceLimits.newBuilder().// + statementLimit(limit, null).// + onLimit((e) -> { + synchronized (events) { + events.put(e.getContext(), e); + } + }).// + build(); + ExecutorService executorService = Executors.newFixedThreadPool(20); + List> futures = new ArrayList<>(); + final int tasks = 1000; + for (int i = 0; i < tasks; i++) { + futures.add(executorService.submit(() -> { + try (Context c = Context.newBuilder().engine(engine).resourceLimits(limits).build()) { + c.eval(statements(limit)); + try { + c.eval(statements(1)); + fail(); + } catch (PolyglotException e) { + assertStatementCountLimit(c, e, limit); synchronized (events) { - events.put(e.getContext(), e); + assertNotNull(events.get(c)); + assertSame(c, events.get(c).getContext()); + assertNotNull(events.get(c).toString()); } - }).// - build(); - ExecutorService executorService = Executors.newFixedThreadPool(20); - List> futures = new ArrayList<>(); - final int tasks = 1000; - for (int i = 0; i < tasks; i++) { - futures.add(executorService.submit(() -> { - try (Context c = Context.newBuilder().engine(engine).resourceLimits(limits).build()) { - c.eval(statements(limit)); - try { - c.eval(statements(1)); - fail(); - } catch (PolyglotException e) { - assertStatementCountLimit(c, e, limit); - synchronized (events) { - assertNotNull(events.get(c)); - assertSame(c, events.get(c).getContext()); - assertNotNull(events.get(c).toString()); } } - } - })); - } - for (Future future : futures) { - future.get(); - } - executorService.shutdown(); - executorService.awaitTermination(100, TimeUnit.SECONDS); - synchronized (events) { - assertEquals(tasks, events.size()); + })); + } + for (Future future : futures) { + future.get(); + } + executorService.shutdown(); + executorService.awaitTermination(100, TimeUnit.SECONDS); + synchronized (events) { + assertEquals(tasks, events.size()); + } } } @@ -405,66 +413,112 @@ public void testParallelContextStatementLimit() throws InterruptedException, Exe } @Test - public void testParallelMultiContextStatementLimit() throws InterruptedException, ExecutionException { - Engine engine = Engine.create(); - Map events = new ConcurrentHashMap<>(); - final int executions = 100; - final int contexts = 100; - final int threads = 20; + public void testParallelContextStatementLimit2() throws InterruptedException, ExecutionException { + Map events = new HashMap<>(); + final int limit = 10000000; ResourceLimits limits = ResourceLimits.newBuilder().// - statementLimit(executions, null).// + statementLimit(limit, null).// onLimit((e) -> { - events.put(e.getContext(), e); + synchronized (events) { + if (events.isEmpty()) { + events.put(e.getContext(), e); + } else { + assertTrue(events.containsKey(e.getContext())); + } + } }).// build(); - ExecutorService executorService = Executors.newFixedThreadPool(threads); - List> testFutures = new ArrayList<>(); - - for (int contextIndex = 0; contextIndex < contexts; contextIndex++) { - Context c = Context.newBuilder().engine(engine).resourceLimits(limits).build(); - forceMultiThreading(executorService, c); - - List> futures = new ArrayList<>(); - for (int i = 0; i < executions; i++) { + ExecutorService executorService = Executors.newFixedThreadPool(20); + List> futures = new ArrayList<>(); + try (Context c = Context.newBuilder().resourceLimits(limits).build()) { + for (int i = 0; i < 20; i++) { futures.add(executorService.submit(() -> { - c.eval(statements(1)); + try { + c.eval(statements(Integer.MAX_VALUE)); + fail(); + } catch (PolyglotException e) { + if (!e.isCancelled() || !e.isResourceExhausted()) { + throw e; + } + } + })); } - testFutures.add(executorService.submit(() -> { - for (Future future : futures) { - try { - future.get(); - } catch (InterruptedException | ExecutionException e1) { - if (e1 instanceof ExecutionException) { - if (e1.getCause() instanceof PolyglotException) { - PolyglotException e = (PolyglotException) e1.getCause(); - if (e.isCancelled()) { - throw new AssertionError("Context was cancelled too early.", e); + for (Future future : futures) { + future.get(); + } + synchronized (events) { + assertNotNull(events.get(c)); + assertSame(c, events.get(c).getContext()); + assertNotNull(events.get(c).toString()); + } + } + executorService.shutdown(); + executorService.awaitTermination(100, TimeUnit.SECONDS); + } + + @Test + public void testParallelMultiContextStatementLimit() throws InterruptedException, ExecutionException { + try (Engine engine = Engine.create()) { + Map events = new ConcurrentHashMap<>(); + final int executions = 100; + final int contexts = 100; + final int threads = 20; + ResourceLimits limits = ResourceLimits.newBuilder().// + statementLimit(executions, null).// + onLimit((e) -> { + events.put(e.getContext(), e); + }).// + build(); + ExecutorService executorService = Executors.newFixedThreadPool(threads); + List> testFutures = new ArrayList<>(); + + for (int contextIndex = 0; contextIndex < contexts; contextIndex++) { + Context c = Context.newBuilder().engine(engine).resourceLimits(limits).build(); + forceMultiThreading(executorService, c); + + List> futures = new ArrayList<>(); + for (int i = 0; i < executions; i++) { + futures.add(executorService.submit(() -> { + c.eval(statements(1)); + })); + } + testFutures.add(executorService.submit(() -> { + for (Future future : futures) { + try { + future.get(); + } catch (InterruptedException | ExecutionException e1) { + if (e1 instanceof ExecutionException) { + if (e1.getCause() instanceof PolyglotException) { + PolyglotException e = (PolyglotException) e1.getCause(); + if (e.isCancelled()) { + throw new AssertionError("Context was cancelled too early.", e); + } } } + throw new RuntimeException(e1); } - throw new RuntimeException(e1); } - } - try { - c.eval(statements(1)); - fail(); - } catch (PolyglotException e) { - assertStatementCountLimit(c, e, executions); - assertNotNull(events.get(c)); - assertSame(c, events.get(c).getContext()); - assertNotNull(events.get(c).toString()); - } - c.close(); - })); - } - for (Future future : testFutures) { - future.get(); - } + try { + c.eval(statements(1)); + fail(); + } catch (PolyglotException e) { + assertStatementCountLimit(c, e, executions); + assertNotNull(events.get(c)); + assertSame(c, events.get(c).getContext()); + assertNotNull(events.get(c).toString()); + } + c.close(); + })); + } + for (Future future : testFutures) { + future.get(); + } - assertEquals(contexts, events.size()); - executorService.shutdown(); - executorService.awaitTermination(100, TimeUnit.SECONDS); + assertEquals(contexts, events.size()); + executorService.shutdown(); + executorService.awaitTermination(100, TimeUnit.SECONDS); + } } private static void forceMultiThreading(ExecutorService executorService, Context c) throws InterruptedException, ExecutionException { @@ -480,67 +534,68 @@ public void run() { @Test public void testParallelMultiContextStatementResetLimit() throws InterruptedException, ExecutionException { - Engine engine = Engine.create(); - Map events = new ConcurrentHashMap<>(); - final int executions = 100; - final int contexts = 100; - final int threads = 20; - ResourceLimits limits = ResourceLimits.newBuilder().// - statementLimit(executions, null).// - onLimit((e) -> { - events.put(e.getContext(), e); - }).// - build(); - ExecutorService executorService = Executors.newFixedThreadPool(threads); - List> testFutures = new ArrayList<>(); - for (int contextIndex = 0; contextIndex < contexts; contextIndex++) { - Context c = Context.newBuilder().engine(engine).resourceLimits(limits).build(); - forceMultiThreading(executorService, c); - List> futures = new ArrayList<>(); - for (int i = 0; i < executions; i++) { - futures.add(executorService.submit(() -> { - c.eval(statements(1)); - })); - } - Future prev = executorService.submit(() -> { - for (Future future : futures) { + try (Engine engine = Engine.create()) { + Map events = new ConcurrentHashMap<>(); + final int executions = 100; + final int contexts = 100; + final int threads = 20; + ResourceLimits limits = ResourceLimits.newBuilder().// + statementLimit(executions, null).// + onLimit((e) -> { + events.put(e.getContext(), e); + }).// + build(); + ExecutorService executorService = Executors.newFixedThreadPool(threads); + List> testFutures = new ArrayList<>(); + for (int contextIndex = 0; contextIndex < contexts; contextIndex++) { + Context c = Context.newBuilder().engine(engine).resourceLimits(limits).build(); + forceMultiThreading(executorService, c); + List> futures = new ArrayList<>(); + for (int i = 0; i < executions; i++) { + futures.add(executorService.submit(() -> { + c.eval(statements(1)); + })); + } + Future prev = executorService.submit(() -> { + for (Future future : futures) { + try { + future.get(); + } catch (InterruptedException | ExecutionException e1) { + throw new RuntimeException(e1); + } + } + c.resetLimits(); + }); + + testFutures.add(executorService.submit(() -> { try { - future.get(); + prev.get(); } catch (InterruptedException | ExecutionException e1) { throw new RuntimeException(e1); } - } - c.resetLimits(); - }); + c.eval(statements(executions)); + try { + c.eval(statements(1)); + fail(); + } catch (PolyglotException e) { + assertStatementCountLimit(c, e, executions); + assertNotNull(events.get(c)); + assertSame(c, events.get(c).getContext()); + assertNotNull(events.get(c).toString()); + } + c.close(); + })); + } + for ( - testFutures.add(executorService.submit(() -> { - try { - prev.get(); - } catch (InterruptedException | ExecutionException e1) { - throw new RuntimeException(e1); - } - c.eval(statements(executions)); - try { - c.eval(statements(1)); - fail(); - } catch (PolyglotException e) { - assertStatementCountLimit(c, e, executions); - assertNotNull(events.get(c)); - assertSame(c, events.get(c).getContext()); - assertNotNull(events.get(c).toString()); - } - c.close(); - })); - } - for ( + Future future : testFutures) { + future.get(); + } - Future future : testFutures) { - future.get(); + assertEquals(contexts, events.size()); + executorService.shutdown(); + executorService.awaitTermination(100, TimeUnit.SECONDS); } - - assertEquals(contexts, events.size()); - executorService.shutdown(); - executorService.awaitTermination(100, TimeUnit.SECONDS); } @Test diff --git a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/TruffleContextTest.java b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/TruffleContextTest.java index 4a6aae858b74..c63fe00a5a84 100644 --- a/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/TruffleContextTest.java +++ b/truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/TruffleContextTest.java @@ -217,6 +217,7 @@ public void testCloseInEntered() { assertSame(getCancelExecutionLocation(e), node); assertEquals("testreason", ((Throwable) e).getMessage()); }); + tc.leave(null, prev); } diff --git a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/LanguageFinalizationFailureTest.java b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/LanguageFinalizationFailureTest.java new file mode 100644 index 000000000000..e435aaa83db5 --- /dev/null +++ b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/LanguageFinalizationFailureTest.java @@ -0,0 +1,151 @@ +/* + * Copyright (c) 2021, 2021, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * The Universal Permissive License (UPL), Version 1.0 + * + * Subject to the condition set forth below, permission is hereby granted to any + * person obtaining a copy of this software, associated documentation and/or + * data (collectively the "Software"), free of charge and under any and all + * copyright rights in the Software, and any and all patent rights owned or + * freely licensable by each licensor hereunder covering either (i) the + * unmodified Software as contributed to or provided by such licensor, or (ii) + * the Larger Works (as defined below), to deal in both + * + * (a) the Software, and + * + * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if + * one is included with the Software each a "Larger Work" to which the Software + * is contributed by such licensors), + * + * without restriction, including without limitation the rights to copy, create + * derivative works of, display, perform, and distribute the Software and make, + * use, sell, offer for sale, import, export, have made, and have sold the + * Software and the Larger Work(s), and to sublicense the foregoing rights on + * either these or other terms. + * + * This license is subject to the following condition: + * + * The above copyright notice and either this complete permission notice or at a + * minimum a reference to the UPL must be included in all copies or substantial + * portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package com.oracle.truffle.api.test.polyglot; + +import java.io.ByteArrayOutputStream; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.graalvm.polyglot.Context; +import org.graalvm.polyglot.PolyglotException; +import org.junit.Assert; +import org.junit.Test; + +import com.oracle.truffle.api.TruffleSafepoint; +import com.oracle.truffle.api.exception.AbstractTruffleException; +import com.oracle.truffle.api.interop.ExceptionType; +import com.oracle.truffle.api.interop.InteropLibrary; +import com.oracle.truffle.api.library.ExportLibrary; +import com.oracle.truffle.api.library.ExportMessage; +import com.oracle.truffle.api.nodes.Node; + +public class LanguageFinalizationFailureTest extends AbstractPolyglotTest { + private static final Node DUMMY_NODE = new Node() { + }; + + @ExportLibrary(InteropLibrary.class) + static final class DummyRuntimeException extends AbstractTruffleException { + private static final long serialVersionUID = 5292066718048069141L; + + DummyRuntimeException(Node location) { + super("Dummy runtime error.", location); + } + + @ExportMessage + @SuppressWarnings("static-method") + ExceptionType getExceptionType() { + return ExceptionType.RUNTIME_ERROR; + } + } + + @Test + public void testFinalizationFailureTruffleException() { + AtomicBoolean disposeCalled = new AtomicBoolean(); + setupEnv(Context.newBuilder(), new ProxyLanguage() { + @Override + protected void finalizeContext(LanguageContext languageContext) { + throw new DummyRuntimeException(DUMMY_NODE); + } + + @Override + protected void disposeContext(LanguageContext languageContext) { + disposeCalled.set(true); + } + }); + try { + context.close(); + Assert.fail(); + } catch (PolyglotException pe) { + if (!"Dummy runtime error.".equals(pe.getMessage())) { + throw pe; + } + Assert.assertFalse(pe.isInternalError()); + } + Assert.assertFalse(disposeCalled.get()); + context.close(true); + Assert.assertTrue(disposeCalled.get()); + } + + @Test + public void testFinalizationFailureCancelException() { + AtomicBoolean disposeCalled = new AtomicBoolean(); + ByteArrayOutputStream outStream = new ByteArrayOutputStream(); + setupEnv(Context.newBuilder().logHandler(outStream), new ProxyLanguage() { + @Override + protected void finalizeContext(LanguageContext languageContext) { + TruffleSafepoint.pollHere(DUMMY_NODE); + Assert.fail(); + } + + @Override + protected void disposeContext(LanguageContext languageContext) { + disposeCalled.set(true); + } + + }); + context.close(true); + Assert.assertTrue(disposeCalled.get()); + Assert.assertTrue(outStream.toString().contains("Exception was thrown while finalizing a polyglot context that is being cancelled. Such exceptions are expected during cancelling.")); + } + + @Test + public void testDisposeFailure() { + AtomicBoolean failDispose = new AtomicBoolean(true); + setupEnv(Context.newBuilder(), new ProxyLanguage() { + @Override + protected void disposeContext(LanguageContext languageContext) { + if (failDispose.get()) { + throw new DummyRuntimeException(DUMMY_NODE); + } + } + }); + try { + context.close(); + Assert.fail(); + } catch (PolyglotException pe) { + if (!"java.lang.IllegalStateException: Guest language code was run during language disposal!".equals(pe.getMessage())) { + throw pe; + } + Assert.assertTrue(pe.isInternalError()); + } + failDispose.set(false); + context.close(); + } +} diff --git a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/LanguageSPITest.java b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/LanguageSPITest.java index 1fd4d4f3a108..304ce2ae5d83 100644 --- a/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/LanguageSPITest.java +++ b/truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/LanguageSPITest.java @@ -241,12 +241,53 @@ public void testContextCloseInsideFromSameThreadCancelExecution() { Engine engine = Engine.create(); langContext = null; Context context = Context.newBuilder(LanguageSPITestLanguage.ID).engine(engine).build(); - eval(context, new Function() { - public Object apply(Env t) { - context.close(true); - return null; + try { + eval(context, new Function() { + public Object apply(Env t) { + context.close(true); + return null; + } + }); + fail(); + } catch (PolyglotException pe) { + if (!pe.isCancelled()) { + throw pe; } - }); + } + engine.close(); + assertEquals(1, langContext.disposeCalled); + } + + @Test + public void testContextCloseInsideFromSameThreadCancelExecutionNestedEval() { + Engine engine = Engine.create(); + langContext = null; + Context context = Context.newBuilder(LanguageSPITestLanguage.ID).engine(engine).build(); + try { + eval(context, new Function() { + public Object apply(Env t) { + try { + eval(context, new Function() { + public Object apply(Env t2) { + context.close(true); + return null; + } + }); + fail(); + } catch (PolyglotException pe) { + if (!pe.isCancelled()) { + throw pe; + } + } + return null; + } + }); + fail(); + } catch (PolyglotException pe) { + if (!pe.isCancelled()) { + throw pe; + } + } engine.close(); assertEquals(1, langContext.disposeCalled); } @@ -271,12 +312,53 @@ public void testEngineCloseInsideFromSameThreadCancelExecution() { Engine engine = Engine.create(); langContext = null; Context context = Context.newBuilder(LanguageSPITestLanguage.ID).engine(engine).build(); - eval(context, new Function() { - public Object apply(Env t) { - engine.close(true); - return null; + try { + eval(context, new Function() { + public Object apply(Env t) { + engine.close(true); + return null; + } + }); + fail(); + } catch (PolyglotException pe) { + if (!pe.isCancelled()) { + throw pe; } - }); + } + assertEquals(1, langContext.disposeCalled); + engine.close(); + } + + @Test + public void testEngineCloseInsideFromSameThreadCancelExecutionNestedEval() { + Engine engine = Engine.create(); + langContext = null; + Context context = Context.newBuilder(LanguageSPITestLanguage.ID).engine(engine).build(); + try { + eval(context, new Function() { + public Object apply(Env t) { + try { + eval(context, new Function() { + public Object apply(Env t2) { + engine.close(true); + return null; + } + }); + fail(); + } catch (PolyglotException pe) { + if (!pe.isCancelled()) { + throw pe; + } + } + return null; + } + }); + fail(); + } catch (PolyglotException pe) { + if (!pe.isCancelled()) { + throw pe; + } + } assertEquals(1, langContext.disposeCalled); engine.close(); } diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/TruffleContext.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/TruffleContext.java index b821204b93ba..9b8a88ac8433 100644 --- a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/TruffleContext.java +++ b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/TruffleContext.java @@ -272,11 +272,11 @@ public boolean isCancelling() { * to be paused. The future is completed when all active threads are paused. New threads entered * after this point are paused immediately after entering until * {@link TruffleContext#resume(Future)} is called. - * + * * @return a future that can be used to wait for the execution to be paused. Also, the future is * used to resume execution by passing it to the {@link TruffleContext#resume(Future)} * method. - * + * * @since 21.2 */ @TruffleBoundary @@ -292,13 +292,13 @@ public Future pause() { * Resume previously paused execution on all threads for this context. The execution will not * resume if {@link TruffleContext#pause()} was called multiple times and for some of the other * calls resume was not called yet. - * + * * @param pauseFuture pause future returned by a previous call to * {@link TruffleContext#pause()}. - * + * * @throws IllegalArgumentException in case the passed pause future was not obtained by a * previous call to {@link TruffleContext#pause()} on this context. - * + * * @since 21.2 */ @TruffleBoundary diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/TruffleLanguage.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/TruffleLanguage.java index 54a224b81da9..d486fa52fa09 100644 --- a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/TruffleLanguage.java +++ b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/TruffleLanguage.java @@ -599,10 +599,13 @@ protected void initializeContext(C context) throws Exception { /** * Performs language context finalization actions that are necessary before language contexts - * are {@link #disposeContext(Object) disposed}. All installed languages must remain usable - * after finalization. The finalization order can be influenced by specifying - * {@link Registration#dependentLanguages() language dependencies}. By default internal - * languages are finalized last, otherwise the default order is unspecified but deterministic. + * are {@link #disposeContext(Object) disposed}. However, in case the underlying polyglot + * context is being cancelled, {@link #disposeContext(Object)} is called even if + * {@link #finalizeContext(Object)} throws a {@link TruffleException} or a {@link ThreadDeath} + * exception. All installed languages must remain usable after finalization. The finalization + * order can be influenced by specifying {@link Registration#dependentLanguages() language + * dependencies}. By default internal languages are finalized last, otherwise the default order + * is unspecified but deterministic. *

* While finalization code is run, other language contexts may become initialized. In such a * case, the finalization order may be non-deterministic and/or not respect the order specified @@ -670,7 +673,10 @@ protected void initializeMultipleContexts() { * resources associated with a context. The context may become unusable after it was disposed. * It is not allowed to run guest language code while disposing a context. Finalization code * should be run in {@link #finalizeContext(Object)} instead. Finalization will be performed - * prior to context {@link #disposeContext(Object) disposal}. + * prior to context {@link #disposeContext(Object) disposal}. However, in case the underlying + * polyglot context is being cancelled, {@link #disposeContext(Object)} is called even if + * {@link #finalizeContext(Object)} throws {@link TruffleException} or {@link ThreadDeath} + * exception.. *

* The disposal order can be influenced by specifying {@link Registration#dependentLanguages() * language dependencies}. By default internal languages are disposed last, otherwise the diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/DefaultCallTarget.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/DefaultCallTarget.java index 62accdd0c95f..8eb6dd87f62c 100644 --- a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/DefaultCallTarget.java +++ b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/DefaultCallTarget.java @@ -82,16 +82,14 @@ Object callDirectOrIndirect(final Node callNode, Object... args) { final DefaultVirtualFrame frame = new DefaultVirtualFrame(rootNode.getFrameDescriptor(), args); DefaultFrameInstance callerFrame = getRuntime().pushFrame(frame, this, callNode); try { - return rootNode.execute(frame); + Object toRet = rootNode.execute(frame); + TruffleSafepoint.poll(rootNode); + return toRet; } catch (Throwable t) { DefaultRuntimeAccessor.LANGUAGE.onThrowable(callNode, this, t, frame); throw t; } finally { - try { - TruffleSafepoint.poll(rootNode); - } finally { - getRuntime().popFrame(callerFrame); - } + getRuntime().popFrame(callerFrame); } } diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/ThreadLocalHandshake.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/ThreadLocalHandshake.java index 1d9caa16f66d..ce6d4abb140e 100644 --- a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/ThreadLocalHandshake.java +++ b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/ThreadLocalHandshake.java @@ -107,7 +107,19 @@ public void testSupport() { public final > Future runThreadLocal(Thread[] threads, T onThread, Consumer onDone, boolean sideEffecting, boolean syncStartOfEvent, boolean syncEndOfEvent) { testSupport(); + assert threads.length > 0; Handshake handshake = new Handshake<>(threads, onThread, onDone, sideEffecting, threads.length, syncStartOfEvent, syncEndOfEvent); + if (syncStartOfEvent || syncEndOfEvent) { + synchronized (ThreadLocalHandshake.class) { + addHandshakes(threads, handshake); + } + } else { + addHandshakes(threads, handshake); + } + return handshake; + } + + private > void addHandshakes(Thread[] threads, Handshake handshake) { for (int i = 0; i < threads.length; i++) { Thread t = threads[i]; if (!t.isAlive()) { @@ -115,7 +127,6 @@ public final > Future runThreadLocal(Thread[] thr } getThreadState(t).addHandshake(t, handshake); } - return handshake; } @SuppressWarnings("static-method") diff --git a/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/GuestToHostRootNode.java b/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/GuestToHostRootNode.java index 19c685a7e847..66e5fdb3d5c9 100644 --- a/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/GuestToHostRootNode.java +++ b/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/GuestToHostRootNode.java @@ -74,7 +74,6 @@ public final String getName() { return boundaryName; } - @SuppressWarnings("deprecation") @Override public Object execute(VirtualFrame frame) { Object[] arguments = frame.getArguments(); diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/EngineAccessor.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/EngineAccessor.java index f974424dba2d..eb5e06bb5e47 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/EngineAccessor.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/EngineAccessor.java @@ -93,6 +93,7 @@ import com.oracle.truffle.api.TruffleLanguage; import com.oracle.truffle.api.TruffleLanguage.Env; import com.oracle.truffle.api.TruffleLogger; +import com.oracle.truffle.api.TruffleSafepoint; import com.oracle.truffle.api.frame.Frame; import com.oracle.truffle.api.impl.Accessor; import com.oracle.truffle.api.impl.TruffleLocator; @@ -726,7 +727,7 @@ public Object enterInternalContext(Node location, Object polyglotLanguageContext useLocation = engine.getUncachedLocation(); } if (CompilerDirectives.isPartialEvaluationConstant(engine)) { - return engine.enter(context, useLocation, true); + return engine.enter(context, true, useLocation, true, false); } else { return enterInternalContextBoundary(context, useLocation, engine); } @@ -734,7 +735,7 @@ public Object enterInternalContext(Node location, Object polyglotLanguageContext @TruffleBoundary private static Object enterInternalContextBoundary(PolyglotContextImpl context, Node location, PolyglotEngineImpl engine) { - return engine.enter(context, location, true); + return engine.enter(context, true, location, true, false); } @Override @@ -743,7 +744,7 @@ public void leaveInternalContext(Node node, Object impl, Object prev) { PolyglotContextImpl context = ((PolyglotContextImpl) impl); PolyglotEngineImpl engine = resolveEngine(node, context); if (CompilerDirectives.isPartialEvaluationConstant(engine)) { - engine.leave((PolyglotContextImpl) prev, context); + engine.leave((PolyglotContextImpl) prev, context, true); } else { leaveInternalContextBoundary(prev, context, engine); } @@ -751,7 +752,7 @@ public void leaveInternalContext(Node node, Object impl, Object prev) { @TruffleBoundary private static void leaveInternalContextBoundary(Object prev, PolyglotContextImpl context, PolyglotEngineImpl engine) { - engine.leave((PolyglotContextImpl) prev, context); + engine.leave((PolyglotContextImpl) prev, context, true); } private static PolyglotEngineImpl resolveEngine(Node node, PolyglotContextImpl context) { @@ -783,6 +784,9 @@ public TruffleContext createInternalContext(Object sourcePolyglotLanguageContext PolyglotContextImpl impl; synchronized (creator.context) { impl = new PolyglotContextImpl(creator, config); + creator.context.engine.noInnerContexts.invalidate(); + creator.context.addChildContext(impl); + PolyglotContextImpl.initializeStaticContext(impl); impl.api = creator.getImpl().getAPIAccess().newContext(creator.getImpl().contextDispatch, impl, creator.context.engine.api); } synchronized (impl) { @@ -1308,16 +1312,15 @@ public OptionValues getInstrumentContextOptions(Object polyglotInstrument, Objec @Override public boolean isContextClosed(Object polyglotContext) { PolyglotContextImpl context = ((PolyglotContextImpl) polyglotContext); - if (context.invalid && context.closingThread != Thread.currentThread()) { - return true; - } - return context.closed; + PolyglotContextImpl.State localContextState = context.state; + return localContextState.isInvalidOrClosed(); } @Override public boolean isContextCancelling(Object polyglotContext) { PolyglotContextImpl context = ((PolyglotContextImpl) polyglotContext); - return context.cancelling; + PolyglotContextImpl.State localContextState = context.state; + return localContextState.isCancelling(); } @Override @@ -1350,9 +1353,9 @@ public void closeContext(Object impl, boolean force, Node closeLocation, boolean throw PolyglotEngineException.illegalState(String.format("The context is currently active on the current thread but another different context is entered as top-most context. " + "Leave or close the top-most context first or close the context on a separate thread to resolve this problem.")); } - context.cancel(resourceExhaused, resourceExhausedReason, !entered); + context.cancel(resourceExhaused, resourceExhausedReason); if (entered) { - throw context.createCancelException(closeLocation); + TruffleSafepoint.pollHere(closeLocation != null ? closeLocation : context.engine.getUncachedLocation()); } } else { synchronized (context) { @@ -1361,7 +1364,8 @@ public void closeContext(Object impl, boolean force, Node closeLocation, boolean * context which could lead to the following IllegalStateException if the * cancelling flag was not checked. */ - if (context.isActiveNotCancelled(false) && !context.cancelling) { + PolyglotContextImpl.State localContextState = context.state; + if (context.isActiveNotCancelled(false) && !localContextState.isCancelling()) { /* * Polyglot threads are still allowed to run at this point. They are * required to be finished after finalizeContext. @@ -1369,7 +1373,8 @@ public void closeContext(Object impl, boolean force, Node closeLocation, boolean throw new IllegalStateException("The context is currently active and cannot be closed. Make sure no thread is running or call closeCancelled on the context to resolve this."); } } - context.closeImpl(false, false, true); + context.closeImpl(true); + context.finishCleanup(); } } diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/HostToGuestRootNode.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/HostToGuestRootNode.java index 4cd305428f2f..2b2c53f5c4d1 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/HostToGuestRootNode.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/HostToGuestRootNode.java @@ -107,7 +107,7 @@ public final Object execute(VirtualFrame frame) { CompilerDirectives.transferToInterpreterAndInvalidate(); seenEnter = true; } - prev = engine.enter(context, this, true); + prev = engine.enter(context, true, this, true, false); } else { if (!seenNonEnter) { CompilerDirectives.transferToInterpreterAndInvalidate(); @@ -130,7 +130,7 @@ public final Object execute(VirtualFrame frame) { } finally { if (needsEnter) { try { - engine.leave(prev, context); + engine.leave(prev, context, true); } catch (Throwable e) { throw handleException(languageContext, e, false, RuntimeException.class); } diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PauseThreadLocalAction.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PauseThreadLocalAction.java index ecc247db7dc6..d4e21876e0d8 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PauseThreadLocalAction.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PauseThreadLocalAction.java @@ -71,7 +71,8 @@ protected void perform(ThreadLocalAction.Access access) { @Override public void apply(Object waitObject) throws InterruptedException { synchronized (waitObject) { - while (pause && !context.closed && !context.cancelling) { + PolyglotContextImpl.State localContextState = context.state; + while (pause && !localContextState.isClosed() && !localContextState.isCancelling()) { waitObject.wait(); } } diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotContextImpl.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotContextImpl.java index 2b4730d20345..7986fb6f4228 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotContextImpl.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotContextImpl.java @@ -56,6 +56,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.EnumMap; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashSet; @@ -66,7 +67,12 @@ import java.util.Set; import java.util.WeakHashMap; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.locks.ReentrantLock; @@ -182,23 +188,198 @@ static boolean isSingleContextAssumptionValid() { volatile PolyglotThreadInfo cachedThreadInfo = PolyglotThreadInfo.NULL; volatile Context api; - volatile boolean interrupting; + + private static final Map VALID_TRANSITIONS = new EnumMap<>(State.class); + + static { + VALID_TRANSITIONS.put(State.DEFAULT, new State[]{ + State.CLOSING, + State.INTERRUPTING, + State.CANCELLING + }); + VALID_TRANSITIONS.put(State.CLOSING, new State[]{ + State.CLOSED, + State.CLOSING_INTERRUPTING, + State.CLOSING_CANCELLING, + State.DEFAULT + }); + VALID_TRANSITIONS.put(State.INTERRUPTING, new State[]{ + State.DEFAULT, + State.CLOSING_INTERRUPTING, + State.CANCELLING, + }); + VALID_TRANSITIONS.put(State.CANCELLING, new State[]{ + State.CLOSING_CANCELLING + }); + VALID_TRANSITIONS.put(State.CLOSING_INTERRUPTING, new State[]{ + State.CLOSED, + State.CLOSING, + State.CLOSING_CANCELLING, + State.INTERRUPTING + }); + VALID_TRANSITIONS.put(State.CLOSING_CANCELLING, new State[]{ + State.CLOSED_CANCELLED, + State.CANCELLING + }); + VALID_TRANSITIONS.put(State.CLOSED, + new State[0]); + VALID_TRANSITIONS.put(State.CLOSED_CANCELLED, + new State[0]); + } + + enum State { + /* + * Initial state. Context is valid and ready for use. + */ + DEFAULT, + /* + * Interrupt operation has been started. Threads are being interrupted. + */ + INTERRUPTING, + /* + * Cancel operation has been initiated. Threads are being stopped. The CANCELLING state + * overrides the INTERRUPTING state. + */ + CANCELLING, + /* + * Close operation has been initiated in the DEFAULT state, or it has been initiated in the + * INTERRUPTING state and the interrupt operation stopped during closing. The thread that + * initiated the operation is stored in the closingThread field. The close operation either + * finishes successfully and the context goes into one of the closed states, or the close + * operation fails and the context goes back to the DEFAULT state. + */ + CLOSING, + /* + * Close operation has been initiated in the INTERRUPTING state and the interrupt operation + * is still in progress, or it has been initiated in the DEFAULT state and the interrupt + * operation started during closing, i.e., the transition to this state can either be from + * the CLOSING or the INTERRUPTING state. Even if the transition is from the CLOSING state + * the closingThread is still the one that initiated the close operation, not the one that + * initiated the interrupt operation. The close operation either finishes successfully and + * the context goes into one of the closed states, or the close operation fails and the + * context goes back to the INTERRUPTING state. + */ + CLOSING_INTERRUPTING, + /* + * Close operation has been initiated and at the same time the cancel operation is in + * progress. Transition to this state can either be from the CLOSING, the CANCELLING, or the + * CLOSING_INTERRUPTING state. Even if the transition is from one of the closing states the + * closingThread is still the one that initiated the close operation. The CLOSING_CANCELLING + * state overrides the CLOSING and the CLOSING_INTERRUPTING states. Close operation that + * started in the CLOSING_CANCELLING state must finish successfully, otherwise it is an + * internal error. Close operation that did not start in the CLOSING_CANCELLING state and + * the state was overridden by CLOSING_CANCELLING during the operation can fail in which + * case the state goes back to CANCELLING. + */ + CLOSING_CANCELLING, + /* + * Closing operation in the CLOSING or the CLOSING_INTERRUPTING state has finished + * successfully. + */ + CLOSED, + /* + * Closing operation in the CLOSING_CANCELLING state has finished successfully. + */ + CLOSED_CANCELLED; + + /* + * The context is not usable and may be in an inconsistent state. + */ + boolean isInvalidOrClosed() { + switch (this) { + case CANCELLING: + case CLOSING_CANCELLING: + case CLOSED: + case CLOSED_CANCELLED: + return true; + default: + return false; + } + } + + boolean isInterrupting() { + switch (this) { + case INTERRUPTING: + case CLOSING_INTERRUPTING: + return true; + default: + return false; + } + } + + boolean isCancelling() { + switch (this) { + case CANCELLING: + case CLOSING_CANCELLING: + return true; + default: + return false; + } + } + + boolean isClosing() { + switch (this) { + case CLOSING: + case CLOSING_INTERRUPTING: + case CLOSING_CANCELLING: + return true; + default: + return false; + } + } + + boolean isClosed() { + switch (this) { + case CLOSED: + case CLOSED_CANCELLED: + return true; + default: + return false; + } + } + + private boolean shouldCacheThreadInfo() { + switch (this) { + case DEFAULT: + return true; + default: + return false; + } + } + } + + volatile State state = State.DEFAULT; /* - * While canceling the context can no longer be entered. The context goes from canceling into - * closed state. + * Used only in asserts. */ - volatile boolean cancelling; - volatile boolean cancelled; - volatile String invalidMessage; - volatile boolean invalidResourceLimit; + private boolean isTransitionAllowed(State fromState, State toState) { + assert Thread.holdsLock(this); + State[] successors = VALID_TRANSITIONS.get(fromState); + for (State successor : successors) { + if (successor == toState) { + return isAdditionalTransitionConditionSatisfied(fromState, toState); + } + } + return false; + } + + private boolean isAdditionalTransitionConditionSatisfied(State fromState, State toState) { + assert Thread.holdsLock(this); + if (fromState.isClosing() && !toState.isClosing()) { + return closingThread == Thread.currentThread(); + } + return true; + } + + private ExecutorService cleanupExecutorService; + private Future cleanupFuture; + private volatile String invalidMessage; + private volatile boolean invalidResourceLimit; volatile Thread closingThread; private final ReentrantLock closingLock = new ReentrantLock(); - /* - * If the context is closed all operations should fail with IllegalStateException. - */ - volatile boolean closed; - volatile boolean invalid; + private final ReentrantLock interruptingLock = new ReentrantLock(); + volatile boolean disposing; final PolyglotEngineImpl engine; @CompilationFinal(dimensions = 1) final PolyglotLanguageContext[] contexts; @@ -319,13 +500,14 @@ private PolyglotContextImpl() { this.creatorArguments = langConfig; this.statementLimit = 0; // inner context limit must not be used anyway this.weakReference = new ContextWeakReference(this); - this.parent.addChildContext(this); this.creatorTruffleContext = EngineAccessor.LANGUAGE.createTruffleContext(this, true); this.currentTruffleContext = EngineAccessor.LANGUAGE.createTruffleContext(this, false); - this.interrupting = parent.interrupting; - this.invalid = this.parent.invalid; + if (parent.state.isInterrupting()) { + this.state = State.INTERRUPTING; + } else if (parent.state.isCancelling()) { + this.state = State.CANCELLING; + } this.invalidMessage = this.parent.invalidMessage; - this.cancelling = this.parent.cancelling; this.contextBoundLoggers = this.parent.contextBoundLoggers; this.threadLocalActions = new PolyglotThreadLocalActions(this); if (!parent.config.logLevels.isEmpty()) { @@ -335,8 +517,6 @@ private PolyglotContextImpl() { this.contexts = createContextArray(engine.hostLanguageInstance); this.subProcesses = new HashSet<>(); // notifyContextCreated() is called after spiContext.impl is set to this. - this.engine.noInnerContexts.invalidate(); - initializeStaticContext(this); } OptionValues getInstrumentContextOptions(PolyglotInstrument instrument) { @@ -412,7 +592,7 @@ static void disposeStaticContext(PolyglotContextImpl context) { if (state.singleContextAssumption.isValid()) { synchronized (state) { if (state.singleContextAssumption.isValid()) { - assert state.singleContext == context; + assert state.singleContext == null || state.singleContext == context; state.singleContext = null; } } @@ -465,8 +645,9 @@ void notifyContextCreated() { EngineAccessor.INSTRUMENT.notifyContextCreated(engine, creatorTruffleContext); } - private synchronized void addChildContext(PolyglotContextImpl child) { - if (closingThread != null) { + synchronized void addChildContext(PolyglotContextImpl child) { + assert !state.isClosed(); + if (state.isClosing()) { throw PolyglotEngineException.illegalState("Adding child context into a closing context."); } childContexts.add(child); @@ -518,7 +699,7 @@ static PolyglotContextImpl requireContext() { public synchronized void explicitEnter() { try { - PolyglotContextImpl prev = engine.enter(this, engine.getUncachedLocation(), true); + PolyglotContextImpl prev = engine.enter(this, true, engine.getUncachedLocation(), true, false); PolyglotThreadInfo current = getCachedThreadInfo(); assert current.getThread() == Thread.currentThread(); current.explicitContextStack.addLast(prev); @@ -528,9 +709,11 @@ public synchronized void explicitEnter() { } public synchronized void explicitLeave() { - if (closed || closingThread == Thread.currentThread()) { - // explicit leaves if already closed are allowed. - // as close may automatically leave the context on threads. + if (state.isClosed()) { + /* + * closeImpl leaves automatically for all explicit enters on the closingThread, so + * nothing else needs to be done if context is already closed. + */ return; } try { @@ -539,7 +722,7 @@ public synchronized void explicitLeave() { if (stack.isEmpty() || current.getThread() == null) { throw PolyglotEngineException.illegalState("The context is not entered explicity. A context can only be left if it was previously entered."); } - engine.leave(stack.removeLast(), this); + engine.leave(stack.removeLast(), this, true); } catch (Throwable t) { throw PolyglotImpl.guestToHostException(engine, t); } @@ -547,7 +730,7 @@ public synchronized void explicitLeave() { synchronized Future pause() { PauseThreadLocalAction pauseAction = new PauseThreadLocalAction(this); - Future future = threadLocalActions.submit(null, PolyglotEngineImpl.ENGINE_ID, pauseAction, true, true, false); + Future future = threadLocalActions.submit(null, PolyglotEngineImpl.ENGINE_ID, pauseAction, true, true, false, false); pauseThreadLocalActions.add(pauseAction); return new ContextPauseHandle(pauseAction, future); } @@ -562,7 +745,7 @@ void resume(Future pauseFuture) { } @TruffleBoundary - PolyglotContextImpl enterThreadChanged(Node safepointLocation, boolean enterReverted, boolean pollSafepoint) { + PolyglotContextImpl enterThreadChanged(boolean notifyEnter, Node safepointLocation, boolean enterReverted, boolean pollSafepoint, boolean deactivateSafepoints) { PolyglotThreadInfo enteredThread = null; PolyglotContextImpl prev = null; try { @@ -572,14 +755,17 @@ PolyglotContextImpl enterThreadChanged(Node safepointLocation, boolean enterReve PolyglotThreadInfo threadInfo = getCachedThreadInfo(); if (enterReverted && threadInfo.getEnteredCount() == 0) { threadLocalActions.notifyThreadActivation(threadInfo, false); - if ((cancelling || cancelled) && !threadInfo.isActiveNotCancelled()) { - notifyThreadClosed(); + if ((state.isCancelling() || state == State.CLOSED_CANCELLED) && !threadInfo.isActive()) { + notifyThreadClosed(threadInfo); } - if (interrupting && !threadInfo.isActiveNotCancelled()) { + if (state.isInterrupting() && !threadInfo.isActive()) { Thread.interrupted(); notifyAll(); } } + if (deactivateSafepoints && threadInfo != PolyglotThreadInfo.NULL) { + threadLocalActions.notifyThreadActivation(threadInfo, false); + } checkClosed(); assert threadInfo != null; @@ -588,6 +774,15 @@ PolyglotContextImpl enterThreadChanged(Node safepointLocation, boolean enterReve threadInfo = createThreadInfo(current); needsInitialization = true; } + if (singleThreaded.isValid()) { + /* + * If this is the only thread, then setting the cached thread info to NULL is no + * performance problem. If there is other thread that is just about to enter, we + * are making sure that it initializes multi-threading if this thread doesn't do + * it. + */ + setCachedThreadInfo(PolyglotThreadInfo.NULL); + } boolean transitionToMultiThreading = singleThreaded.isValid() && hasActiveOtherThread(true); if (transitionToMultiThreading) { // recheck all thread accesses @@ -600,13 +795,11 @@ PolyglotContextImpl enterThreadChanged(Node safepointLocation, boolean enterReve * local initialization depends on single thread per context. */ engine.singleThreadPerContext.invalidate(); + assert singleThreaded.isValid(); + singleThreaded.invalidate(); } - Thread closing = this.closingThread; if (needsInitialization) { - if (closing != null && closing != current) { - throw PolyglotEngineException.illegalState("Can not create new threads in closing context.", true); - } threads.put(current, threadInfo); } @@ -619,11 +812,13 @@ PolyglotContextImpl enterThreadChanged(Node safepointLocation, boolean enterReve } prev = threadInfo.enterInternal(); - try { - threadInfo.notifyEnter(engine, this); - } catch (Throwable t) { - threadInfo.leaveInternal(prev); - throw t; + if (notifyEnter) { + try { + threadInfo.notifyEnter(engine, this); + } catch (Throwable t) { + threadInfo.leaveInternal(prev); + throw t; + } } enteredThread = threadInfo; @@ -634,7 +829,7 @@ PolyglotContextImpl enterThreadChanged(Node safepointLocation, boolean enterReve // new thread became active so we need to check potential active thread local // actions and process them. Set activatedActions = null; - if (enteredThread.getEnteredCount() == 1) { + if (enteredThread.getEnteredCount() == 1 && !deactivateSafepoints) { activatedActions = threadLocalActions.notifyThreadActivation(threadInfo, true); } @@ -655,7 +850,7 @@ PolyglotContextImpl enterThreadChanged(Node safepointLocation, boolean enterReve threadLocalActionIterator.remove(); } else { if (activatedActions == null || !activatedActions.contains(threadLocalAction)) { - threadLocalActions.submit(new Thread[]{Thread.currentThread()}, PolyglotEngineImpl.ENGINE_ID, threadLocalAction, true, true, false); + threadLocalActions.submit(new Thread[]{Thread.currentThread()}, PolyglotEngineImpl.ENGINE_ID, threadLocalAction, true, true, false, false); } } } @@ -684,7 +879,7 @@ PolyglotContextImpl enterThreadChanged(Node safepointLocation, boolean enterReve * again. */ if (enteredThread != null) { - engine.leave(prev, this); + engine.leave(prev, this, notifyEnter); } throw t; } @@ -694,7 +889,7 @@ PolyglotContextImpl enterThreadChanged(Node safepointLocation, boolean enterReve void setCachedThreadInfo(PolyglotThreadInfo info) { assert Thread.holdsLock(this); - if (closed || closingThread != null || invalid || interrupting) { + if (!state.shouldCacheThreadInfo()) { // never set the cached thread when closed closing or invalid cachedThreadInfo = PolyglotThreadInfo.NULL; } else { @@ -739,7 +934,7 @@ private void checkAllThreadAccesses(Thread enteringThread, boolean singleThread) } @TruffleBoundary - PolyglotThreadInfo leaveThreadChanged(PolyglotContextImpl prev, boolean entered) { + PolyglotThreadInfo leaveThreadChanged(PolyglotContextImpl prev, boolean notifyLeft, boolean entered) { PolyglotThreadInfo info; synchronized (this) { Thread current = Thread.currentThread(); @@ -751,7 +946,9 @@ PolyglotThreadInfo leaveThreadChanged(PolyglotContextImpl prev, boolean entered) if (entered) { try { - info.notifyLeave(engine, this); + if (notifyLeft) { + info.notifyLeave(engine, this); + } } finally { info.leaveInternal(prev); } @@ -760,8 +957,8 @@ PolyglotThreadInfo leaveThreadChanged(PolyglotContextImpl prev, boolean entered) threadLocalActions.notifyThreadActivation(threadInfo, false); } - if ((cancelling || cancelled) && !info.isActiveNotCancelled()) { - notifyThreadClosed(); + if ((state.isCancelling() || state == State.CLOSED_CANCELLED) && !info.isActive()) { + notifyThreadClosed(info); } boolean somePauseThreadLocalActionIsActive = false; @@ -776,13 +973,31 @@ PolyglotThreadInfo leaveThreadChanged(PolyglotContextImpl prev, boolean entered) } } - if (!closed && !cancelling && !invalid && !interrupting && !somePauseThreadLocalActionIsActive) { + if (entered && state.shouldCacheThreadInfo() && !somePauseThreadLocalActionIsActive) { + /* + * Must not cache thread info when this synchronized leave was called as a slow-path + * fallback (entered == false). The slow-path fallback does not perform enteredCount + * decrement and so other threads may see this thread as already left before the + * synchronized block is entered. If we cached the thread info in this case, then a + * subsequent fast-path enter would not perform operations that might be necessary, + * e.g. initialize multithreading. + */ setCachedThreadInfo(threadInfo); } - if (interrupting && !info.isActiveNotCancelled()) { + + if (state.isInterrupting() && !info.isActive()) { Thread.interrupted(); notifyAll(); } + + if (state.isClosed() && engine.boundEngine && !isActive()) { + /* + * Context can be closed form an entered thread in which case the context must not + * be disposed during closing, but only when the thread leaves the context because + * it is needed up to that point. + */ + disposeStaticContext(this); + } } return info; } @@ -806,7 +1021,6 @@ long getStatementsExecuted() { } private void transitionToMultiThreaded() { - assert singleThreaded.isValid(); assert Thread.holdsLock(this); for (PolyglotLanguageContext context : contexts) { @@ -814,7 +1028,6 @@ private void transitionToMultiThreaded() { LANGUAGE.initializeMultiThreading(context.env); } } - singleThreaded.invalidate(); long statementsExecuted = statementLimit - statementCounter; volatileStatementCounter.getAndAdd(-statementsExecuted); @@ -936,13 +1149,13 @@ public Object getPolyglotBindingsObject() { } void checkClosed() { - if (invalid && closingThread != Thread.currentThread() && invalidMessage != null) { + if (state.isInvalidOrClosed() && closingThread != Thread.currentThread() && invalidMessage != null) { /* * If invalidMessage == null, then invalid flag was set by close. */ throw createCancelException(null); } - if (closed) { + if (state.isClosed()) { throw PolyglotEngineException.illegalState("The Context is already closed."); } } @@ -1187,40 +1400,86 @@ public void close(boolean cancelIfExecuting) { * Cancel does invalidate. We always need to invalidate before force-closing a * context that might be active in other threads. */ - cancel(false, null, true); + cancel(false, null); } else { - closeAndMaybeWait(false); + closeAndMaybeWait(false, null); } } catch (Throwable t) { - throw PolyglotImpl.guestToHostException(engine, t); + throw PolyglotImpl.guestToHostException(getHostContext(), t, false); } } - void cancel(boolean resourceLimit, String message, boolean wait) { - boolean invalidated = invalidateAll(resourceLimit, message == null ? "Context execution was cancelled." : message); - if (wait && invalidated && !closed) { - closeAndMaybeWait(true); + void cancel(boolean resourceLimit, String message) { + if (!state.isClosed()) { + List> futures = setCancelling(resourceLimit, message == null ? "Context execution was cancelled." : message); + closeHereOrCancelInCleanupThread(futures); } } - void closeAndMaybeWait(boolean cancelIfExecuting) { - boolean closeCompleted = closeImpl(cancelIfExecuting, cancelIfExecuting, true); - if (cancelIfExecuting && !closeCompleted) { - engine.cancel(Arrays.asList(this)); - } else if (!closeCompleted) { - throw PolyglotEngineException.illegalState(String.format("The context is currently executing on another thread. " + - "Set cancelIfExecuting to true to stop the execution on this thread.")); + private void closeAndMaybeWait(boolean force, List> futures) { + if (force) { + PolyglotEngineImpl.cancel(this, futures); + } else { + boolean closeCompleted = closeImpl(true); + if (!closeCompleted) { + throw PolyglotEngineException.illegalState(String.format("The context is currently executing on another thread. " + + "Set cancelIfExecuting to true to stop the execution on this thread.")); + } } + finishCleanup(); checkSubProcessFinished(); if (engine.boundEngine && parent == null) { - engine.ensureClosed(cancelIfExecuting, false); + engine.ensureClosed(force, false); + } + } + + private void setState(State targetState) { + assert Thread.holdsLock(this); + assert isTransitionAllowed(state, targetState); + state = targetState; + notifyAll(); + } + + private List> setInterrupting() { + assert Thread.holdsLock(this); + State targetState; + List> futures = new ArrayList<>(); + if (!state.isInterrupting() && !state.isInvalidOrClosed()) { + switch (state) { + case CLOSING: + targetState = State.CLOSING_INTERRUPTING; + break; + default: + targetState = State.INTERRUPTING; + break; + } + setState(targetState); + setCachedThreadInfo(PolyglotThreadInfo.NULL); + futures.add(threadLocalActions.submit(null, PolyglotEngineImpl.ENGINE_ID, new InterruptThreadLocalAction(), true)); + } + return futures; + } + + private void unsetInterrupting() { + assert Thread.holdsLock(this); + if (state.isInterrupting()) { + State targetState; + switch (state) { + case CLOSING_INTERRUPTING: + targetState = State.CLOSING; + break; + default: + targetState = State.DEFAULT; + break; + } + setState(targetState); } } private void finishInterruptForChildContexts() { PolyglotContextImpl[] childContextsToInterrupt; synchronized (this) { - interrupting = false; + unsetInterrupting(); childContextsToInterrupt = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); } for (PolyglotContextImpl childCtx : childContextsToInterrupt) { @@ -1228,20 +1487,25 @@ private void finishInterruptForChildContexts() { } } - private void interruptChildContexts() { - PolyglotContextImpl[] childContextsToInterrupt; + List> interruptChildContexts() { + PolyglotContextImpl[] childContextsToInterrupt = null; + List> futures; synchronized (this) { PolyglotThreadInfo info = getCachedThreadInfo(); if (info != PolyglotThreadInfo.NULL && info.isActive()) { throw PolyglotEngineException.illegalState("Cannot interrupt context from a thread where its child context is active."); } - interrupting = true; - setCachedThreadInfo(PolyglotThreadInfo.NULL); - childContextsToInterrupt = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); + futures = new ArrayList<>(setInterrupting()); + if (!futures.isEmpty()) { + childContextsToInterrupt = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); + } } - for (PolyglotContextImpl childCtx : childContextsToInterrupt) { - childCtx.interruptChildContexts(); + if (childContextsToInterrupt != null) { + for (PolyglotContextImpl childCtx : childContextsToInterrupt) { + futures.addAll(childCtx.interruptChildContexts()); + } } + return futures; } public boolean interrupt(Duration timeout) { @@ -1249,74 +1513,59 @@ public boolean interrupt(Duration timeout) { if (parent != null) { throw PolyglotEngineException.illegalState("Cannot interrupt inner context separately."); } - engine.neverInterrupted.invalidate(); long startMillis = System.currentTimeMillis(); - PolyglotContextImpl[] childContextsToInterrupt; - boolean waitForCloseOrInterrupt = false; - while (true) { - if (waitForCloseOrInterrupt) { - closingLock.lock(); - closingLock.unlock(); - waitForCloseOrInterrupt = false; - } + PolyglotContextImpl[] childContextsToInterrupt = null; + /* + * Two interrupt operations cannot be simultaneously in progress in the whole context + * hierarchy. Inner contexts cannot use interrupt separately and outer context use + * exclusive lock. + */ + interruptingLock.lock(); + try { + List> futures; synchronized (this) { - if (closed) { + if (state.isClosed()) { // already closed return true; } - if (interrupting) { - // currently interrupting on another thread -> wait for other thread to - // complete interrupting - waitForCloseOrInterrupt = true; - continue; - } - Thread localClosingThread = closingThread; - if (localClosingThread != null) { - if (localClosingThread == Thread.currentThread()) { - // interrupt was invoked as a part of closing -> just complete - return true; - } else { - // currently closing on another thread -> wait for other thread to - // complete closing - waitForCloseOrInterrupt = true; - continue; - } - } PolyglotThreadInfo info = getCachedThreadInfo(); if (info != PolyglotThreadInfo.NULL && info.isActive()) { throw PolyglotEngineException.illegalState("Cannot interrupt context from a thread where the context is active."); } - interrupting = true; - setCachedThreadInfo(PolyglotThreadInfo.NULL); - childContextsToInterrupt = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); - /* - * Two interrupt operations cannot be simultaneously in progress in the whole - * context hierarchy. Inner contexts cannot use interrupt separately and outer - * context use exclusive lock that is shared with close. - */ - closingLock.lock(); - break; + futures = new ArrayList<>(setInterrupting()); + if (!futures.isEmpty()) { + childContextsToInterrupt = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); + } } - } - try { - for (PolyglotContextImpl childCtx : childContextsToInterrupt) { - childCtx.interruptChildContexts(); + if (childContextsToInterrupt != null) { + for (PolyglotContextImpl childCtx : childContextsToInterrupt) { + futures.addAll(childCtx.interruptChildContexts()); + } } - return engine.cancelOrInterrupt(Collections.singletonList(this), startMillis, timeout); + /* + * No matter whether we successfully transitioned into one of the interrupting + * states, we wait for threads to be completed (which is done as a part of the + * cancel method) as the states that override interrupting states also lead to + * threads being stopped. If that happens before the timeout, the interrupt is + * successful. + */ + return PolyglotEngineImpl.cancelOrInterrupt(this, futures, startMillis, timeout); } finally { try { - PolyglotContextImpl[] childContextsToFinishInterrupt; - synchronized (this) { - interrupting = false; - childContextsToFinishInterrupt = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); - } - for (PolyglotContextImpl childCtx : childContextsToFinishInterrupt) { - childCtx.finishInterruptForChildContexts(); + if (childContextsToInterrupt != null) { + PolyglotContextImpl[] childContextsToFinishInterrupt; + synchronized (this) { + unsetInterrupting(); + childContextsToFinishInterrupt = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); + } + for (PolyglotContextImpl childCtx : childContextsToFinishInterrupt) { + childCtx.finishInterruptForChildContexts(); + } } } finally { - closingLock.unlock(); + interruptingLock.unlock(); } } } catch (Throwable thr) { @@ -1373,21 +1622,11 @@ Object toGuestValue(Node location, Object hostValue) { } } - void waitForClose() { - while (!closeImpl(false, true, true)) { - try { - synchronized (this) { - wait(1000); - } - } catch (InterruptedException e) { - } - } - } - boolean waitForThreads(long startMillis, long timeoutMillis) { synchronized (this) { long timeElapsed = System.currentTimeMillis() - startMillis; - while (hasActiveOtherThread(true) && (timeoutMillis == 0 || timeElapsed < timeoutMillis)) { + boolean otherThreadActive; + while ((otherThreadActive = hasActiveOtherThread(true)) && (timeoutMillis == 0 || timeElapsed < timeoutMillis)) { try { if (timeoutMillis == 0) { wait(); @@ -1398,7 +1637,16 @@ boolean waitForThreads(long startMillis, long timeoutMillis) { } timeElapsed = System.currentTimeMillis() - startMillis; } - return !hasActiveOtherThread(true); + /* + * hasActiveOtherThread is racy. E.g. one of the threads might be just about to enter + * via fast path and so hasActiveOtherThread might return a different result if we + * executed it again after the while loop. The fast-path enter might not go through in + * the end, especially if this is waiting for cancellation of all threads, so it is not + * a problem that hasActiveOtherThread is racy, but it is important that waitForThreads + * does not return a wrong value. That is why we store the result in a boolean so that + * the returned value corresponds to the reason why the while loop has ended. + */ + return !otherThreadActive; } } @@ -1411,7 +1659,7 @@ Map getSeenThreads() { return threads; } - boolean isActiveNotCancelled() { + private boolean isActiveNotCancelled() { return isActiveNotCancelled(true); } @@ -1433,14 +1681,6 @@ synchronized boolean isActive() { return false; } - synchronized boolean isActiveNotCancelled(Thread thread) { - PolyglotThreadInfo info = threads.get(thread); - if (info == null || info == PolyglotThreadInfo.NULL) { - return false; - } - return info.isActiveNotCancelled(); - } - synchronized boolean isActive(Thread thread) { PolyglotThreadInfo info = threads.get(thread); if (info == null || info == PolyglotThreadInfo.NULL) { @@ -1449,14 +1689,14 @@ synchronized boolean isActive(Thread thread) { return info.isActive(); } - PolyglotThreadInfo getFirstActiveOtherThread(boolean includePolyglotThreads) { + private PolyglotThreadInfo getFirstActiveOtherThread(boolean includePolyglotThreads) { assert Thread.holdsLock(this); // send enters and leaves into a lock by setting the lastThread to null. for (PolyglotThreadInfo otherInfo : threads.values()) { if (!includePolyglotThreads && otherInfo.isPolyglotThread(this)) { continue; } - if (!otherInfo.isCurrent() && otherInfo.isActiveNotCancelled()) { + if (!otherInfo.isCurrent() && otherInfo.isActive()) { return otherInfo; } } @@ -1467,15 +1707,15 @@ boolean hasActiveOtherThread(boolean includePolyglotThreads) { return getFirstActiveOtherThread(includePolyglotThreads) != null; } - synchronized void notifyThreadClosed() { - PolyglotThreadInfo currentTInfo = getCachedThreadInfo(); - if (currentTInfo != PolyglotThreadInfo.NULL) { - currentTInfo.cancelled = true; + private void notifyThreadClosed(PolyglotThreadInfo info) { + assert Thread.holdsLock(this); + if (!info.cancelled) { // clear interrupted status after closingThread // needed because we interrupt when closingThread from another thread. + info.cancelled = true; Thread.interrupted(); - notifyAll(); } + notifyAll(); } long calculateHeapSize(long stopAtBytes, AtomicBoolean calculationCancelled) { @@ -1494,7 +1734,7 @@ long calculateHeapSize(long stopAtBytes, AtomicBoolean calculationCancelled) { } } - Object[] getContextHeapRoots() { + private Object[] getContextHeapRoots() { List heapRoots = new ArrayList<>(); addRootPointersForContext(heapRoots); addRootPointersForStackFrames(heapRoots); @@ -1537,113 +1777,229 @@ private void addRootPointersForContext(List heapRoots) { } } - private void cancelChildContexts() { - PolyglotContextImpl[] childContextsToCancel; + private List> setCancelling(boolean resourceLimit, String message) { + assert message != null; + PolyglotContextImpl[] childContextsToCancel = null; + List> futures = new ArrayList<>(); synchronized (this) { - this.cancelling = true; - childContextsToCancel = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); + if (!state.isInvalidOrClosed()) { + State targetState; + if (state.isClosing()) { + targetState = State.CLOSING_CANCELLING; + } else { + targetState = State.CANCELLING; + } + invalidResourceLimit = resourceLimit; + invalidMessage = message; + setState(targetState); + PolyglotThreadInfo info = getCachedThreadInfo(); + futures.add(threadLocalActions.submit(null, PolyglotEngineImpl.ENGINE_ID, new CancellationThreadLocalAction(), true)); + if (info != PolyglotThreadInfo.NULL) { + info.cancelled = true; + Thread.interrupted(); + } + setCachedThreadInfo(PolyglotThreadInfo.NULL); + childContextsToCancel = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); + } } - for (PolyglotContextImpl childCtx : childContextsToCancel) { - childCtx.cancelChildContexts(); + if (childContextsToCancel != null) { + for (PolyglotContextImpl childCtx : childContextsToCancel) { + futures.addAll(childCtx.setCancelling(resourceLimit, message)); + } } + return futures; } + private void setClosingState() { + assert Thread.holdsLock(this); + closingThread = Thread.currentThread(); + clearExplicitContextStack(); + closingLock.lock(); + State targetState; + switch (state) { + case CANCELLING: + targetState = State.CLOSING_CANCELLING; + break; + case INTERRUPTING: + targetState = State.CLOSING_INTERRUPTING; + break; + default: + targetState = State.CLOSING; + break; + } + setState(targetState); + } + + private void setClosedState() { + assert Thread.holdsLock(this); + assert state.isClosing() : state.name(); + State targetState; + switch (state) { + case CLOSING_CANCELLING: + targetState = State.CLOSED_CANCELLED; + break; + case CLOSING_INTERRUPTING: + case CLOSING: + targetState = State.CLOSED; + break; + default: + throw new IllegalStateException("Cannot close polyglot context in the current state!"); + } + setState(targetState); + assert state.isClosed() : state.name(); + } + + private void restoreFromClosingState(boolean cancelOperation) { + assert Thread.holdsLock(this); + assert state.isClosing() : state.name(); + State targetState; + assert !cancelOperation : "Close initiated for an invalid context must not fail!"; + switch (state) { + case CLOSING_INTERRUPTING: + targetState = State.INTERRUPTING; + break; + case CLOSING_CANCELLING: + targetState = State.CANCELLING; + break; + default: + targetState = State.DEFAULT; + break; + } + setState(targetState); + } + + @SuppressWarnings({"fallthrough"}) @SuppressFBWarnings("UL_UNRELEASED_LOCK_EXCEPTION_PATH") - boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads, boolean notifyInstruments) { + boolean closeImpl(boolean notifyInstruments) { /* - * As a first step we prepare for close by waiting for other threads to finish closing and - * checking whether other threads are still executing. This block performs the following - * checks: + * Close operation initiated in the DEFAULT or INTERRUPTING state can fail in which case the + * context will go back to the corresponsing non-closing e.g. DEFAULT -> CLOSING -> DEFAULT. + * Please note that while the default close is in progress, i.e. the context state is in + * CLOSING or CLOSING_INTERRUPTING state, the state can be overriden by CLOSING_CANCELLING. + * Even in this case the default close can still fail and if that is the case the context + * state goes back to CANCELLING. The close operation is then guaranteed to be completed by + * the process that initiated cancel. + * + * This block performs the following checks: * * 1) The close was already performed on another thread -> return true * * 2) The close is currently already being performed on this thread -> return true * - * 3) The close was not yet performed but other threads are still executing -> mark current - * thread as cancelled and return false + * 3) The close is currently being performed on another thread -> wait for the other thread + * to finish closing and start checking again from check 1). + * + * 4) The close was not yet performed, cancelling or executing is not in progress, but other + * threads are still executing -> return false * - * 4) The close was not yet performed and no thread is executing -> perform close + * 5) The close was not yet performed and cancelling is in progress -> wait for other + * threads to complete and start checking again from check 1) skipping check 5) (this check) + * as no other threads can be executing anymore. + * + * 6) The close was not yet performed and no thread is executing -> perform close */ - boolean waitForCloseOrInterrupt = false; - PolyglotContextImpl[] childContextsToCancel = null; - try { - while (true) { - if (waitForCloseOrInterrupt) { - closingLock.lock(); - closingLock.unlock(); - waitForCloseOrInterrupt = false; - } - synchronized (this) { - if (closed) { - // already cancelled + boolean waitForClose = false; + boolean finishCancel = false; + boolean cancelOperation; + acquireClosingLock: while (true) { + if (waitForClose) { + closingLock.lock(); + closingLock.unlock(); + waitForClose = false; + } + synchronized (this) { + switch (state) { + case CLOSED: + case CLOSED_CANCELLED: return true; - } - Thread localClosingThread = closingThread; - if (localClosingThread != null) { - if (localClosingThread == Thread.currentThread()) { - // currently canceling recursively -> just complete + case CLOSING: + case CLOSING_INTERRUPTING: + case CLOSING_CANCELLING: + assert closingThread != null; + if (closingThread == Thread.currentThread()) { + // currently closing recursively -> just complete return true; } else { - // currently canceling on another thread -> wait for other thread to + // currently closing on another thread -> wait for other thread to // complete closing - waitForCloseOrInterrupt = true; - continue; + waitForClose = true; + continue acquireClosingLock; } - } - PolyglotThreadInfo threadInfo = getCachedThreadInfo(); - if (interrupting) { - // currently interrupting on another thread - if (parent == null) { - // interrupt operation holds the closingLock -> wait for the interrupt - // to - // complete - waitForCloseOrInterrupt = true; - continue; - } - } - - // triggers a thread changed event which requires slow path enter - setCachedThreadInfo(PolyglotThreadInfo.NULL); - if (cancelIfExecuting) { - cancelling = true; - childContextsToCancel = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); - if (threadInfo != PolyglotThreadInfo.NULL) { - threadInfo.cancelled = true; - // clear interrupted status after closingThread - // needed because we interrupt when closingThread from another thread. - Thread.interrupted(); + case CANCELLING: + assert cachedThreadInfo == PolyglotThreadInfo.NULL; + /* + * When cancelling, we have to wait for all other threads to complete - even + * for the the default close, otherwise the default close executed + * prematurely as the result of leaving the context on the main thread due + * to cancel exception could fail because of other threads still being + * active. The correct behavior is that the normal close finishes + * successfully and the cancel exception spreads further (if not caught + * before close is executed). + */ + if (!finishCancel) { + waitForThreads(0, 0); + waitForClose = true; + finishCancel = true; + /* + * During wait this thread didn't hold the polyglot context lock, so + * some other thread might have acquired closingLock in the meantime. In + * that case it wouldn't be possible to acquire the closingLock by this + * thread in the current synchronized block, because the thread that + * holds it might need to acquire the context lock before releasing the + * closingLock, but the context lock is held by this thread, and so we + * have to exit the synchronized block and try again. + */ + continue acquireClosingLock; } - } - - if (hasActiveOtherThread(waitForPolyglotThreads)) { /* - * We are not done executing, cannot close yet. + * Just continue with the close if we have already waited for threads in the + * previous iteration of the main loop. We cannot wait for the close to be + * completed by the thread that executes cancelling, because it might be + * waiting for this thread which would lead to a deadlock. Default close is + * allowed to be executed when entered. Also, this might be an inner + * context, which, even if not entered, might block a parent's thread which + * could be entered on the current thread. */ - return false; - } - closingThread = Thread.currentThread(); - if (!threadInfo.explicitContextStack.isEmpty()) { - PolyglotContextImpl c = this; - while (!threadInfo.explicitContextStack.isEmpty()) { - PolyglotContextImpl prev = threadInfo.explicitContextStack.removeLast(); - engine.leave(prev, c); - c = prev; + setClosingState(); + cancelOperation = true; + break acquireClosingLock; + case INTERRUPTING: + case DEFAULT: + // triggers a thread changed event which requires slow path enter + setCachedThreadInfo(PolyglotThreadInfo.NULL); + if (hasActiveOtherThread(false)) { + /* + * We are not done executing, cannot close yet. + */ + return false; } - threadInfo.explicitContextStack.clear(); - } - closingLock.lock(); - break; + setClosingState(); + cancelOperation = false; + break acquireClosingLock; + default: + assert false; } } - } finally { - if (childContextsToCancel != null) { - for (PolyglotContextImpl childCtx : childContextsToCancel) { - childCtx.cancelChildContexts(); - } + } + + return finishClose(cancelOperation, notifyInstruments); + } + + private void clearExplicitContextStack() { + assert Thread.holdsLock(this); + PolyglotThreadInfo threadInfo = getCachedThreadInfo(); + if (!threadInfo.explicitContextStack.isEmpty()) { + PolyglotContextImpl c = this; + while (!threadInfo.explicitContextStack.isEmpty()) { + PolyglotContextImpl prev = threadInfo.explicitContextStack.removeLast(); + engine.leave(prev, c, true); + c = prev; } } + } + private boolean finishClose(boolean cancelOperation, boolean notifyInstruments) { /* * If we reach here then we can continue with the close. This means that no other concurrent * close is running and no other thread is currently executing. Note that only the context @@ -1655,12 +2011,29 @@ boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads, boo try { assert closingThread == Thread.currentThread(); assert closingLock.isHeldByCurrentThread() : "lock is acquired"; - assert !closed; - PolyglotContextImpl prev = engine.enter(this, engine.getUncachedLocation(), true); + assert !state.isClosed(); + PolyglotContextImpl prev; try { - closeChildContexts(cancelIfExecuting, waitForPolyglotThreads, notifyInstruments); + prev = engine.enter(this, false, engine.getUncachedLocation(), !cancelOperation, cancelOperation); + } catch (Throwable t) { + synchronized (this) { + restoreFromClosingState(cancelOperation); + } + throw t; + } + if (cancelOperation) { + synchronized (this) { + /* + * Cancellation thread local action needs to be submitted here in case + * finalizeContext runs guest code. + */ + threadLocalActions.submit(new Thread[]{Thread.currentThread()}, PolyglotEngineImpl.ENGINE_ID, new CancellationThreadLocalAction(), true); + } + } + try { + closeChildContexts(notifyInstruments); - finalizeContext(notifyInstruments); + finalizeContext(notifyInstruments, cancelOperation); // finalization performed commit close -> no reinitialization allowed @@ -1674,37 +2047,40 @@ boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads, boo * synchronized. */ assert !success || childContextsClosed() : "Polyglot context close marked as successful, but there are unclosed child contexts."; - engine.leave(prev, this); + engine.leave(prev, this, false); if (success) { remainingThreads = threads.keySet().toArray(new Thread[0]); } - if (success && cancelling) { - cancelled = true; - } - cancelling = false; if (success) { - closed = true; - invalid = true; + setClosedState(); + } else { + restoreFromClosingState(cancelOperation); } + disposing = false; // triggers a thread changed event which requires slow path enter setCachedThreadInfo(PolyglotThreadInfo.NULL); - } - if (success && engine.boundEngine) { - disposeStaticContext(this); + if (success && engine.boundEngine && !isActive(Thread.currentThread())) { + /* + * We cannot dispose the context if the context is still entered in this + * thread, we can dispose it only when the thread leaves the context. + */ + disposeStaticContext(this); + } } } } finally { - closingThread = null; - closingLock.unlock(); + synchronized (this) { + assert !state.isClosing(); + closingThread = null; + closingLock.unlock(); + } } /* * No longer any lock is held. So we can acquire other locks to cleanup. */ - if (disposedContexts != null) { - for (PolyglotLanguageContext context : disposedContexts) { - context.notifyDisposed(notifyInstruments); - } + for (PolyglotLanguageContext context : disposedContexts) { + context.notifyDisposed(notifyInstruments); } if (success) { @@ -1734,12 +2110,10 @@ boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads, boo // sends all threads to do slow-path enter/leave setCachedThreadInfo(PolyglotThreadInfo.NULL); /* - * This should be reworked. We shouldn't need to check isActive here. When a context - * is closed from within an entered thread we should just throw an error that - * propagates the cancel for the current thread only. This might require some - * changes in language launchers (Node.js). + * If we are closing from within an entered thread, we cannot clear locals as they + * might be needed in e.g. onLeaveThread events. */ - if (!isActive()) { + if (!isActive(Thread.currentThread())) { threadLocalActions.notifyContextClosed(); if (contexts != null) { @@ -1760,8 +2134,8 @@ boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads, boo Arrays.fill(threadLocals, null); } } + localsCleared = true; } - localsCleared = true; } if (parent == null) { if (!this.config.logLevels.isEmpty()) { @@ -1783,55 +2157,144 @@ boolean closeImpl(boolean cancelIfExecuting, boolean waitForPolyglotThreads, boo private boolean childContextsClosed() { assert Thread.holdsLock(this); for (PolyglotContextImpl childCtx : childContexts) { - if (!childCtx.closed) { + if (!childCtx.state.isClosed()) { return false; } } return true; } - private void closeChildContexts(boolean cancelIfExecuting, boolean waitForPolyglotThreads, boolean notifyInstruments) { + private void closeChildContexts(boolean notifyInstruments) { PolyglotContextImpl[] childrenToClose; synchronized (this) { childrenToClose = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); } for (PolyglotContextImpl childContext : childrenToClose) { - childContext.closeImpl(cancelIfExecuting, waitForPolyglotThreads, notifyInstruments); + childContext.closeImpl(notifyInstruments); + } + } + + private void closeHereOrCancelInCleanupThread(List> futures) { + boolean cancelInSeparateThread = false; + synchronized (this) { + PolyglotThreadInfo info = getCachedThreadInfo(); + if (info.isPolyglotThread(this) || (!singleThreaded.isValid() && isActive(Thread.currentThread()))) { + /* + * Polyglot thread must not cancel a context, because cancel waits for polyglot + * threads to complete. Also, it is not allowed to cancel in a thread where a + * multi-threaded context is entered. This would lead to deadlock if more than one + * thread tried to do that as cancel waits for the context not to be entered in all + * other threads. + */ + cancelInSeparateThread = true; + } + } + if (cancelInSeparateThread) { + if (!futures.isEmpty()) { + /* + * Checking the futures for emptiness makes sure we don't register multiple cleanup + * tasks if this is called from multiple threads + */ + registerCleanupTask(new Runnable() { + @Override + public void run() { + PolyglotEngineImpl.cancel(PolyglotContextImpl.this, futures); + } + }); + } + } else { + closeAndMaybeWait(true, futures); + } + } + + private void registerCleanupTask(Runnable cleanupTask) { + synchronized (this) { + if (cleanupExecutorService == null) { + cleanupExecutorService = Executors.newFixedThreadPool(1, new ThreadFactory() { + @Override + public Thread newThread(Runnable r) { + Thread t = new Thread(r); + t.setDaemon(true); + return t; + } + }); + } + assert cleanupFuture == null : "Multiple cleanup tasks are currently not supported!"; + cleanupFuture = cleanupExecutorService.submit(cleanupTask); + } + } + + @SuppressWarnings("deprecation") + void finishCleanup() { + ExecutorService localCleanupService; + synchronized (this) { + if (isActive(Thread.currentThread())) { + /* + * The cleanup must be able to wait for the context to leave all threads which would + * be impossible if it is still entered in the current thread. + */ + return; + } + localCleanupService = cleanupExecutorService; + } + if (localCleanupService != null) { + try { + try { + cleanupFuture.get(); + } catch (InterruptedException ie) { + engine.getEngineLogger().log(Level.INFO, "Waiting for polyglot context cleanup was interrupted!", ie); + } catch (ExecutionException ee) { + assert !(ee.getCause() instanceof com.oracle.truffle.api.TruffleException); + throw sneakyThrow(ee.getCause()); + } + } finally { + localCleanupService.shutdownNow(); + while (!localCleanupService.isTerminated()) { + try { + if (!localCleanupService.awaitTermination(1, TimeUnit.MINUTES)) { + throw new IllegalStateException("Context cleanup service timeout!"); + } + } catch (InterruptedException ie) { + engine.getEngineLogger().log(Level.INFO, "Waiting for polyglot context cleanup was interrupted!", ie); + } + } + } } } + @SuppressWarnings("unchecked") + private static RuntimeException sneakyThrow(Throwable ex) throws T { + throw (T) ex; + } + private List disposeContext() { assert !this.disposing; this.disposing = true; - try { - List disposedContexts = new ArrayList<>(contexts.length); - Closeable[] toClose; - synchronized (this) { - for (int i = contexts.length - 1; i >= 0; i--) { - PolyglotLanguageContext context = contexts[i]; - boolean disposed = context.dispose(); - if (disposed) { - disposedContexts.add(context); - } + List disposedContexts = new ArrayList<>(contexts.length); + Closeable[] toClose; + synchronized (this) { + for (int i = contexts.length - 1; i >= 0; i--) { + PolyglotLanguageContext context = contexts[i]; + boolean disposed = context.dispose(); + if (disposed) { + disposedContexts.add(context); } - toClose = closeables == null ? null : closeables.toArray(new Closeable[0]); } - if (toClose != null) { - for (Closeable closeable : toClose) { - try { - closeable.close(); - } catch (IOException ioe) { - engine.getEngineLogger().log(Level.WARNING, "Failed to close " + closeable, ioe); - } + toClose = closeables == null ? null : closeables.toArray(new Closeable[0]); + } + if (toClose != null) { + for (Closeable closeable : toClose) { + try { + closeable.close(); + } catch (IOException ioe) { + engine.getEngineLogger().log(Level.WARNING, "Failed to close " + closeable, ioe); } } - return disposedContexts; - } finally { - this.disposing = false; } + return disposedContexts; } - private void finalizeContext(boolean notifyInstruments) { + private void finalizeContext(boolean notifyInstruments, boolean cancelOperation) { // we need to run finalization at least twice in case a finalization run has // initialized a new contexts boolean finalizationPerformed; @@ -1842,14 +2305,14 @@ private void finalizeContext(boolean notifyInstruments) { for (int i = contexts.length - 1; i >= 0; i--) { PolyglotLanguageContext context = contexts[i]; if (context.isInitialized()) { - finalizationPerformed |= context.finalizeContext(notifyInstruments); + finalizationPerformed |= context.finalizeContext(cancelOperation, notifyInstruments); } } } while (finalizationPerformed); } synchronized void sendInterrupt() { - if (!cancelling && !interrupting) { + if (!state.isInterrupting() && !state.isCancelling()) { return; } for (PolyglotThreadInfo threadInfo : threads.values()) { @@ -2113,7 +2576,7 @@ boolean patch(PolyglotContextConfig newConfig) { if (!newConfig.logLevels.isEmpty()) { EngineAccessor.LANGUAGE.configureLoggers(this, newConfig.logLevels, getAllLoggers()); } - final PolyglotContextImpl prev = engine.enter(this, engine.getUncachedLocation(), true); + final PolyglotContextImpl prev = engine.enter(this, true, engine.getUncachedLocation(), true, false); try { for (int i = 0; i < this.contexts.length; i++) { final PolyglotLanguageContext context = this.contexts[i]; @@ -2125,7 +2588,7 @@ boolean patch(PolyglotContextConfig newConfig) { } } } finally { - engine.leave(prev, this); + engine.leave(prev, this, true); } return true; } @@ -2236,7 +2699,7 @@ static PolyglotContextImpl preInitialize(final PolyglotEngineImpl engine) { if (!languagesToPreinitialize.isEmpty()) { context.inContextPreInitialization = true; try { - PolyglotContextImpl prev = context.engine.enter(context, context.engine.getUncachedLocation(), true); + PolyglotContextImpl prev = context.engine.enter(context, true, context.engine.getUncachedLocation(), true, false); try { for (String languageId : engine.getLanguages().keySet()) { if (languagesToPreinitialize.contains(languageId)) { @@ -2302,7 +2765,7 @@ void leaveAndDisposeThread(PolyglotContextImpl prev, Thread thread) { LANGUAGE.disposeThread(languageContext.env, thread); } } - engine.leave(prev, this); + engine.leave(prev, this, true); info.setContextThreadLocals(DISPOSED_CONTEXT_THREAD_LOCALS); setCurrentThreadLocals(DISPOSED_CONTEXT_THREAD_LOCALS); seenThreads.remove(thread); @@ -2348,40 +2811,10 @@ static class ContextWeakReference extends WeakReference { } - CancelExecution createCancelException(Node location) { + private CancelExecution createCancelException(Node location) { return new CancelExecution(location, invalidMessage, invalidResourceLimit); } - synchronized boolean invalidate(boolean resourceLimit, String message) { - assert message != null; - if (!invalid) { - setCachedThreadInfo(PolyglotThreadInfo.NULL); - /* - * Setting the invalid message and invalid flag will cause a special invalid message - * when the context was disabled. - */ - invalidMessage = message; - invalidResourceLimit = resourceLimit; - invalid = true; - return true; - } - return false; - } - - boolean invalidateAll(boolean resourceLimit, String message) { - boolean invalidated; - PolyglotContextImpl[] childContextsToInvalidate; - synchronized (this) { - invalidated = invalidate(resourceLimit, message); - childContextsToInvalidate = childContexts.toArray(new PolyglotContextImpl[childContexts.size()]); - } - - for (PolyglotContextImpl childCtx : childContextsToInvalidate) { - invalidated = childCtx.invalidateAll(resourceLimit, invalidMessage) || invalidated; - } - return invalidated; - } - private static boolean overridesPatchContext(String languageId) { LanguageCache cache = LanguageCache.languages().get(languageId); for (Method m : cache.loadLanguage().getClass().getDeclaredMethods()) { @@ -2415,18 +2848,13 @@ public String toString() { StringBuilder b = new StringBuilder(); b.append("PolyglotContextImpl["); b.append("state="); - if (closed) { - b.append("closed"); - if (this.invalid) { - b.append(" invalid"); - } - } else if (cancelling) { - b.append("cancelling"); - } else { + State localState = state; + b.append(localState.name()); + if (!localState.isClosed()) { if (isActive()) { - b.append("active"); + b.append(", active"); } else { - b.append("inactive"); + b.append(", inactive"); } } @@ -2444,48 +2872,47 @@ public String toString() { return b.toString(); } - Future submitCancellationThreadLocal() { - assert Thread.holdsLock(this); - return threadLocalActions.submit(null, PolyglotEngineImpl.ENGINE_ID, new ThreadLocalAction(true, false) { - @Override - protected void perform(Access access) { - cancelExecution(access); - } + private final class CancellationThreadLocalAction extends ThreadLocalAction { + CancellationThreadLocalAction() { + super(false, false); + } - private void cancelExecution(Access access) { - threadLocalActions.submit(new Thread[]{access.getThread()}, PolyglotEngineImpl.ENGINE_ID, new ThreadLocalAction(true, false) { - @Override - protected void perform(Access access2) { - cancelExecution(access2); - } - }, true); + @Override + protected void perform(Access access) { + PolyglotContextImpl.this.threadLocalActions.submit(new Thread[]{access.getThread()}, PolyglotEngineImpl.ENGINE_ID, this, true, false, false, true); - if (access.getThread() != PolyglotContextImpl.this.closingThread) { - if (invalid || cancelling) { - throw createCancelException(access.getLocation()); - } else if (interrupting) { - throw new PolyglotEngineImpl.InterruptExecution(access.getLocation()); - } - } + State localState = PolyglotContextImpl.this.state; + if (localState.isInvalidOrClosed() || localState.isCancelling()) { + throw createCancelException(access.getLocation()); } - }, true); + } } - List> submitCancellationThreadLocals() { - List> futures = new ArrayList<>(); - PolyglotContextImpl[] localChildContexts = null; - synchronized (this) { - if (!closed) { - futures.add(submitCancellationThreadLocal()); - localChildContexts = childContexts.toArray(new PolyglotContextImpl[0]); - } + private final class InterruptThreadLocalAction extends ThreadLocalAction { + InterruptThreadLocalAction() { + super(true, false); } - if (localChildContexts != null) { - for (PolyglotContextImpl childContext : localChildContexts) { - futures.addAll(childContext.submitCancellationThreadLocals()); + + @Override + protected void perform(Access access) { + PolyglotContextImpl.this.threadLocalActions.submit(new Thread[]{access.getThread()}, PolyglotEngineImpl.ENGINE_ID, this, true); + + State localState = state; + if (access.getThread() != PolyglotContextImpl.this.closingThread) { + if (localState.isInterrupting()) { + PolyglotContextImpl[] interruptingChildContexts; + synchronized (PolyglotContextImpl.this) { + interruptingChildContexts = PolyglotContextImpl.this.childContexts.toArray(new PolyglotContextImpl[0]); + } + for (PolyglotContextImpl childCtx : interruptingChildContexts) { + if (access.getThread() == childCtx.closingThread) { + return; + } + } + // Interrupt should never break a closing operation + throw new PolyglotEngineImpl.InterruptExecution(access.getLocation()); + } } } - return futures; } - } diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineException.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineException.java index 509c1f753ccc..9824a995c00c 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineException.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineException.java @@ -40,13 +40,14 @@ */ package com.oracle.truffle.polyglot; -import com.oracle.truffle.api.TruffleLanguage; -import com.oracle.truffle.api.TruffleLanguage.Env; -import org.graalvm.polyglot.Context; - import java.util.ConcurrentModificationException; import java.util.NoSuchElementException; +import org.graalvm.polyglot.Context; + +import com.oracle.truffle.api.TruffleLanguage; +import com.oracle.truffle.api.TruffleLanguage.Env; + /** * Represents an expected user exception caused by the polyglot engine. It is wrapped such that it * can be differentiated from internal errors. This class is not supposed to be visible to the @@ -98,16 +99,10 @@ final class PolyglotEngineException extends RuntimeException { final RuntimeException e; - final boolean closingContext; PolyglotEngineException(RuntimeException e) { - this(e, false); - } - - private PolyglotEngineException(RuntimeException e, boolean closingContext) { super(null, e); this.e = e; - this.closingContext = closingContext; } @SuppressWarnings("sync-override") @@ -134,10 +129,6 @@ static PolyglotEngineException illegalState(String message) { return new PolyglotEngineException(new IllegalStateException(message)); } - static PolyglotEngineException illegalState(String message, boolean closingContext) { - return new PolyglotEngineException(new IllegalStateException(message), closingContext); - } - static PolyglotEngineException nullPointer(String message) { return new PolyglotEngineException(new NullPointerException(message)); } diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineImpl.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineImpl.java index e653d6ea51cb..26e68e8149fa 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineImpl.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineImpl.java @@ -73,7 +73,6 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import java.util.function.Predicate; @@ -108,7 +107,6 @@ import com.oracle.truffle.api.TruffleFile; import com.oracle.truffle.api.TruffleLanguage; import com.oracle.truffle.api.TruffleLogger; -import com.oracle.truffle.api.TruffleSafepoint; import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.dsl.SpecializationStatistics; import com.oracle.truffle.api.exception.AbstractTruffleException; @@ -191,7 +189,6 @@ final class PolyglotEngineImpl implements com.oracle.truffle.polyglot.PolyglotIm final Assumption singleThreadPerContext = Truffle.getRuntime().createAssumption("Single thread per context of an engine."); final Assumption noInnerContexts = Truffle.getRuntime().createAssumption("No inner contexts."); final Assumption customHostClassLoader = Truffle.getRuntime().createAssumption("No custom host class loader needed."); - final Assumption neverInterrupted = Truffle.getRuntime().createAssumption("No context interrupted."); volatile OptionDescriptors allOptions; volatile boolean closed; @@ -1039,7 +1036,7 @@ > PolyglotLanguageInstance getCurrentLanguageInstan return null; } - void ensureClosed(boolean cancelIfExecuting, boolean inShutdownHook) { + void ensureClosed(boolean force, boolean inShutdownHook) { synchronized (this.lock) { if (!closed) { workContextReferenceQueue(); @@ -1049,11 +1046,11 @@ void ensureClosed(boolean cancelIfExecuting, boolean inShutdownHook) { * contexts. */ if (!inShutdownHook) { - if (!cancelIfExecuting) { + if (!force) { for (PolyglotContextImpl context : localContexts) { assert !Thread.holdsLock(context); synchronized (context) { - if (context.hasActiveOtherThread(false) && context.closingThread == null) { + if (context.hasActiveOtherThread(false) && !context.state.isClosing()) { throw PolyglotEngineException.illegalState(String.format("One of the context instances is currently executing. " + "Set cancelIfExecuting to true to stop the execution on this thread.")); } @@ -1062,15 +1059,17 @@ void ensureClosed(boolean cancelIfExecuting, boolean inShutdownHook) { } for (PolyglotContextImpl context : localContexts) { assert !Thread.holdsLock(context); - boolean closeCompleted = context.closeImpl(cancelIfExecuting, cancelIfExecuting, true); - if (!closeCompleted && !cancelIfExecuting) { - throw PolyglotEngineException.illegalState(String.format("One of the context instances is currently executing. " + - "Set cancelIfExecuting to true to stop the execution on this thread.")); + if (force) { + context.cancel(false, null); + } else { + boolean closeCompleted = context.closeImpl(true); + if (!closeCompleted) { + throw PolyglotEngineException.illegalState(String.format("One of the context instances is currently executing. " + + "Set cancelIfExecuting to true to stop the execution on this thread.")); + } + context.finishCleanup(); + context.checkSubProcessFinished(); } - context.checkSubProcessFinished(); - } - if (cancelIfExecuting) { - cancel(localContexts); } } @@ -1338,77 +1337,62 @@ public void run() { } } - void cancel(List localContexts) { - cancelOrInterrupt(localContexts, 0, null); + static void cancel(PolyglotContextImpl context, List> cancelationFutures) { + cancelOrInterrupt(context, cancelationFutures, 0, null); } - boolean cancelOrInterrupt(List localContexts, long startMillis, Duration timeout) { - boolean cancelling = false; - for (PolyglotContextImpl context : localContexts) { - if (context.cancelling || context.interrupting) { - cancelling = true; - break; + @SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter") + static boolean cancelOrInterrupt(PolyglotContextImpl context, List> futures, long startMillis, Duration timeout) { + try { + synchronized (context) { + assert context.singleThreaded.isValid() || !context.isActive(Thread.currentThread()) : "Cancel while entered is only allowed for single-threaded contexts!"; + context.sendInterrupt(); } - } - if (cancelling) { - List> futures = new ArrayList<>(); - try { - for (PolyglotContextImpl context : localContexts) { - futures.addAll(context.submitCancellationThreadLocals()); - } - for (PolyglotContextImpl context : localContexts) { - context.sendInterrupt(); - } - if (timeout == null) { - for (PolyglotContextImpl context : localContexts) { - context.waitForClose(); - } - } else { - long cancelTimeoutMillis = timeout != Duration.ZERO ? timeout.toMillis() : 0; - boolean success = true; - for (PolyglotContextImpl context : localContexts) { - if (!context.waitForThreads(startMillis, cancelTimeoutMillis)) { - success = false; - } - } - return success; - } - } finally { - for (Future future : futures) { - AtomicBoolean timedOut = new AtomicBoolean(); - TruffleSafepoint.setBlockedThreadInterruptible(getUncachedLocation(), new TruffleSafepoint.Interruptible>() { - @Override - public void apply(Future cancelationFuture) throws InterruptedException { + if (timeout == null) { + boolean closeCompleted = context.closeImpl(true); + assert closeCompleted : "Close was not completed!"; + } else { + return waitForThreads(context, startMillis, timeout); + } + } finally { + for (Future future : futures) { + boolean timedOut = false; + try { + if (timeout != null) { + long timeElapsed = System.currentTimeMillis() - startMillis; + long timeoutMillis = timeout.toMillis(); + if (timeElapsed < timeoutMillis) { try { - if (timeout != null) { - long timeElapsed = System.currentTimeMillis() - startMillis; - long timeoutMillis = timeout.toMillis(); - if (timeElapsed < timeoutMillis) { - try { - cancelationFuture.get(timeoutMillis - timeElapsed, TimeUnit.MILLISECONDS); - } catch (TimeoutException te) { - timedOut.set(true); - } - } else { - timedOut.set(true); - } - } else { - cancelationFuture.get(); - } - } catch (ExecutionException e) { - throw CompilerDirectives.shouldNotReachHere(e); + future.get(timeoutMillis - timeElapsed, TimeUnit.MILLISECONDS); + } catch (TimeoutException te) { + timedOut = true; } + } else { + timedOut = true; } - }, future); - if (timedOut.get()) { - return false; + } else { + future.get(); } + } catch (ExecutionException | InterruptedException e) { + throw CompilerDirectives.shouldNotReachHere(e); + } + if (timedOut) { + return false; } } } return true; } + private static boolean waitForThreads(PolyglotContextImpl context, long startMillis, Duration timeout) { + long cancelTimeoutMillis = timeout != Duration.ZERO ? timeout.toMillis() : 0; + boolean success = true; + if (!context.waitForThreads(startMillis, cancelTimeoutMillis)) { + success = false; + } + return success; + } + @SuppressWarnings("serial") static final class CancelExecution extends ThreadDeath { @@ -1626,7 +1610,7 @@ public PolyglotContextImpl createContext(OutputStream configOut, OutputStream co // events must be replayed without engine lock. final PolyglotContextImpl prev; try { - prev = enter(context, getUncachedLocation(), true); + prev = enter(context, true, getUncachedLocation(), true, false); } catch (Throwable t) { throw PolyglotImpl.guestToHostException(context.getHostContext(), t, false); } @@ -1636,7 +1620,7 @@ public PolyglotContextImpl createContext(OutputStream configOut, OutputStream co throw PolyglotImpl.guestToHostException(context.getHostContext(), t, true); } finally { try { - leave(prev, context); + leave(prev, context, true); } catch (Throwable t) { throw PolyglotImpl.guestToHostException(context.getHostContext(), t, false); } @@ -1689,7 +1673,7 @@ private PolyglotContextImpl loadPreinitializedContext(PolyglotContextConfig conf addContext(context); } } else { - context.closeImpl(false, false, false); + context.closeImpl(false); PolyglotContextImpl.disposeStaticContext(null); config.fileSystem = oldFileSystem; config.internalFileSystem = oldInternalFileSystem; @@ -1776,7 +1760,7 @@ boolean needsEnter(PolyglotContextImpl context) { Object enterIfNeeded(PolyglotContextImpl context, boolean pollSafepoint) { if (needsEnter(context)) { - return enter(context, getUncachedLocation(), pollSafepoint); + return enter(context, true, getUncachedLocation(), pollSafepoint, false); } assert PolyglotContextImpl.currentNotEntered() != null; return NO_ENTER; @@ -1784,11 +1768,11 @@ Object enterIfNeeded(PolyglotContextImpl context, boolean pollSafepoint) { void leaveIfNeeded(Object prev, PolyglotContextImpl context) { if (prev != NO_ENTER) { - leave((PolyglotContextImpl) prev, context); + leave((PolyglotContextImpl) prev, context, true); } } - PolyglotContextImpl enter(PolyglotContextImpl context, Node safepointLocation, boolean pollSafepoint) { + PolyglotContextImpl enter(PolyglotContextImpl context, boolean notifyEnter, Node safepointLocation, boolean pollSafepoint, boolean deactivateSafepoints) { PolyglotContextImpl prev; PolyglotThreadInfo info = context.cachedThreadInfo; boolean enterReverted = false; @@ -1804,11 +1788,13 @@ PolyglotContextImpl enter(PolyglotContextImpl context, Node safepointLocation, b * submitted cached thread info will be null and we will enter slow path, where the * safepoint is polled. */ - try { - info.notifyEnter(this, context); - } catch (Throwable e) { - info.leaveInternal(prev); - throw e; + if (notifyEnter) { + try { + info.notifyEnter(this, context); + } catch (Throwable e) { + info.leaveInternal(prev); + throw e; + } } return prev; } else { @@ -1826,24 +1812,27 @@ PolyglotContextImpl enter(PolyglotContextImpl context, Node safepointLocation, b * thread. The slow path acquires context lock to ensure ordering for context operations * like close. */ - prev = context.enterThreadChanged(safepointLocation, enterReverted, pollSafepoint); + prev = context.enterThreadChanged(notifyEnter, safepointLocation, enterReverted, pollSafepoint, deactivateSafepoints); assert verifyContext(context); return prev; } private static boolean verifyContext(PolyglotContextImpl context) { - return context == PolyglotContextImpl.currentNotEntered() || context.closed || context.invalid; + PolyglotContextImpl.State localState = context.state; + return context == PolyglotContextImpl.currentNotEntered() || localState.isInvalidOrClosed(); } - void leave(PolyglotContextImpl prev, PolyglotContextImpl context) { - assert context.closed || context.closingThread == Thread.currentThread() || + void leave(PolyglotContextImpl prev, PolyglotContextImpl context, boolean notifyLeft) { + assert context.state.isClosed() || PolyglotContextImpl.currentNotEntered() == context : "Cannot leave context that is currently not entered. Forgot to enter or leave a context?"; boolean entered = true; PolyglotThreadInfo info = context.cachedThreadInfo; if (CompilerDirectives.injectBranchProbability(CompilerDirectives.LIKELY_PROBABILITY, info.getThread() == Thread.currentThread())) { try { - info.notifyLeave(this, context); + if (notifyLeft) { + info.notifyLeave(this, context); + } } finally { info.leaveInternal(prev); entered = false; @@ -1853,7 +1842,7 @@ void leave(PolyglotContextImpl prev, PolyglotContextImpl context) { return; } } - context.leaveThreadChanged(prev, entered); + context.leaveThreadChanged(prev, notifyLeft, entered); } static final class LogConfig { diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotExceptionImpl.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotExceptionImpl.java index 96d3c37eb300..02ca68e5990b 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotExceptionImpl.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotExceptionImpl.java @@ -101,17 +101,17 @@ final class PolyglotExceptionImpl { private final Value guestObject; private final String message; - PolyglotExceptionImpl(PolyglotEngineImpl engine, boolean polyglotContextCancellingOrCancelled, Throwable original) { - this(engine.impl, engine, polyglotContextCancellingOrCancelled, null, original, false, false); + PolyglotExceptionImpl(PolyglotEngineImpl engine, PolyglotContextImpl.State polyglotContextState, Throwable original) { + this(engine.impl, engine, polyglotContextState, null, original, false, false); } // Exception coming from an instrument PolyglotExceptionImpl(PolyglotImpl polyglot, Throwable original) { - this(polyglot, null, false, null, original, true, false); + this(polyglot, null, null, null, original, true, false); } @SuppressWarnings("deprecation") - PolyglotExceptionImpl(PolyglotImpl polyglot, PolyglotEngineImpl engine, boolean polyglotContextCancellingOrCancelled, PolyglotLanguageContext languageContext, Throwable original, + PolyglotExceptionImpl(PolyglotImpl polyglot, PolyglotEngineImpl engine, PolyglotContextImpl.State polyglotContextState, PolyglotLanguageContext languageContext, Throwable original, boolean allowInterop, boolean entered) { this.polyglot = polyglot; @@ -127,7 +127,8 @@ final class PolyglotExceptionImpl { try { ExceptionType exceptionType = interop.getExceptionType(exception); this.internal = false; - this.cancelled = polyglotContextCancellingOrCancelled || isLegacyTruffleExceptionCancelled(exception); + this.cancelled = (polyglotContextState != null && (polyglotContextState.isCancelling() || polyglotContextState == PolyglotContextImpl.State.CLOSED_CANCELLED)) || + isLegacyTruffleExceptionCancelled(exception); this.syntaxError = exceptionType == ExceptionType.PARSE_ERROR; this.exit = exceptionType == ExceptionType.EXIT; this.exitStatus = this.exit ? interop.getExceptionExitStatus(exception) : 0; @@ -158,7 +159,9 @@ final class PolyglotExceptionImpl { throw CompilerDirectives.shouldNotReachHere(ume); } } else { - this.cancelled = polyglotContextCancellingOrCancelled || (exception instanceof CancelExecution) || isLegacyTruffleExceptionCancelled(exception); + this.cancelled = (polyglotContextState != null && (polyglotContextState.isCancelling() || polyglotContextState == PolyglotContextImpl.State.CLOSED_CANCELLED)) || + (exception instanceof CancelExecution) || + isLegacyTruffleExceptionCancelled(exception); /* * When polyglot context is invalid, we cannot obtain the exception type from * InterruptExecution exception via interop. Please note that in this case the diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotImpl.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotImpl.java index 0ca1e80fda8f..2a8422872970 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotImpl.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotImpl.java @@ -487,11 +487,12 @@ static PolyglotException guestToHostException(PolyglotLang PolyglotContextImpl context = languageContext.context; PolyglotExceptionImpl exceptionImpl; - if (context.closed || context.invalid) { - exceptionImpl = new PolyglotExceptionImpl(context.engine, context.cancelling || context.cancelled, e); + PolyglotContextImpl.State localContextState = context.state; + if (localContextState.isInvalidOrClosed()) { + exceptionImpl = new PolyglotExceptionImpl(context.engine, localContextState, e); } else { try { - exceptionImpl = new PolyglotExceptionImpl(languageContext.getImpl(), languageContext.context.engine, context.cancelling || context.cancelled, + exceptionImpl = new PolyglotExceptionImpl(languageContext.getImpl(), languageContext.context.engine, localContextState, languageContext, e, true, entered); } catch (Throwable t) { /* @@ -499,7 +500,7 @@ static PolyglotException guestToHostException(PolyglotLang * Report the exception as an internal error. */ e.addSuppressed(t); - exceptionImpl = new PolyglotExceptionImpl(context.engine, context.cancelling || context.cancelled, e); + exceptionImpl = new PolyglotExceptionImpl(context.engine, localContextState, e); } } APIAccess access = getInstance().getAPIAccess(); @@ -511,7 +512,7 @@ static PolyglotException guestToHostException(PolyglotEngi PolyglotEngineException.rethrow(e); APIAccess access = engine.getAPIAccess(); - PolyglotExceptionImpl exceptionImpl = new PolyglotExceptionImpl(engine, false, e); + PolyglotExceptionImpl exceptionImpl = new PolyglotExceptionImpl(engine, null, e); return access.newLanguageException(exceptionImpl.getMessage(), getInstance().exceptionDispatch, exceptionImpl); } diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLanguageContext.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLanguageContext.java index 81b9643b202d..3fe7ab2ac7ec 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLanguageContext.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLanguageContext.java @@ -368,10 +368,31 @@ Env requireEnv() { return localEnv; } - boolean finalizeContext(boolean notifyInstruments) { + @SuppressWarnings("deprecation") + boolean finalizeContext(boolean cancelOperation, boolean notifyInstruments) { if (!finalized) { finalized = true; - LANGUAGE.finalizeContext(env); + try { + LANGUAGE.finalizeContext(env); + } catch (Throwable t) { + if (cancelOperation) { + /* + * finalizeContext can run guest code, and so truffle and cancel exceptions are + * expected. However, they must not fail the cancel operation, and so we just + * log them. + */ + assert context.state.isClosing(); + assert context.state.isInvalidOrClosed(); + if (t instanceof com.oracle.truffle.api.TruffleException || t instanceof PolyglotEngineImpl.CancelExecution) { + context.engine.getEngineLogger().log(Level.INFO, + "Exception was thrown while finalizing a polyglot context that is being cancelled. Such exceptions are expected during cancelling.", t); + } else { + throw t; + } + } else { + throw t; + } + } if (eventsEnabled && notifyInstruments) { EngineAccessor.INSTRUMENT.notifyLanguageContextFinalized(context.engine, context.creatorTruffleContext, language.info); } @@ -380,6 +401,7 @@ boolean finalizeContext(boolean notifyInstruments) { return false; } + @SuppressWarnings("deprecation") boolean dispose() { assert Thread.holdsLock(context); Env localEnv = this.env; @@ -388,17 +410,41 @@ boolean dispose() { // this should show up as internal error so it does not use PolyglotEngineException throw new IllegalStateException("The language did not complete all polyglot threads but should have: " + lazy.activePolyglotThreads); } - for (PolyglotThreadInfo threadInfo : context.getSeenThreads().values()) { - assert threadInfo != PolyglotThreadInfo.NULL; - final Thread thread = threadInfo.getThread(); - if (thread == null) { - continue; + try { + for (PolyglotThreadInfo threadInfo : context.getSeenThreads().values()) { + assert threadInfo != PolyglotThreadInfo.NULL; + final Thread thread = threadInfo.getThread(); + if (thread == null) { + continue; + } + assert !threadInfo.isPolyglotThread(context) : "Polyglot threads must no longer be active in TruffleLanguage.finalizeContext, but polyglot thread " + thread.getName() + + " is still active."; + if (!threadInfo.isCurrent() && threadInfo.isActive() && !context.state.isInvalidOrClosed()) { + /* + * No other thread than the current thread should be active here. However, + * we do this check only for non-invalid contexts for the following reasons. + * enteredCount for a thread can be incremented on the fast-path even though + * the thread is not allowed to enter in the end because the context is + * invalid and so the enter falls back to the slow path which checks the + * invalid flag. threadInfo.isActive() returns true in this case and we + * cannot tell whether it is because the thread is before the fallback to + * the slow path or it is already fully entered (which would be an error) + * without adding further checks to the fast path, and so we don't perform + * the check for invalid contexts. Non-invalid context can have the same + * problem with the enteredCount of one of its threads, but closing + * non-invalid context in that state is an user error. + */ + throw PolyglotEngineException.illegalState("Another main thread was started while closing a polyglot context!"); + } + LANGUAGE.disposeThread(localEnv, thread); + } + LANGUAGE.dispose(localEnv); + } catch (Throwable t) { + if (t instanceof com.oracle.truffle.api.TruffleException || t instanceof PolyglotEngineImpl.CancelExecution) { + throw new IllegalStateException("Guest language code was run during language disposal!", t); } - assert !threadInfo.isPolyglotThread(context) : "Polyglot threads must no longer be active in TruffleLanguage.finalizeContext, but polyglot thread " + thread.getName() + - " is still active."; - LANGUAGE.disposeThread(localEnv, thread); + throw t; } - LANGUAGE.dispose(localEnv); return true; } return false; @@ -415,7 +461,7 @@ PolyglotContextImpl enterThread(PolyglotThread thread) { assert isInitialized(); assert Thread.currentThread() == thread; synchronized (context) { - PolyglotContextImpl prev = context.engine.enter(context, language.engine.getUncachedLocation(), true); + PolyglotContextImpl prev = context.engine.enter(context, true, language.engine.getUncachedLocation(), true, false); lazy.activePolyglotThreads.add(thread); return prev; } @@ -794,7 +840,7 @@ private class PolyglotUncaughtExceptionHandler implements Thread.UncaughtExcepti @Override public void uncaughtException(Thread t, Throwable e) { Env currentEnv = env; - if (currentEnv != null) { + if (currentEnv != null && !(e instanceof ThreadDeath)) { try { e.printStackTrace(new PrintStream(currentEnv.err())); } catch (Throwable exc) { diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLimits.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLimits.java index 8f88d4a3d53b..d443cb2f581c 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLimits.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotLimits.java @@ -51,6 +51,7 @@ import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.CompilerDirectives.CompilationFinal; import com.oracle.truffle.api.Truffle; +import com.oracle.truffle.api.TruffleSafepoint; import com.oracle.truffle.api.frame.FrameDescriptor; import com.oracle.truffle.api.frame.FrameSlot; import com.oracle.truffle.api.frame.FrameSlotKind; @@ -184,17 +185,12 @@ private void notifyStatementLimitReached(PolyglotContextImpl context, long actua } } if (limitReached) { - String message = String.format("Statement count limit of %s exceeded. Statements executed %s.", - limit, actualCount); - boolean invalidated = context.invalidate(true, message); - if (invalidated) { - context.close(true); - RuntimeException e = limits.notifyEvent(context); - if (e != null) { - throw e; - } - throw context.createCancelException(eventContext.getInstrumentedNode()); + context.cancel(true, String.format("Statement count limit of %s exceeded. Statements executed %s.", limit, actualCount)); + RuntimeException e = limits.notifyEvent(context); + if (e != null) { + throw e; } + TruffleSafepoint.pollHere(eventContext.getInstrumentedNode()); } } diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotStackFramesRetriever.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotStackFramesRetriever.java index 90b2087df79b..0c7423e7ddad 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotStackFramesRetriever.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotStackFramesRetriever.java @@ -64,7 +64,7 @@ static FrameInstance[][] getStackFrames(PolyglotContextImpl context) { Future future; synchronized (context) { threads = context.getSeenThreads().keySet().toArray(new Thread[0]); - if (!context.closed) { + if (!context.state.isClosed()) { future = context.threadLocalActions.submit(null, PolyglotEngineImpl.ENGINE_ID, new ThreadLocalAction(false, false) { @Override protected void perform(Access access) { diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThread.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThread.java index 53ef8048699d..e37281efe8f1 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThread.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThread.java @@ -115,15 +115,7 @@ public Object execute(VirtualFrame frame) { @TruffleBoundary private static Object executeImpl(PolyglotLanguageContext languageContext, PolyglotThread thread, PolyglotThreadRunnable run) { PolyglotContextImpl prev; - try { - prev = languageContext.enterThread(thread); - } catch (PolyglotEngineException polyglotException) { - if (polyglotException.closingContext) { - return null; - } else { - throw polyglotException; - } - } + prev = languageContext.enterThread(thread); assert prev == null; // is this assertion correct? try { run.execute(); diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadInfo.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadInfo.java index b7508ae8f7de..5ddeec710134 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadInfo.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadInfo.java @@ -103,7 +103,7 @@ boolean isCurrent() { /** * Not to be used directly. Use - * {@link PolyglotEngineImpl#enter(PolyglotContextImpl, Node, boolean)} instead. + * {@link PolyglotEngineImpl#enter(PolyglotContextImpl, boolean, Node, boolean)} instead. */ @SuppressFBWarnings("VO_VOLATILE_INCREMENT") PolyglotContextImpl enterInternal() { @@ -119,7 +119,7 @@ int getEnteredCount() { /** * Not to be used directly. Use - * {@link PolyglotEngineImpl#leave(PolyglotContextImpl, PolyglotContextImpl)} instead. + * {@link PolyglotEngineImpl#leave(PolyglotContextImpl, PolyglotContextImpl, boolean)} instead. */ @SuppressFBWarnings("VO_VOLATILE_INCREMENT") void leaveInternal(PolyglotContextImpl prev) { diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadLocalActions.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadLocalActions.java index ebb05b2fb0a7..4173749cfc61 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadLocalActions.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotThreadLocalActions.java @@ -132,7 +132,7 @@ void notifyEnterCreatedThread() { void notifyContextClosed() { assert Thread.holdsLock(context); - assert !context.isActive() || context.cancelled : "context is still active, cannot flush safepoints"; + assert !context.isActive() || context.state == PolyglotContextImpl.State.CLOSED_CANCELLED : "context is still active, cannot flush safepoints"; if (intervalTimer != null) { intervalTimer.cancel(); } @@ -146,7 +146,7 @@ void notifyContextClosed() { for (AbstractTLHandshake handshake : activeEventsList) { Future future = handshake.future; if (!future.isDone()) { - if (context.cancelled) { + if (context.state == PolyglotContextImpl.State.CLOSED_CANCELLED) { // we allow cancellation for invalid or cancelled contexts future.cancel(true); pendingThreadLocalAction = true; @@ -202,10 +202,10 @@ private static void formatStatisticLine(StringBuilder s, String label, LongSumma Future submit(Thread[] threads, String originId, ThreadLocalAction action, boolean needsEnter) { boolean sync = EngineAccessor.LANGUAGE.isSynchronousTLAction(action); - return submit(threads, originId, action, needsEnter, sync, sync); + return submit(threads, originId, action, needsEnter, sync, sync, false); } - Future submit(Thread[] threads, String originId, ThreadLocalAction action, boolean needsEnter, boolean syncStartOfEvent, boolean syncEndOfEvent) { + Future submit(Thread[] threads, String originId, ThreadLocalAction action, boolean needsEnter, boolean syncStartOfEvent, boolean syncEndOfEvent, boolean ignoreContextClosed) { TL_HANDSHAKE.testSupport(); Objects.requireNonNull(action); if (threads != null) { @@ -218,7 +218,7 @@ Future submit(Thread[] threads, String originId, ThreadLocalAction action, // send enter/leave to slow-path context.setCachedThreadInfo(PolyglotThreadInfo.NULL); - if (context.closed) { + if (context.state.isClosed() && !ignoreContextClosed) { return CompletableFuture.completedFuture(null); } @@ -232,7 +232,7 @@ Future submit(Thread[] threads, String originId, ThreadLocalAction action, List activePolyglotThreads = new ArrayList<>(); for (PolyglotThreadInfo info : context.getSeenThreads().values()) { Thread t = info.getThread(); - if (info.isActiveNotCancelled() && (filterThreads == null || filterThreads.contains(t))) { + if (info.isActive() && (filterThreads == null || filterThreads.contains(t))) { if (info.isCurrent() && sync && info.isSafepointActive()) { throw new IllegalStateException( "Recursive synchronous thread local action detected. " + @@ -246,6 +246,7 @@ Future submit(Thread[] threads, String originId, ThreadLocalAction action, Thread[] activeThreads = activePolyglotThreads.toArray(new Thread[0]); AbstractTLHandshake handshake; if (sync) { + assert syncStartOfEvent || syncEndOfEvent : "No synchronization requested for sync event!"; handshake = new SyncEvent(context, threads, originId, action, needsEnter); } else { assert !syncStartOfEvent : "Start of event sync requested for async event!"; @@ -291,7 +292,7 @@ private void log(String action, AbstractTLHandshake handshake, String details) { } Set notifyThreadActivation(PolyglotThreadInfo info, boolean active) { - assert info.getEnteredCount() == (active ? 1 : 0) : "must be currently entered successfully"; + assert !active || info.getEnteredCount() == 1 : "must be currently entered successfully"; assert Thread.holdsLock(context); if (activeEvents.isEmpty()) { @@ -425,9 +426,6 @@ final void notifyDone() { } public final void accept(Node location) { - if (context.closed) { - return; - } Object prev = null; if (needsEnter) { prev = context.engine.enterIfNeeded(context, false); diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotWrapper.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotWrapper.java index 6bcaa3425270..b6be679ecb16 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotWrapper.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotWrapper.java @@ -103,10 +103,14 @@ static boolean equals(Object context, Object receiver, Object obj) { } else if (receiver == obj) { return true; } + PolyglotLanguageContext languageContext = (PolyglotLanguageContext) context; - if (languageContext != null && (languageContext.context.closed || languageContext.context.invalid)) { - return false; + if (languageContext != null) { + PolyglotContextImpl.State localContextState = languageContext.context.state; + if (localContextState.isInvalidOrClosed()) { + return false; + } } Object prev = null; try { @@ -130,15 +134,17 @@ static boolean equals(Object context, Object receiver, Object obj) { // ignore errors leaving we cannot propagate them. } } - } @TruffleBoundary static int hashCode(Object context, Object receiver) { PolyglotLanguageContext languageContext = (PolyglotLanguageContext) context; - if (languageContext != null && (languageContext.context.closed || languageContext.context.invalid)) { - return System.identityHashCode(receiver); + if (languageContext != null) { + PolyglotContextImpl.State localContextState = languageContext.context.state; + if (localContextState.isInvalidOrClosed()) { + return System.identityHashCode(receiver); + } } Object prev = null; try { From bffea6c03aed408682344cb0f6773bb23b8811aa Mon Sep 17 00:00:00 2001 From: Pavel Marek Date: Sun, 20 Jun 2021 13:28:08 +0000 Subject: [PATCH 18/18] [GR-30277] Re-enable API checksum of recommended packages cache (2). PullRequest: fastr/2626 --- vm/mx.vm/suite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vm/mx.vm/suite.py b/vm/mx.vm/suite.py index 5f07a9f1e6f0..d55ce138ffdb 100644 --- a/vm/mx.vm/suite.py +++ b/vm/mx.vm/suite.py @@ -66,7 +66,7 @@ }, { "name": "fastr", - "version": "8d754b3a4673675285083ac9fd7152c9e30fea38", + "version": "260c1dacd55c1d83c7278e51fe8ea703bff8c810", "dynamic": True, "urls": [ {"url": "https://github.com/oracle/fastr.git", "kind": "git"},