From 88960361bb976ffc68be4f15db0d75889f56209c Mon Sep 17 00:00:00 2001 From: Tomas Zezula Date: Mon, 3 Jan 2022 18:57:02 +0100 Subject: [PATCH 1/5] [GR-35703] ProcessHandler for TruffleIsolates. --- .../svm/truffle/api/SubstrateTruffleRuntime.java | 4 ++-- .../com/oracle/truffle/polyglot/ProcessHandlers.java | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/api/SubstrateTruffleRuntime.java b/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/api/SubstrateTruffleRuntime.java index 74e16bd28313..a675fbc1599f 100644 --- a/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/api/SubstrateTruffleRuntime.java +++ b/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/api/SubstrateTruffleRuntime.java @@ -428,11 +428,11 @@ private static final class CompilerThreadScope implements AutoCloseable { open(); } - // Substituted by EnterpriseTruffleFeature + // Substituted by PolyglotIsolateFeature private void open() { } - // Substituted by EnterpriseTruffleFeature + // Substituted by PolyglotIsolateFeature @Override public void close() { } diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java index d149bb71b2f0..afbc53859e73 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java @@ -250,6 +250,7 @@ private static final class CopierThread extends Thread { @Override public void run() { + open(); try { while (true) { if (isInterrupted()) { @@ -262,7 +263,17 @@ public void run() { out.write(buffer, 0, read); } } catch (IOException e) { + } finally { + close(); } } + + // Substituted by PolyglotIsolateFeature + private void open() { + } + + // Substituted by PolyglotIsolateFeature + private void close() { + } } } From eb0e1b182e7a4c3f4ec8d6fbef879370625b29db Mon Sep 17 00:00:00 2001 From: Tomas Zezula Date: Wed, 5 Jan 2022 09:17:56 +0100 Subject: [PATCH 2/5] [GR-35703] Added factory method for default ProcessHandler. --- .../src/org/graalvm/polyglot/Engine.java | 6 ++++++ .../src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java | 4 ++++ .../src/com/oracle/truffle/polyglot/PolyglotImpl.java | 6 ++++++ 3 files changed, 16 insertions(+) diff --git a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java index d18dab7b0924..d042075f1471 100644 --- a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java +++ b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java @@ -91,6 +91,7 @@ import org.graalvm.polyglot.impl.AbstractPolyglotImpl.AbstractValueDispatch; import org.graalvm.polyglot.io.FileSystem; import org.graalvm.polyglot.io.MessageTransport; +import org.graalvm.polyglot.io.ProcessHandler; /** * An execution engine for Graal {@linkplain Language guest languages} that allows to inspect the @@ -1019,6 +1020,11 @@ public FileSystem newDefaultFileSystem() { throw noPolyglotImplementationFound(); } + @Override + public ProcessHandler newDefaultProcessHandler() { + throw noPolyglotImplementationFound(); + } + @Override public Object newTargetTypeMapping(Class sourceType, Class targetType, Predicate acceptsValue, Function convertValue, TargetMappingPrecedence precedence) { return new Object(); diff --git a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java index 9189f5da17fc..33b8a9a6907a 100644 --- a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java +++ b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java @@ -1081,6 +1081,10 @@ public FileSystem newDefaultFileSystem() { return getNext().newDefaultFileSystem(); } + public ProcessHandler newDefaultProcessHandler() { + return getNext().newDefaultProcessHandler(); + } + /** * Creates a union of all available option descriptors including prev implementations. This * allows to validate the full set of options. 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 caf6dcbcceb7..8fa2757bbfa7 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 @@ -76,6 +76,7 @@ import org.graalvm.polyglot.io.ByteSequence; import org.graalvm.polyglot.io.FileSystem; import org.graalvm.polyglot.io.MessageTransport; +import org.graalvm.polyglot.io.ProcessHandler; import org.graalvm.polyglot.proxy.Proxy; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; @@ -456,6 +457,11 @@ public FileSystem newDefaultFileSystem() { return FileSystems.newDefaultFileSystem(); } + @Override + public ProcessHandler newDefaultProcessHandler() { + return ProcessHandlers.newDefaultProcessHandler(); + } + @Override public AbstractHostAccess createHostAccess() { return new PolyglotHostAccess(this); From 8bdae52e5e19ac9859d8b386cfbe805d0a549bc1 Mon Sep 17 00:00:00 2001 From: Tomas Zezula Date: Wed, 5 Jan 2022 11:00:57 +0100 Subject: [PATCH 3/5] [GR-35703] Fixed EngineImpl#hasDefaultProcessHandler. --- .../api/impl/InternalProcessHandler.java | 81 +++++++++++++++++++ .../truffle/polyglot/EngineAccessor.java | 4 +- .../truffle/polyglot/ProcessHandlers.java | 13 +-- 3 files changed, 92 insertions(+), 6 deletions(-) create mode 100644 truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/InternalProcessHandler.java diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/InternalProcessHandler.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/InternalProcessHandler.java new file mode 100644 index 000000000000..5ec98383e595 --- /dev/null +++ b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/InternalProcessHandler.java @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2022, 2022, 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.impl; + +import org.graalvm.polyglot.io.ProcessHandler; + +import java.io.IOException; + +/** + * Extends the {@link ProcessHandler} interface by {@link InternalProcessHandler#isDefault()} used + * to detect default {@link ProcessHandler} implementation. + */ +public interface InternalProcessHandler extends ProcessHandler { + + /** + * Returns {@code true} if this process handler is a default implementation. + */ + boolean isDefault(); + + /** + * Returns a {@link InternalProcessHandler} for given {@link ProcessHandler}. If + * {@code processHandler} is an instance of the {@link InternalProcessHandler} it's returned + * otherwise the {@code processHandler} is decorated by a new {@link InternalProcessHandler} + * whose {@link #isDefault()} operation returns {@code false}. + */ + static InternalProcessHandler wrap(ProcessHandler processHandler) { + if (processHandler instanceof InternalProcessHandler) { + return (InternalProcessHandler) processHandler; + } else { + return new InternalProcessHandler() { + @Override + public boolean isDefault() { + return false; + } + + @Override + public Process start(ProcessCommand command) throws IOException { + return processHandler.start(command); + } + }; + } + } +} 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 21d4cf57ac4c..a37d3a521cb5 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 @@ -71,6 +71,7 @@ import java.util.logging.Level; import java.util.logging.LogRecord; +import com.oracle.truffle.api.impl.InternalProcessHandler; import org.graalvm.collections.Pair; import org.graalvm.nativeimage.ImageInfo; import org.graalvm.options.OptionKey; @@ -1171,7 +1172,8 @@ public Process createSubProcess(Object polyglotLanguageContext, List cmd @Override public boolean hasDefaultProcessHandler(Object polyglotLanguageContext) { - return ProcessHandlers.isDefault(((PolyglotLanguageContext) polyglotLanguageContext).context.config.processHandler); + ProcessHandler handler = ((PolyglotLanguageContext) polyglotLanguageContext).context.config.processHandler; + return handler instanceof InternalProcessHandler && ((InternalProcessHandler) handler).isDefault(); } @Override diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java index afbc53859e73..4eabaffee55d 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java @@ -50,6 +50,8 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.TimeUnit; + +import com.oracle.truffle.api.impl.InternalProcessHandler; import org.graalvm.polyglot.io.ProcessHandler; final class ProcessHandlers { @@ -62,10 +64,6 @@ static ProcessHandler newDefaultProcessHandler() { return new DefaultProcessHandler(); } - static boolean isDefault(ProcessHandler handler) { - return handler instanceof DefaultProcessHandler; - } - static Process decorate(PolyglotLanguageContext owner, List cmd, Process process, OutputStream out, OutputStream err) { ProcessDecorator decorator = new ProcessDecorator(owner, cmd.get(0), process, out, err); @@ -75,7 +73,12 @@ static Process decorate(PolyglotLanguageContext owner, List cm return decorator; } - private static final class DefaultProcessHandler implements ProcessHandler { + private static final class DefaultProcessHandler implements InternalProcessHandler { + + @Override + public boolean isDefault() { + return true; + } @Override public Process start(ProcessCommand command) throws IOException { From 2d7fe90c17e66681fe77d23b87a7d71bcca9607f Mon Sep 17 00:00:00 2001 From: Tomas Zezula Date: Wed, 5 Jan 2022 19:13:39 +0100 Subject: [PATCH 4/5] [GR-35703] Added AbstractPolyglotImpl#createThreadScope to attach threads to JavaVM. --- .../runtime/BackgroundCompileQueue.java | 3 ++- .../src/org/graalvm/polyglot/Engine.java | 2 +- .../polyglot/impl/AbstractPolyglotImpl.java | 9 ++++++++ .../truffle/api/SubstrateTruffleRuntime.java | 22 ------------------- .../com/oracle/truffle/api/impl/Accessor.java | 2 ++ .../truffle/polyglot/EngineAccessor.java | 5 +++++ .../oracle/truffle/polyglot/PolyglotImpl.java | 9 ++++++-- .../truffle/polyglot/PolyglotThread.java | 4 +++- .../truffle/polyglot/ProcessHandlers.java | 15 +++---------- 9 files changed, 32 insertions(+), 39 deletions(-) diff --git a/compiler/src/org.graalvm.compiler.truffle.runtime/src/org/graalvm/compiler/truffle/runtime/BackgroundCompileQueue.java b/compiler/src/org.graalvm.compiler.truffle.runtime/src/org/graalvm/compiler/truffle/runtime/BackgroundCompileQueue.java index 22f4206a1270..64c7185959a7 100644 --- a/compiler/src/org.graalvm.compiler.truffle.runtime/src/org/graalvm/compiler/truffle/runtime/BackgroundCompileQueue.java +++ b/compiler/src/org.graalvm.compiler.truffle.runtime/src/org/graalvm/compiler/truffle/runtime/BackgroundCompileQueue.java @@ -282,7 +282,8 @@ public Thread newThread(Runnable r) { @Override public void run() { setContextClassLoader(getClass().getClassLoader()); - try (AutoCloseable scope = runtime.openCompilerThreadScope()) { + try (AutoCloseable compilerThreadScope = runtime.openCompilerThreadScope(); + AutoCloseable polyglotThreadScope = GraalRuntimeAccessor.ENGINE.createPolyglotThreadScope()) { super.run(); if (compilationExecutorService.allowsCoreThreadTimeOut()) { // If core threads are always kept alive (no timeout), the diff --git a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java index d042075f1471..b9c9e66666b8 100644 --- a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java +++ b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2017, 2022, 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 diff --git a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java index 33b8a9a6907a..e6976bccfc94 100644 --- a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java +++ b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java @@ -1085,6 +1085,10 @@ public ProcessHandler newDefaultProcessHandler() { return getNext().newDefaultProcessHandler(); } + public ThreadScope createThreadScope() { + return getNext().createThreadScope(); + } + /** * Creates a union of all available option descriptors including prev implementations. This * allows to validate the full set of options. @@ -1110,4 +1114,9 @@ protected OptionDescriptors createEngineOptionDescriptors() { return OptionDescriptors.EMPTY; } + public interface ThreadScope extends AutoCloseable { + @Override + void close(); + } + } diff --git a/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/api/SubstrateTruffleRuntime.java b/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/api/SubstrateTruffleRuntime.java index a675fbc1599f..5cfa5aabeefb 100644 --- a/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/api/SubstrateTruffleRuntime.java +++ b/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/api/SubstrateTruffleRuntime.java @@ -131,11 +131,6 @@ protected AbstractFastThreadLocal getFastThreadLocalImpl() { return SubstrateFastThreadLocal.SINGLETON; } - @Override - protected AutoCloseable openCompilerThreadScope() { - return new CompilerThreadScope(); - } - private void initializeAtRuntime(OptimizedCallTarget callTarget) { truffleCompiler.initialize(getOptionsForCompiler(callTarget), callTarget, true); if (SubstrateTruffleOptions.isMultiThreaded()) { @@ -421,21 +416,4 @@ public boolean hasNextTier() { } } - - private static final class CompilerThreadScope implements AutoCloseable { - - CompilerThreadScope() { - open(); - } - - // Substituted by PolyglotIsolateFeature - private void open() { - } - - // Substituted by PolyglotIsolateFeature - @Override - public void close() { - } - } - } diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java index d34755bfb031..1db02f0e2815 100644 --- a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java +++ b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/Accessor.java @@ -678,6 +678,8 @@ public abstract Iterator mergeHostGuestFrames(Object instrumentEnv, St public abstract boolean getNeedsAllEncodings(); public abstract boolean requireLanguageWithAllEncodings(Object encoding); + + public abstract AutoCloseable createPolyglotThreadScope(); } public abstract static class LanguageSupport extends Support { 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 a37d3a521cb5..9045944948ed 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 @@ -1621,6 +1621,11 @@ public Object getGuestToHostCodeCache(Object polyglotContextImpl) { public Object installGuestToHostCodeCache(Object polyglotContextImpl, Object cache) { return ((PolyglotContextImpl) polyglotContextImpl).getHostContext().getLanguageInstance().installGuestToHostCodeCache(cache); } + + @Override + public AutoCloseable createPolyglotThreadScope() { + return PolyglotImpl.getActivePolyglot().createThreadScope(); + } } abstract static class AbstractClassLoaderSupplier implements Supplier { 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 8fa2757bbfa7..d9ee4c4b6a97 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 @@ -130,7 +130,7 @@ public int getPriority() { return 0; // default priority } - private static AbstractPolyglotImpl getImpl() { + static AbstractPolyglotImpl getActivePolyglot() { AbstractPolyglotImpl local = abstractImpl; if (local == null) { try { @@ -146,7 +146,7 @@ private static AbstractPolyglotImpl getImpl() { } static PolyglotImpl getInstance() { - AbstractPolyglotImpl polyglot = getImpl(); + AbstractPolyglotImpl polyglot = getActivePolyglot(); while (polyglot != null && !(polyglot instanceof PolyglotImpl)) { polyglot = polyglot.getNext(); } @@ -462,6 +462,11 @@ public ProcessHandler newDefaultProcessHandler() { return ProcessHandlers.newDefaultProcessHandler(); } + @Override + public ThreadScope createThreadScope() { + return null; + } + @Override public AbstractHostAccess createHostAccess() { return new PolyglotHostAccess(this); 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 1bdefd28263c..f0f1ccc9809b 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 @@ -47,6 +47,7 @@ import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.nodes.RootNode; import com.oracle.truffle.polyglot.PolyglotEngineImpl.CancelExecution; +import org.graalvm.polyglot.impl.AbstractPolyglotImpl.ThreadScope; final class PolyglotThread extends Thread { @@ -127,10 +128,11 @@ public Object execute(VirtualFrame frame) { } @TruffleBoundary + @SuppressWarnings("try") private static Object executeImpl(PolyglotLanguageContext languageContext, PolyglotThread thread, PolyglotThreadRunnable run) { Object[] prev = languageContext.enterThread(thread); assert prev == null; // is this assertion correct? - try { + try (ThreadScope scope = PolyglotImpl.getActivePolyglot().createThreadScope()) { run.execute(); } catch (CancelExecution cancel) { if (PolyglotEngineOptions.TriggerUncaughtExceptionHandlerForCancel.getValue(languageContext.context.engine.getEngineOptionValues())) { diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java index 4eabaffee55d..6a1cb391c247 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java @@ -52,6 +52,7 @@ import java.util.concurrent.TimeUnit; import com.oracle.truffle.api.impl.InternalProcessHandler; +import org.graalvm.polyglot.impl.AbstractPolyglotImpl.ThreadScope; import org.graalvm.polyglot.io.ProcessHandler; final class ProcessHandlers { @@ -252,9 +253,9 @@ private static final class CopierThread extends Thread { } @Override + @SuppressWarnings("try") public void run() { - open(); - try { + try (ThreadScope scope = PolyglotImpl.getActivePolyglot().createThreadScope()) { while (true) { if (isInterrupted()) { return; @@ -266,17 +267,7 @@ public void run() { out.write(buffer, 0, read); } } catch (IOException e) { - } finally { - close(); } } - - // Substituted by PolyglotIsolateFeature - private void open() { - } - - // Substituted by PolyglotIsolateFeature - private void close() { - } } } From 5e3b52322b7085ee8dbace451f1f3fbaf81818f1 Mon Sep 17 00:00:00 2001 From: Tomas Zezula Date: Fri, 4 Feb 2022 17:57:35 +0100 Subject: [PATCH 5/5] [GR-35703] Fixed review comments. --- .../src/org/graalvm/polyglot/Engine.java | 10 +++ .../polyglot/impl/AbstractPolyglotImpl.java | 21 ++++- .../api/impl/InternalProcessHandler.java | 81 ------------------- .../truffle/polyglot/EngineAccessor.java | 7 +- .../truffle/polyglot/PolyglotEngineImpl.java | 2 +- .../oracle/truffle/polyglot/PolyglotImpl.java | 9 ++- .../truffle/polyglot/PolyglotThread.java | 2 +- .../truffle/polyglot/ProcessHandlers.java | 24 +++--- 8 files changed, 54 insertions(+), 102 deletions(-) delete mode 100644 truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/InternalProcessHandler.java diff --git a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java index b9c9e66666b8..0240306f6aba 100644 --- a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java +++ b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Engine.java @@ -1025,6 +1025,16 @@ public ProcessHandler newDefaultProcessHandler() { throw noPolyglotImplementationFound(); } + @Override + public boolean isDefaultProcessHandler(ProcessHandler processHandler) { + return false; + } + + @Override + public ThreadScope createThreadScope() { + return null; + } + @Override public Object newTargetTypeMapping(Class sourceType, Class targetType, Predicate acceptsValue, Function convertValue, TargetMappingPrecedence precedence) { return new Object(); diff --git a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java index e6976bccfc94..5da0cf0499ca 100644 --- a/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java +++ b/sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/impl/AbstractPolyglotImpl.java @@ -1085,6 +1085,10 @@ public ProcessHandler newDefaultProcessHandler() { return getNext().newDefaultProcessHandler(); } + public boolean isDefaultProcessHandler(ProcessHandler processHandler) { + return getNext().isDefaultProcessHandler(processHandler); + } + public ThreadScope createThreadScope() { return getNext().createThreadScope(); } @@ -1114,9 +1118,22 @@ protected OptionDescriptors createEngineOptionDescriptors() { return OptionDescriptors.EMPTY; } - public interface ThreadScope extends AutoCloseable { + public final AbstractPolyglotImpl getRootImpl() { + AbstractPolyglotImpl current = this; + while (current.prev != null) { + current = current.prev; + } + return current; + } + + public abstract static class ThreadScope implements AutoCloseable { + + protected ThreadScope(AbstractPolyglotImpl engineImpl) { + Objects.requireNonNull(engineImpl); + } + @Override - void close(); + public abstract void close(); } } diff --git a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/InternalProcessHandler.java b/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/InternalProcessHandler.java deleted file mode 100644 index 5ec98383e595..000000000000 --- a/truffle/src/com.oracle.truffle.api/src/com/oracle/truffle/api/impl/InternalProcessHandler.java +++ /dev/null @@ -1,81 +0,0 @@ -/* - * Copyright (c) 2022, 2022, 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.impl; - -import org.graalvm.polyglot.io.ProcessHandler; - -import java.io.IOException; - -/** - * Extends the {@link ProcessHandler} interface by {@link InternalProcessHandler#isDefault()} used - * to detect default {@link ProcessHandler} implementation. - */ -public interface InternalProcessHandler extends ProcessHandler { - - /** - * Returns {@code true} if this process handler is a default implementation. - */ - boolean isDefault(); - - /** - * Returns a {@link InternalProcessHandler} for given {@link ProcessHandler}. If - * {@code processHandler} is an instance of the {@link InternalProcessHandler} it's returned - * otherwise the {@code processHandler} is decorated by a new {@link InternalProcessHandler} - * whose {@link #isDefault()} operation returns {@code false}. - */ - static InternalProcessHandler wrap(ProcessHandler processHandler) { - if (processHandler instanceof InternalProcessHandler) { - return (InternalProcessHandler) processHandler; - } else { - return new InternalProcessHandler() { - @Override - public boolean isDefault() { - return false; - } - - @Override - public Process start(ProcessCommand command) throws IOException { - return processHandler.start(command); - } - }; - } - } -} 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 9045944948ed..e4acfb3ff87d 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 @@ -71,7 +71,6 @@ import java.util.logging.Level; import java.util.logging.LogRecord; -import com.oracle.truffle.api.impl.InternalProcessHandler; import org.graalvm.collections.Pair; import org.graalvm.nativeimage.ImageInfo; import org.graalvm.options.OptionKey; @@ -1172,8 +1171,8 @@ public Process createSubProcess(Object polyglotLanguageContext, List cmd @Override public boolean hasDefaultProcessHandler(Object polyglotLanguageContext) { - ProcessHandler handler = ((PolyglotLanguageContext) polyglotLanguageContext).context.config.processHandler; - return handler instanceof InternalProcessHandler && ((InternalProcessHandler) handler).isDefault(); + PolyglotLanguageContext context = (PolyglotLanguageContext) polyglotLanguageContext; + return context.getImpl().getRootImpl().isDefaultProcessHandler(context.context.config.processHandler); } @Override @@ -1624,7 +1623,7 @@ public Object installGuestToHostCodeCache(Object polyglotContextImpl, Object cac @Override public AutoCloseable createPolyglotThreadScope() { - return PolyglotImpl.getActivePolyglot().createThreadScope(); + return PolyglotImpl.getInstance().getRootImpl().createThreadScope(); } } 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 dea2509927f7..bd455708e5e0 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 @@ -1734,7 +1734,7 @@ public PolyglotContextImpl createContext(OutputStream configOut, OutputStream co if (!ALLOW_CREATE_PROCESS) { throw PolyglotEngineException.illegalArgument("Cannot allowCreateProcess() because the privilege is removed at image build time"); } - useProcessHandler = processHandler != null ? processHandler : ProcessHandlers.newDefaultProcessHandler(); + useProcessHandler = processHandler != null ? processHandler : getImpl().newDefaultProcessHandler(); } else { useProcessHandler = null; } 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 d9ee4c4b6a97..82a7a21072b3 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 @@ -130,7 +130,7 @@ public int getPriority() { return 0; // default priority } - static AbstractPolyglotImpl getActivePolyglot() { + private static AbstractPolyglotImpl getImpl() { AbstractPolyglotImpl local = abstractImpl; if (local == null) { try { @@ -146,7 +146,7 @@ static AbstractPolyglotImpl getActivePolyglot() { } static PolyglotImpl getInstance() { - AbstractPolyglotImpl polyglot = getActivePolyglot(); + AbstractPolyglotImpl polyglot = getImpl(); while (polyglot != null && !(polyglot instanceof PolyglotImpl)) { polyglot = polyglot.getNext(); } @@ -462,6 +462,11 @@ public ProcessHandler newDefaultProcessHandler() { return ProcessHandlers.newDefaultProcessHandler(); } + @Override + public boolean isDefaultProcessHandler(ProcessHandler processHandler) { + return ProcessHandlers.isDefault(processHandler); + } + @Override public ThreadScope createThreadScope() { return null; 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 f0f1ccc9809b..851ad1b670cd 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 @@ -132,7 +132,7 @@ public Object execute(VirtualFrame frame) { private static Object executeImpl(PolyglotLanguageContext languageContext, PolyglotThread thread, PolyglotThreadRunnable run) { Object[] prev = languageContext.enterThread(thread); assert prev == null; // is this assertion correct? - try (ThreadScope scope = PolyglotImpl.getActivePolyglot().createThreadScope()) { + try (ThreadScope scope = languageContext.getImpl().getRootImpl().createThreadScope()) { run.execute(); } catch (CancelExecution cancel) { if (PolyglotEngineOptions.TriggerUncaughtExceptionHandlerForCancel.getValue(languageContext.context.engine.getEngineOptionValues())) { diff --git a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java index 6a1cb391c247..c32e97a0d34f 100644 --- a/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java +++ b/truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/ProcessHandlers.java @@ -51,7 +51,7 @@ import java.util.Objects; import java.util.concurrent.TimeUnit; -import com.oracle.truffle.api.impl.InternalProcessHandler; +import org.graalvm.polyglot.impl.AbstractPolyglotImpl; import org.graalvm.polyglot.impl.AbstractPolyglotImpl.ThreadScope; import org.graalvm.polyglot.io.ProcessHandler; @@ -65,6 +65,10 @@ static ProcessHandler newDefaultProcessHandler() { return new DefaultProcessHandler(); } + static boolean isDefault(ProcessHandler handler) { + return handler instanceof DefaultProcessHandler; + } + static Process decorate(PolyglotLanguageContext owner, List cmd, Process process, OutputStream out, OutputStream err) { ProcessDecorator decorator = new ProcessDecorator(owner, cmd.get(0), process, out, err); @@ -74,12 +78,7 @@ static Process decorate(PolyglotLanguageContext owner, List cm return decorator; } - private static final class DefaultProcessHandler implements InternalProcessHandler { - - @Override - public boolean isDefault() { - return true; - } + private static final class DefaultProcessHandler implements ProcessHandler { @Override public Process start(ProcessCommand command) throws IOException { @@ -126,8 +125,8 @@ private ProcessDecorator( this.owner = new WeakReference<>(owner); this.command = command; this.delegate = delegate; - this.outCopier = out == null ? null : new CopierThread(createThreadName(owner, command, "stdout"), delegate.getInputStream(), out); - this.errCopier = err == null ? null : new CopierThread(createThreadName(owner, command, "stderr"), delegate.getErrorStream(), err); + this.outCopier = out == null ? null : new CopierThread(owner.getImpl(), createThreadName(owner, command, "stdout"), delegate.getInputStream(), out); + this.errCopier = err == null ? null : new CopierThread(owner.getImpl(), createThreadName(owner, command, "stderr"), delegate.getErrorStream(), err); if (outCopier != null) { outCopier.start(); } @@ -238,15 +237,18 @@ private static final class CopierThread extends Thread { private static final int BUFSIZE = 8192; + private final AbstractPolyglotImpl polyglot; private final InputStream in; private final OutputStream out; private final byte[] buffer; - CopierThread(String name, InputStream in, OutputStream out) { + CopierThread(AbstractPolyglotImpl polyglot, String name, InputStream in, OutputStream out) { + Objects.requireNonNull(polyglot, "Polyglot must be non null."); Objects.requireNonNull(name, "Name must be non null."); Objects.requireNonNull(in, "In must be non null."); Objects.requireNonNull(out, "Out must be non null."); setName(name); + this.polyglot = polyglot; this.in = in; this.out = out; this.buffer = new byte[BUFSIZE]; @@ -255,7 +257,7 @@ private static final class CopierThread extends Thread { @Override @SuppressWarnings("try") public void run() { - try (ThreadScope scope = PolyglotImpl.getActivePolyglot().createThreadScope()) { + try (ThreadScope scope = polyglot.getRootImpl().createThreadScope()) { while (true) { if (isInterrupted()) { return;