Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Allow polyglot embedders to control throwDeniedThreadAccess #8266

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
26 changes: 23 additions & 3 deletions sdk/src/org.graalvm.polyglot/src/org/graalvm/polyglot/Context.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import java.util.Map.Entry;
import java.util.Objects;
import java.util.concurrent.TimeoutException;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.logging.Handler;
import java.util.logging.Level;
Expand Down Expand Up @@ -286,8 +287,9 @@
* instance may be used from multiple threads at the same time depends on if all initialized
* languages support it. If initialized languages support multi-threading, then the context instance
* may be used from multiple threads at the same time. If a context is used from multiple threads
* and the language does not fit, then an {@link IllegalStateException} is thrown by the accessing
* method.
* and the language does not fit, then an
* {@link IllegalStateException} is thrown by the accessing
* method (unless {@link Builder#onDeniedThreadAccess} is specified).
* <p>
* Meta-data from the context's underlying {@link #getEngine() engine} can be retrieved safely by
* any thread at any time.
Expand Down Expand Up @@ -1060,6 +1062,7 @@ public final class Builder {
private ClassLoader hostClassLoader;
private boolean useSystemExit;
private SandboxPolicy sandboxPolicy;
private Consumer<RuntimeException> onDeniedThreadAccess;

Builder(String... permittedLanguages) {
Objects.requireNonNull(permittedLanguages);
Expand Down Expand Up @@ -1180,6 +1183,23 @@ public Builder allowCreateThread(boolean enabled) {
return this;
}

/** Installs handler to control what happens on multiple thread access.
* When multiple threads are accessing a context which isn't ready for
* multithreaded access an exception is yielded by default. By installing
* this {@code handler} one can control what shall happen. Either to
* throw the provided exception or resolve the multithreaded situation
* and return to retry the thread access again.
*
Copy link
Member

Choose a reason for hiding this comment

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

This could use usage examples. For example customization of the error message and or using a locking strategy?

* @param handler callback that either throws the provided {@code RuntimeException} or returns
* to signal request for retry
* @return this builder
* @since 23.2
Copy link
Member

Choose a reason for hiding this comment

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

Target version is 24.1

*/
public Builder onDeniedThreadAccess(Consumer<RuntimeException> handler) {
Copy link
Member

Choose a reason for hiding this comment

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

API changes like this need an entry in the changelog.

this.onDeniedThreadAccess = handler;
return this;
}

/**
* Sets the default value for all privileges. If not explicitly specified, then all access
* is <code>false</code>. If all access is enabled then certain privileges may still be
Expand Down Expand Up @@ -1935,7 +1955,7 @@ public Context build() {
Object logHandler = customLogHandler != null ? Engine.getImpl().newLogHandler(customLogHandler) : null;
String tmpDir = Engine.getImpl().getIO().hasHostFileAccess(useIOAccess) ? System.getProperty("java.io.tmpdir") : null;
ctx = (Context) engine.dispatch.createContext(engine.receiver, useSandboxPolicy, contextOut, contextErr, contextIn, hostClassLookupEnabled,
hostAccess, polyglotAccess, nativeAccess, createThread, hostClassLoading, innerContextOptions,
hostAccess, polyglotAccess, nativeAccess, createThread, onDeniedThreadAccess, hostClassLoading, innerContextOptions,
experimentalOptions, localHostLookupFilter, contextOptions, arguments == null ? Collections.emptyMap() : arguments,
permittedLanguages, useIOAccess, logHandler, createProcess, processHandler, useEnvironmentAccess, environment, zone, limits,
localCurrentWorkingDirectory, tmpDir, hostClassLoader, allowValueSharing, useSystemExit);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -807,7 +807,9 @@ public abstract Object createContext(Object receiver, SandboxPolicy sandboxPolic
Object hostAccess,
Object polyglotAccess,
boolean allowNativeAccess,
boolean allowCreateThread, boolean allowHostClassLoading, boolean allowInnerContextOptions, boolean allowExperimentalOptions,
boolean allowCreateThread,
Consumer<RuntimeException> onDeniedThreadAccess,
boolean allowHostClassLoading, boolean allowInnerContextOptions, boolean allowExperimentalOptions,
Predicate<String> classFilter,
Map<String, String> options,
Map<String, String[]> arguments, String[] onlyLanguages, Object ioAccess, Object logHandler, boolean allowCreateProcess, ProcessHandler processHandler,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ protected HostEngineDispatch(HostPolyglotDispatch polyglot) {
@Override
public Object createContext(Object receiver, SandboxPolicy sandboxPolicy, OutputStream out, OutputStream err, InputStream in, boolean allowHostAccess, Object hostAccess,
Object polyglotAccess,
boolean allowNativeAccess, boolean allowCreateThread, boolean allowHostClassLoading, boolean allowInnerContextOptions, boolean allowExperimentalOptions,
boolean allowNativeAccess, boolean allowCreateThread, Consumer<RuntimeException> onDeniedThreadAccess, boolean allowHostClassLoading, boolean allowInnerContextOptions,
boolean allowExperimentalOptions,
Predicate<String> classFilter, Map<String, String> options, Map<String, String[]> arguments, String[] onlyLanguages, Object ioAccess, Object logHandler,
boolean allowCreateProcess, ProcessHandler processHandler, Object environmentAccess, Map<String, String> environment, ZoneId zone, Object limitsImpl,
String currentWorkingDirectory, String tmpDir, ClassLoader hostClassLoader, boolean allowValueSharing, boolean useSystemExit) {
Expand All @@ -83,6 +84,7 @@ public Object createContext(Object receiver, SandboxPolicy sandboxPolicy, Output
AbstractEngineDispatch dispatch = api.getEngineDispatch(localEngine);
Object engineReceiver = api.getEngineReceiver(localEngine);
Context localContext = (Context) dispatch.createContext(engineReceiver, sandboxPolicy, out, err, in, allowHostAccess, hostAccess, polyglotAccess, allowNativeAccess, allowCreateThread,
onDeniedThreadAccess,
allowHostClassLoading,
allowInnerContextOptions, allowExperimentalOptions, classFilter, options, arguments, onlyLanguages, ioAccess, logHandler, allowCreateProcess, processHandler,
environmentAccess, environment, zone, limitsImpl, currentWorkingDirectory, tmpDir, hostClassLoader, true, useSystemExit);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public long remoteCreateContext(long engineId, SandboxPolicy sandboxPolicy, Stri
Object receiver = api.getEngineReceiver(engine);
AbstractEngineDispatch dispatch = api.getEngineDispatch(engine);
Context remoteContext = (Context) dispatch.createContext(receiver, sandboxPolicy, null, null, null, false, null, PolyglotAccess.NONE, false,
false, false, false, false, null, new HashMap<>(), new HashMap<>(),
false, null, false, false, false, null, new HashMap<>(), new HashMap<>(),
new String[0], IOAccess.NONE, null,
false, null, EnvironmentAccess.NONE,
null, null, null, null, tmpDir, null, true, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,7 @@ public TruffleContext createInternalContext(Object sourcePolyglotLanguageContext
}

PolyglotContextConfig innerConfig = new PolyglotContextConfig(engine, creatorConfig.sandboxPolicy, sharingEnabled, useOut, useErr, useIn,
useAllowHostLookup, usePolyglotAccess, useAllowNativeAccess, useAllowCreateThread, useAllowHostClassLoading,
useAllowHostLookup, usePolyglotAccess, useAllowNativeAccess, useAllowCreateThread, creatorConfig.onDeniedThreadAccess, useAllowHostClassLoading,
useAllowInnerContextOptions, creatorConfig.allowExperimentalOptions,
useClassFilter, useArguments, allowedLanguages, useOptions, fileSystemConfig, creatorConfig.logHandler,
useAllowCreateProcess, useProcessHandler, useEnvironmentAccess, useCustomEnvironment,
Expand Down Expand Up @@ -1150,7 +1150,14 @@ public Thread createThread(Object polyglotLanguageContext, Runnable runnable, Ob
threadContext = innerContext.getContext(threadContext.language);
}
PolyglotThread newThread = new PolyglotThread(threadContext, runnable, group, stackSize, beforeEnter, afterLeave);
threadContext.context.checkMultiThreadedAccess(newThread);
Copy link
Member

Choose a reason for hiding this comment

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

Can't you move the loop into checkMultiThreadedAccess?

for (;;) {
try {
threadContext.context.checkMultiThreadedAccess(newThread);
break;
} catch (PolyglotThreadAccessException ex) {
ex.rethrow(threadContext.context);
}
}
return newThread;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ final class PolyglotContextConfig {
final Runnable onCancelled;
final Consumer<Integer> onExited;
final Runnable onClosed;
final Consumer<RuntimeException> onDeniedThreadAccess;

/**
* Groups PolyglotContext's filesystem related configurations.
Expand Down Expand Up @@ -231,6 +232,7 @@ private static Map<String, String> computeCommonOptions(Map<String, String> opti
sharableConfig.polyglotAccess == null ? engine.getAPIAccess().getPolyglotAccessAll() : sharableConfig.polyglotAccess,
sharableConfig.nativeAccessAllowed,
sharableConfig.createThreadAllowed,
null,
false,
false,
false,
Expand All @@ -256,7 +258,7 @@ private static Map<String, String> computeCommonOptions(Map<String, String> opti
PolyglotContextConfig(PolyglotEngineImpl engine, SandboxPolicy sandboxPolicy, Boolean forceSharing,
OutputStream out, OutputStream err, InputStream in,
boolean hostLookupAllowed, Object polyglotAccess, boolean nativeAccessAllowed,
boolean createThreadAllowed, boolean hostClassLoadingAllowed,
boolean createThreadAllowed, Consumer<RuntimeException> onDeniedThreadAccess, boolean hostClassLoadingAllowed,
boolean contextOptionsAllowed, boolean allowExperimentalOptions,
Predicate<String> classFilter, Map<String, String[]> applicationArguments,
Set<String> onlyLanguages, Map<String, String> options, FileSystemConfig fileSystemConfig, LogHandler logHandler,
Expand All @@ -278,6 +280,7 @@ private static Map<String, String> computeCommonOptions(Map<String, String> opti
this.polyglotAccess = polyglotAccess;
this.nativeAccessAllowed = nativeAccessAllowed;
this.createThreadAllowed = createThreadAllowed;
this.onDeniedThreadAccess = onDeniedThreadAccess;
this.hostClassLoadingAllowed = hostClassLoadingAllowed;
this.innerContextOptionsAllowed = contextOptionsAllowed;
this.allowExperimentalOptions = allowExperimentalOptions;
Expand Down