Skip to content

Commit

Permalink
Allow hard exit during natural exit notifications executed during clo…
Browse files Browse the repository at this point in the history
…sing before closing reaches finalization stage; always throw exception when polyglot.Context#close() is called for cancelled or exited context, or cancel or exit is requested during the execution of close(); close the local contexts and the local engine for isolated contexts on an isolated engine through notifications from the isolated contexts/engine instead of directly.
  • Loading branch information
jchalou committed May 5, 2022
1 parent 85d6d7a commit 3ab9547
Show file tree
Hide file tree
Showing 48 changed files with 2,084 additions and 692 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ public boolean isActive() {
* {@link NativeIsolate} has active threads the isolate is freed by the last leaving thread.
*/
public boolean shutdown() {
NativeIsolateThread currentIsolateThread = attachedIsolateThread.get();
if (currentIsolateThread != null && currentIsolateThread.isNativeThread()) {
return false;
}
boolean deferredClose = false;
synchronized (this) {
if (state == State.DISPOSED) {
Expand Down Expand Up @@ -202,6 +206,13 @@ public void registerForCleanup(Object cleanableObject, LongPredicate cleanupActi
}
}

/*
* Returns true if the isolate shutdown process has already begun or is finished.
*/
public boolean isDisposed() {
return state == State.DISPOSED;
}

void lastLeave() {
synchronized (this) {
for (NativeIsolateThread nativeIsolateThread : threads) {
Expand Down
72 changes: 32 additions & 40 deletions docs/reference-manual/embedding/sandbox-options.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,14 @@ try (Context context = Context.newBuilder("js")
.option("sandbox.MaxCPUTime", "500ms")
.option("sandbox.MaxCPUTimeCheckInterval", "5ms")
.build();) {
try {
context.eval("js", "while(true);");
assert false;
} catch (PolyglotException e) {
// triggered after 500ms;
// context is closed and can no longer be used
// error message: Maximum CPU time limit of 500ms exceeded.
assert e.isCancelled();
assert e.isResourceExhausted();
}
context.eval("js", "while(true);");
assert false;
} catch (PolyglotException e) {
// triggered after 500ms;
// context is closed and can no longer be used
// error message: Maximum CPU time limit of 500ms exceeded.
assert e.isCancelled();
assert e.isResourceExhausted();
}
```

Expand Down Expand Up @@ -120,17 +118,15 @@ try (Context context = Context.newBuilder("js")
.option("sandbox.MaxStatements", "2")
.option("sandbox.MaxStatementsIncludeInternal", "false")
.build();) {
try {
context.eval("js", "purpose = 41");
context.eval("js", "purpose++");
context.eval("js", "purpose++"); // triggers max statements
assert false;
} catch (PolyglotException e) {
// context is closed and can no longer be used
// error message: Maximum statements limit of 2 exceeded.
assert e.isCancelled();
assert e.isResourceExhausted();
}
context.eval("js", "purpose = 41");
context.eval("js", "purpose++");
context.eval("js", "purpose++"); // triggers max statements
assert false;
} catch (PolyglotException e) {
// context is closed and can no longer be used
// error message: Maximum statements limit of 2 exceeded.
assert e.isCancelled();
assert e.isResourceExhausted();
}
```

Expand Down Expand Up @@ -178,16 +174,14 @@ try (Context context = Context.newBuilder("js")
.allowExperimentalOptions(true)
.option("sandbox.MaxHeapMemory", "100MB")
.build()) {
try {
context.eval("js", "var r = {}; var o = r; while(true) { o.o = {}; o = o.o; };");
assert false;
} catch (PolyglotException e) {
// triggered after the retained size is greater than 100MB;
// context is closed and can no longer be used
// error message: Maximum heap memory limit of 104857600 bytes exceeded. Current memory at least...
assert e.isCancelled();
assert e.isResourceExhausted();
}
context.eval("js", "var r = {}; var o = r; while(true) { o.o = {}; o = o.o; };");
assert false;
} catch (PolyglotException e) {
// triggered after the retained size is greater than 100MB;
// context is closed and can no longer be used
// error message: Maximum heap memory limit of 104857600 bytes exceeded. Current memory at least...
assert e.isCancelled();
assert e.isResourceExhausted();
}
```

Expand Down Expand Up @@ -265,14 +259,12 @@ try (Context context = Context.newBuilder("js")
.allowExperimentalOptions(true)
.option("sandbox.MaxCPUTime", "500ms")
.build();) {
try {
context.eval("js", /*... initialization script ...*/);
context.resetLimits();
context.eval("js", /*... user script ...*/);
assert false;
} catch (PolyglotException e) {
assert e.isCancelled();
assert e.isResourceExhausted();
}
context.eval("js", /*... initialization script ...*/);
context.resetLimits();
context.eval("js", /*... user script ...*/);
assert false;
} catch (PolyglotException e) {
assert e.isCancelled();
assert e.isResourceExhausted();
}
```
5 changes: 5 additions & 0 deletions sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

This changelog summarizes major changes between GraalVM SDK versions. The main focus is on APIs exported by GraalVM SDK.

## Version 22.2.0
* Changed the behavior of [`Context.close()`](https://www.graalvm.org/sdk/javadoc/org/graalvm/polyglot/Context.html#close--) (as well as `Context.close(false)` which is equivalent). In case the context was cancelled during the close operation or the context was exited during the close operation at request of the guest application, or it was already cancelled or exited before the close operation begins,
the close operation throws a [`PolyglotException`](https://www.graalvm.org/sdk/javadoc/org/graalvm/polyglot/PolyglotException) with [`PolyglotException.isCancelled()`](https://www.graalvm.org/sdk/javadoc/org/graalvm/polyglot/PolyglotException#isCancelled--) or [`PolyglotException.isExit()`](https://www.graalvm.org/sdk/javadoc/org/graalvm/polyglot/PolyglotException#isExit--), respectively, equal to `true`.


## Version 22.1.0
* Changed the default [`Object` target type mapping (`Value.as(Object.class)`)](https://www.graalvm.org/sdk/javadoc/org/graalvm/polyglot/Value.html#as-java.lang.Class-) for values that have both array elements and members from `Map` to `List`.
Note: This is an incompatible change. Embedders relying on the dynamic type `Map` after a `Object` target type coercion will have to migrate their code.
Expand Down
4 changes: 2 additions & 2 deletions sdk/src/org.graalvm.polyglot/snapshot.sigtest
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ meth public static org.graalvm.polyglot.Engine$Builder newBuilder()
meth public void close()
meth public void close(boolean)
supr java.lang.Object
hfds EMPTY,currentAPI,dispatch,initializationException,receiver
hcls APIAccessImpl,ImplHolder,PolyglotInvalid
hfds EMPTY,ENGINES,currentAPI,dispatch,initializationException,receiver,shutdownHookInitialized
hcls APIAccessImpl,EngineShutDownHook,ImplHolder,PolyglotInvalid

CLSS public final org.graalvm.polyglot.Engine$Builder
outer org.graalvm.polyglot.Engine
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -818,7 +818,10 @@ private void checkCreatorAccess(String operation) {
* {@link PolyglotException#isCancelled() cancelled}, else an
* {@link IllegalStateException} is thrown.
* @see Engine#close() close an engine.
* @throws PolyglotException in case the close failed due to a guest language error.
* @throws PolyglotException in case the close failed due to a guest language error, or, if
* cancelIfExecuting is <code>false</code>, the exception is also thrown when the
* context was {@link PolyglotException#isCancelled() cancelled} or the context was
* {@link PolyglotException#isExit() exited} at request of the guest application.
* @throws IllegalStateException if the context is still running and cancelIfExecuting is
* <code>false</code>
* @since 19.0
Expand All @@ -843,7 +846,9 @@ public void close(boolean cancelIfExecuting) {
* For convenience, before the actual closing process begins, the close method leaves the
* context on the current thread, if it was entered {@link #enter() explicitly}.
*
* @throws PolyglotException in case the close failed due to a guest language error.
* @throws PolyglotException in case the close failed due to a guest language error, or the
* context was {@link PolyglotException#isCancelled() cancelled} or the context was
* {@link PolyglotException#isExit() exited} at request of the guest application.
* @throws IllegalStateException if the context is currently executing on another thread.
* @see Engine#close() close an engine.
* @since 19.0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ public Engine build() {
throw new IllegalStateException("The Polyglot API implementation failed to load.");
}
Engine engine = polyglot.buildEngine(permittedLanguages, out, err, in, options, useSystemProperties, allowExperimentalOptions,
boundEngine, messageTransport, customLogHandler, polyglot.createHostLanguage(polyglot.createHostAccess()), false);
boundEngine, messageTransport, customLogHandler, polyglot.createHostLanguage(polyglot.createHostAccess()), false, null);
return engine;
}

Expand Down Expand Up @@ -960,7 +960,7 @@ public Context getCurrentContext() {
@Override
public Engine buildEngine(String[] permittedLanguages, OutputStream out, OutputStream err, InputStream in, Map<String, String> arguments, boolean useSystemProperties,
boolean allowExperimentalOptions, boolean boundEngine,
MessageTransport messageInterceptor, Object logHandlerOrStream, Object hostLanguage, boolean hostLanguageOnly) {
MessageTransport messageInterceptor, Object logHandlerOrStream, Object hostLanguage, boolean hostLanguageOnly, AbstractPolyglotHostService polyglotHostService) {
throw noPolyglotImplementationFound();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,10 @@ protected void initialize() {

public Engine buildEngine(String[] permittedLanguages, OutputStream out, OutputStream err, InputStream in, Map<String, String> options, boolean useSystemProperties,
boolean allowExperimentalOptions,
boolean boundEngine, MessageTransport messageInterceptor, Object logHandlerOrStream, Object hostLanguage, boolean hostLanguageOnly) {
boolean boundEngine, MessageTransport messageInterceptor, Object logHandlerOrStream, Object hostLanguage, boolean hostLanguageOnly,
AbstractPolyglotHostService polyglotHostService) {
return getNext().buildEngine(permittedLanguages, out, err, in, options, useSystemProperties, allowExperimentalOptions, boundEngine, messageInterceptor, logHandlerOrStream, hostLanguage,
hostLanguageOnly);
hostLanguageOnly, polyglotHostService);
}

public abstract int getPriority();
Expand Down Expand Up @@ -732,12 +733,31 @@ public abstract <K, V> Map.Entry<K, V> toMapEntry(Object internalContext, Object
public abstract RuntimeException unboxEngineException(RuntimeException e);
}

public abstract static class AbstractHostService extends AbstractDispatchClass {
public abstract static class AbstractPolyglotHostService extends AbstractDispatchClass {

protected AbstractHostService(AbstractPolyglotImpl polyglot) {
protected AbstractPolyglotHostService(AbstractPolyglotImpl polyglot) {
Objects.requireNonNull(polyglot);
}

public abstract void patch(AbstractPolyglotHostService otherService);

public abstract void notifyClearExplicitContextStack(Object contextReceiver);

public abstract void notifyContextCancellingOrExiting(Object contextReceiver, boolean exit, int exitCode, boolean resourceLimit, String message);

public abstract void notifyContextClosed(Object contextReceiver, boolean cancelIfExecuting, boolean resourceLimit, String message);

public abstract void notifyEngineClosed(Object engineReceiver, boolean cancelIfExecuting);
}

public abstract static class AbstractHostLanguageService extends AbstractDispatchClass {

protected AbstractHostLanguageService(AbstractPolyglotImpl polyglot) {
Objects.requireNonNull(polyglot);
}

public abstract void release();

public abstract void initializeHostContext(Object internalContext, Object context, HostAccess access, ClassLoader cl, Predicate<String> clFilter, boolean hostCLAllowed,
boolean hostLookupAllowed);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ private static void assertEntry(Iterator<StackTraceEntry> iterator, String expec

public ManualSamplingTest() {
needsInstrumentEnv = true;
ignoreCancelOnClose = true;
}

@Test
Expand Down
Loading

0 comments on commit 3ab9547

Please sign in to comment.