Skip to content

Commit

Permalink
[GR-49319] Corrections of promise hook events.
Browse files Browse the repository at this point in the history
PullRequest: js/2939
  • Loading branch information
iamstolis committed Oct 9, 2023
2 parents 6dd82d9 + 4d8a0bf commit c5a9c0a
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ protected final Object suspendAwait(VirtualFrame frame, Object value) {
JSPromiseObject promise = promiseResolve(value);
JSFunctionObject onFulfilled = createAwaitFulfilledFunction(resumeTarget, asyncContext, generatorOrCapability);
JSFunctionObject onRejected = createAwaitRejectedFunction(resumeTarget, asyncContext, generatorOrCapability);
context.notifyPromiseHook(-1 /* parent info */, promise);
PromiseCapabilityRecord throwawayCapability = newThrowawayCapability();

fillAsyncStackTrace(frame, onFulfilled, onRejected);
context.notifyPromiseHook(-1 /* parent info */, promise);

echoInput(frame, promise);
performPromiseThenNode.execute(promise, onFulfilled, onRejected, throwawayCapability);
Expand Down Expand Up @@ -221,10 +221,11 @@ private JSPromiseObject promiseResolve(Object value) {
}

private PromiseCapabilityRecord newThrowawayCapability() {
if (context.getEcmaScriptVersion() >= JSConfig.ECMAScript2019) {
if (context.getEcmaScriptVersion() >= JSConfig.ECMAScript2019 && !context.hasPromiseHook()) {
return null;
}
PromiseCapabilityRecord throwawayCapability = newPromiseCapability();
throwawayCapability.markAsThrowaway();
((JSPromiseObject) throwawayCapability.getPromise()).setIsHandled(true);
return throwawayCapability;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ public Object execute(VirtualFrame frame) {
return Undefined.instance;
}
alreadyResolved.value = true;
context.notifyPromiseHook(PromiseHook.TYPE_RESOLVE, promise);

return rejectPromiseNode.execute(promise, reason);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ protected List<TruffleStackTraceElement> findAsynchronousFrames(Frame frame) {
JSDynamicObject functionObject = JSFrameUtil.getFunctionObject(frame);
PromiseReactionRecord reaction = (PromiseReactionRecord) getReaction.getValue(functionObject);
PromiseCapabilityRecord promiseCapability = reaction.getCapability();
if (promiseCapability != null) {
if (promiseCapability != null && !promiseCapability.isThrowaway()) {
return AwaitNode.findAsyncStackFramesFromPromise(promiseCapability.getPromise());
} else if (reaction.getHandler() != null && reaction.getHandler().callback() instanceof JSFunctionObject callbackFunction) {
return AwaitNode.findAsyncStackFramesFromHandler(callbackFunction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import com.oracle.truffle.js.runtime.JSArguments;
import com.oracle.truffle.js.runtime.JSContext;
import com.oracle.truffle.js.runtime.JobCallback;
import com.oracle.truffle.js.runtime.PromiseHook;
import com.oracle.truffle.js.runtime.builtins.JSPromiseObject;
import com.oracle.truffle.js.runtime.objects.JSDynamicObject;
import com.oracle.truffle.js.runtime.objects.Undefined;
Expand Down Expand Up @@ -92,9 +93,11 @@ public Object execute(JSPromiseObject promiseToResolve, Object thenable, JobCall
JSAgent agent = getRealm().getAgent();
try {
var previousContextMapping = agent.asyncContextSwap(then.asyncContextSnapshot());
context.notifyPromiseHook(PromiseHook.TYPE_BEFORE, promiseToResolve);
try {
return callResolveNode.executeCall(JSArguments.create(thenable, then.callback(), resolve, reject));
} finally {
context.notifyPromiseHook(PromiseHook.TYPE_AFTER, promiseToResolve);
agent.asyncContextSwap(previousContextMapping);
}
} catch (AbstractTruffleException ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1646,15 +1646,24 @@ private void invalidatePromiseHookNotUsedAssumption() {
}
}

public final boolean hasPromiseHook() {
return !promiseHookNotUsedAssumption.isValid() && (promiseHook != null);
}

public final void notifyPromiseHook(int changeType, JSDynamicObject promise) {
if (!promiseHookNotUsedAssumption.isValid() && promiseHook != null) {
if (hasPromiseHook()) {
JSRealm realm = JSRealm.getMain(null);
if (changeType == -1) {
// Information about parent for the incoming INIT event
realm.storeParentPromise(promise);
} else {
JSDynamicObject parent = (changeType == PromiseHook.TYPE_INIT) ? realm.fetchParentPromise() : Undefined.instance;
notifyPromiseHookImpl(changeType, promise, parent);
try {
notifyPromiseHookImpl(changeType, promise, parent);
} catch (GraalJSException ex) {
// Resembles ReportMessageFromMicrotask()
notifyPromiseRejectionTracker(JSPromise.create(this, getRealm()), JSPromise.REJECTION_TRACKER_OPERATION_REJECT, ex.getErrorObject());
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2023, 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
Expand Down Expand Up @@ -44,6 +44,7 @@ public final class PromiseCapabilityRecord {
private JSDynamicObject promise;
private Object resolve;
private Object reject;
private boolean throwaway;

private PromiseCapabilityRecord(JSDynamicObject promise, JSDynamicObject resolve, JSDynamicObject reject) {
this.promise = promise;
Expand Down Expand Up @@ -78,4 +79,13 @@ public void setResolve(Object resolve) {
public void setReject(Object reject) {
this.reject = reject;
}

public void markAsThrowaway() {
this.throwaway = true;
}

public boolean isThrowaway() {
return this.throwaway;
}

}
6 changes: 0 additions & 6 deletions graal-nodejs/test/async-hooks/async-hooks.status
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,10 @@ prefix async-hooks

[true] # This section applies to all platforms

# PromiseHook events in graal-nodejs (for await) do not match the events generated by V8
test-async-local-storage-async-await: FAIL
test-async-local-storage-async-functions: FAIL

# Randomly failing GC-based test
test-async-local-storage-gcable: SKIP

# Unclassified
test-async-local-storage-errors: FAIL
test-async-local-storage-thenable: FAIL
test-destroy-not-blocked: FAIL

[$system==win32]
Expand Down
16 changes: 16 additions & 0 deletions graal-nodejs/test/graal/unit/other.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
* SOFTWARE.
*/

var async_hooks = require('async_hooks');
var assert = require('assert');
var fs = require('fs');
var module = require('./_unit');
Expand Down Expand Up @@ -110,4 +111,19 @@ describe('Other', function () {
it('should not define FinalizationGroup', function () {
assert.strictEqual(global.FinalizationGroup, undefined);
});
it('should keep local storage after await', function(done) {
const { AsyncLocalStorage } = async_hooks;
const asyncLocalStorage = new AsyncLocalStorage();

asyncLocalStorage.run({id: 42}, async () => {
try {
assert.strictEqual(asyncLocalStorage.getStore().id, 42);
await 211;
assert.strictEqual(asyncLocalStorage.getStore().id, 42);
done();
} catch (err) {
done(err);
}
});
});
});
8 changes: 0 additions & 8 deletions graal-nodejs/test/parallel/parallel.status
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ prefix parallel

### Graal.js-specific ###

# PromiseHook events in graal-nodejs (for await) do not match the events generated by V8
test-async-hooks-correctly-switch-promise-hook: FAIL
test-async-hooks-execution-async-resource-await: FAIL

# Triggers stack-overflow that may not be handled well
test-async-wrap-pop-id-during-load: SKIP
test-promise-reject-callback-exception: SKIP
Expand Down Expand Up @@ -183,9 +179,7 @@ test-trace-events-worker-metadata: SKIP
test-trace-events-worker-metadata-with-name: SKIP

# Unclassified
test-async-hooks-async-await: FAIL
test-async-hooks-http-parser-destroy: SKIP
test-async-local-storage-contexts: FAIL
test-crypto-op-during-process-exit: SKIP
test-crypto-sign-verify: FAIL
test-domain-no-error-handler-abort-on-uncaught-0: FAIL
Expand All @@ -206,8 +200,6 @@ test-gc-tls-external-memory: SKIP
test-http-server-keepalive-req-gc: SKIP
test-http2-response-splitting: SKIP
test-net-connect-memleak: FAIL
test-promise-hook-exceptions: FAIL
test-promise-hook-on-resolve: FAIL
test-repl: FAIL
test-runner-output: FAIL
test-runner-watch-mode: SKIP
Expand Down

0 comments on commit c5a9c0a

Please sign in to comment.