-
Notifications
You must be signed in to change notification settings - Fork 304
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Patch v8 to support promise cross-context resolution
``` export default { async fetch(req, env, ctx) { if (globalThis.resolve === undefined) { { const { promise, resolve } = Promise.withResolvers(); setTimeout(resolve, 1000); ctx.waitUntil(promise); } // This is our first request. We will create a new promise and resolver // pair and store it in a global. This request will then wait for the // promise to be resolved before continuing. const { promise, resolve } = Promise.withResolvers(); globalThis.resolve = resolve; globalThis.promise = promise; const ab = AbortSignal.abort(); console.log(ab.aborted); promise.then(() => { // This will be run within the correct IoContext now... try { console.log('test1', ab.aborted); } catch (err) { // We would get here if the IoContext was incorrect because // of the call to ab.aborted console.log(err.message); } }); } else { // This is our second request. We will resolve the promise created in the // first request. console.log('....'); globalThis.resolve(); globalThis.resolve = undefined; console.log('test2'); } return new Response("Hello World\n"); } }; ```
- Loading branch information
Showing
9 changed files
with
885 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
339 changes: 339 additions & 0 deletions
339
patches/v8/0019-Implement-cross-request-context-promise-resolve-hand.patch
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,339 @@ | ||
From dd45ea67f36ecc324a60dac15e4e673ba6610f0e Mon Sep 17 00:00:00 2001 | ||
From: James M Snell <jasnell@gmail.com> | ||
Date: Mon, 16 Sep 2024 09:56:04 -0700 | ||
Subject: [PATCH] Implement cross-request context promise resolve handling | ||
|
||
--- | ||
include/v8-callbacks.h | 21 +++++++++++ | ||
include/v8-isolate.h | 2 ++ | ||
src/api/api.cc | 5 +++ | ||
src/builtins/promise-abstract-operations.tq | 6 +++- | ||
src/builtins/promise-resolve.tq | 4 ++- | ||
src/execution/isolate-inl.h | 9 +++++ | ||
src/execution/isolate.cc | 22 ++++++++++++ | ||
src/execution/isolate.h | 14 +++++++- | ||
src/objects/js-promise.h | 5 +++ | ||
src/objects/objects.cc | 39 +++++++++++++++++++++ | ||
src/runtime/runtime-promise.cc | 18 ++++++++++ | ||
src/runtime/runtime.h | 3 +- | ||
12 files changed, 144 insertions(+), 4 deletions(-) | ||
|
||
diff --git a/include/v8-callbacks.h b/include/v8-callbacks.h | ||
index 25f0c8189e5..7fcb1602df3 100644 | ||
--- a/include/v8-callbacks.h | ||
+++ b/include/v8-callbacks.h | ||
@@ -471,6 +471,27 @@ using PromiseCrossContextCallback = MaybeLocal<Promise> (*)(Local<Context> conte | ||
Local<Promise> promise, | ||
Local<Object> tag); | ||
|
||
+/** | ||
+ * PromiseCrossContextResolveCallback is called when resolving or rejecting a | ||
+ * pending promise whose context tag is not strictly equal to the isolate's | ||
+ * current promise context tag. The callback is called with the promise to be | ||
+ * resolved, it's context tag, and a function that when called, causes the | ||
+ * reactions to the resolved promise to be enqueued. The idea is that the | ||
+ * embedder sets this callback in the case it needs to defer the actual | ||
+ * scheduling of the reactions to the given promise to a later time. | ||
+ * Importantly, when this callback is invoked, the state of the promise | ||
+ * should have already been updated. We're simply possibly deferring the | ||
+ * enqueue of the reactions to the promise. | ||
+ */ | ||
+using PromiseCrossContextResolveCallback = void (*)( | ||
+ v8::Isolate* isolate, | ||
+ Local<Value> tag, | ||
+ Local<Data> reactions, | ||
+ Local<Value> argument, | ||
+ std::function<void(v8::Isolate* isolate, | ||
+ Local<Data> reactions, | ||
+ Local<Value> argument)> callback); | ||
+ | ||
} // namespace v8 | ||
|
||
#endif // INCLUDE_V8_ISOLATE_CALLBACKS_H_ | ||
diff --git a/include/v8-isolate.h b/include/v8-isolate.h | ||
index 8112231319b..9fe57f7bde4 100644 | ||
--- a/include/v8-isolate.h | ||
+++ b/include/v8-isolate.h | ||
@@ -1732,6 +1732,8 @@ class V8_EXPORT Isolate { | ||
|
||
class PromiseContextScope; | ||
void SetPromiseCrossContextCallback(PromiseCrossContextCallback callback); | ||
+ void SetPromiseCrossContextResolveCallback( | ||
+ PromiseCrossContextResolveCallback callback); | ||
|
||
Isolate() = delete; | ||
~Isolate() = delete; | ||
diff --git a/src/api/api.cc b/src/api/api.cc | ||
index d3fc6d77ea3..3255fd77e25 100644 | ||
--- a/src/api/api.cc | ||
+++ b/src/api/api.cc | ||
@@ -12246,6 +12246,11 @@ void Isolate::SetPromiseCrossContextCallback(PromiseCrossContextCallback callbac | ||
isolate->set_promise_cross_context_callback(callback); | ||
} | ||
|
||
+void Isolate::SetPromiseCrossContextResolveCallback(PromiseCrossContextResolveCallback callback) { | ||
+ i::Isolate* isolate = reinterpret_cast<i::Isolate*>(this); | ||
+ isolate->set_promise_cross_context_resolve_callback(callback); | ||
+} | ||
+ | ||
Isolate::PromiseContextScope::PromiseContextScope(Isolate* isolate, v8::Local<v8::Object> tag) | ||
: isolate_(reinterpret_cast<i::Isolate*>(isolate)) { | ||
DCHECK(!isolate_->has_promise_context_tag()); | ||
diff --git a/src/builtins/promise-abstract-operations.tq b/src/builtins/promise-abstract-operations.tq | ||
index fdec3d268e4..456d051d3c0 100644 | ||
--- a/src/builtins/promise-abstract-operations.tq | ||
+++ b/src/builtins/promise-abstract-operations.tq | ||
@@ -23,6 +23,9 @@ extern transitioning runtime PromiseRejectEventFromStack( | ||
|
||
extern transitioning runtime PromiseContextCheck( | ||
implicit context: Context)(JSPromise): JSPromise; | ||
+ | ||
+extern transitioning runtime PromiseResolveContextCheck( | ||
+ implicit context: Context)(JSPromise): JSAny; | ||
} | ||
|
||
// https://tc39.es/ecma262/#sec-promise-abstract-operations | ||
@@ -239,7 +242,8 @@ transitioning builtin RejectPromise( | ||
// the runtime handle this operation, which greatly reduces | ||
// the complexity here and also avoids a couple of back and | ||
// forth between JavaScript and C++ land. | ||
- if (IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate( | ||
+ if (ToBoolean(runtime::PromiseResolveContextCheck(promise)) || | ||
+ IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate( | ||
promiseHookFlags) || | ||
!promise.HasHandler()) { | ||
// 7. If promise.[[PromiseIsHandled]] is false, perform | ||
diff --git a/src/builtins/promise-resolve.tq b/src/builtins/promise-resolve.tq | ||
index 202180adbba..c93ac5905d7 100644 | ||
--- a/src/builtins/promise-resolve.tq | ||
+++ b/src/builtins/promise-resolve.tq | ||
@@ -96,7 +96,9 @@ transitioning builtin ResolvePromise( | ||
// We also let the runtime handle it if promise == resolution. | ||
// We can use pointer comparison here, since the {promise} is guaranteed | ||
// to be a JSPromise inside this function and thus is reference comparable. | ||
- if (IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate() || | ||
+ | ||
+ if (ToBoolean(runtime::PromiseResolveContextCheck(promise)) || | ||
+ IsIsolatePromiseHookEnabledOrDebugIsActiveOrHasAsyncEventDelegate() || | ||
TaggedEqual(promise, resolution)) | ||
deferred { | ||
return runtime::ResolvePromise(promise, resolution); | ||
diff --git a/src/execution/isolate-inl.h b/src/execution/isolate-inl.h | ||
index 9e0969460fd..1084adfa7f5 100644 | ||
--- a/src/execution/isolate-inl.h | ||
+++ b/src/execution/isolate-inl.h | ||
@@ -154,6 +154,15 @@ void Isolate::set_promise_cross_context_callback(PromiseCrossContextCallback cal | ||
promise_cross_context_callback_ = callback; | ||
} | ||
|
||
+void Isolate::set_promise_cross_context_resolve_callback( | ||
+ PromiseCrossContextResolveCallback callback) { | ||
+ promise_cross_context_resolve_callback_ = callback; | ||
+} | ||
+ | ||
+bool Isolate::has_promise_context_resolve_callback() { | ||
+ return promise_cross_context_resolve_callback_ != nullptr; | ||
+} | ||
+ | ||
#ifdef DEBUG | ||
Tagged<Object> Isolate::VerifyBuiltinsResult(Tagged<Object> result) { | ||
if (is_execution_terminating() && !v8_flags.strict_termination_checks) { | ||
diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc | ||
index 94194d59bae..cfba2a44966 100644 | ||
--- a/src/execution/isolate.cc | ||
+++ b/src/execution/isolate.cc | ||
@@ -7459,5 +7459,27 @@ MaybeHandle<JSPromise> Isolate::RunPromiseCrossContextCallback(Handle<NativeCont | ||
return v8::Utils::OpenHandle(*result); | ||
} | ||
|
||
+void Isolate::RunPromiseCrossContextResolveCallback(v8::Isolate* isolate, | ||
+ Handle<JSObject> tag, | ||
+ Handle<Object> reactions, | ||
+ Handle<Object> argument, | ||
+ PromiseReaction::Type type) { | ||
+ CHECK(promise_cross_context_resolve_callback_ != nullptr); | ||
+ promise_cross_context_resolve_callback_( | ||
+ isolate, | ||
+ v8::Utils::ToLocal(tag), | ||
+ v8::Utils::ToLocal(reactions), | ||
+ v8::Utils::ToLocal(argument), | ||
+ [type](v8::Isolate* isolate, | ||
+ v8::Local<v8::Data> reactions, | ||
+ v8::Local<v8::Value> argument) { | ||
+ JSPromise::ContinueTriggerPromiseReactions( | ||
+ reinterpret_cast<Isolate*>(isolate), | ||
+ Utils::OpenHandle(*reactions), | ||
+ Utils::OpenHandle(*argument), | ||
+ type); | ||
+ }); | ||
+} | ||
+ | ||
} // namespace internal | ||
} // namespace v8 | ||
diff --git a/src/execution/isolate.h b/src/execution/isolate.h | ||
index 59926e68e97..2bd24b914b0 100644 | ||
--- a/src/execution/isolate.h | ||
+++ b/src/execution/isolate.h | ||
@@ -42,6 +42,7 @@ | ||
#include "src/objects/contexts.h" | ||
#include "src/objects/debug-objects.h" | ||
#include "src/objects/js-objects.h" | ||
+#include "src/objects/promise.h" | ||
#include "src/objects/tagged.h" | ||
#include "src/runtime/runtime.h" | ||
#include "src/sandbox/code-pointer-table.h" | ||
@@ -2249,8 +2250,17 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { | ||
inline void clear_promise_context_tag(); | ||
inline void set_promise_context_tag(Tagged<Object> tag); | ||
inline void set_promise_cross_context_callback(PromiseCrossContextCallback callback); | ||
+ inline void set_promise_cross_context_resolve_callback( | ||
+ PromiseCrossContextResolveCallback callback); | ||
MaybeHandle<JSPromise> RunPromiseCrossContextCallback(Handle<NativeContext> context, | ||
Handle<JSPromise> promise); | ||
+ void RunPromiseCrossContextResolveCallback(v8::Isolate* isolate, | ||
+ Handle<JSObject> tag, | ||
+ Handle<Object> reactions, | ||
+ Handle<Object> argument, | ||
+ PromiseReaction::Type type); | ||
+ | ||
+ inline bool has_promise_context_resolve_callback(); | ||
|
||
#ifdef V8_ENABLE_WASM_SIMD256_REVEC | ||
void set_wasm_revec_verifier_for_test( | ||
@@ -2764,8 +2774,10 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { | ||
#endif | ||
|
||
Tagged<Object> promise_context_tag_; | ||
- PromiseCrossContextCallback promise_cross_context_callback_; | ||
+ PromiseCrossContextCallback promise_cross_context_callback_ = nullptr; | ||
+ PromiseCrossContextResolveCallback promise_cross_context_resolve_callback_ = nullptr; | ||
bool in_promise_cross_context_callback_ = false; | ||
+ bool in_promise_cross_context_resolve_callback_ = false; | ||
|
||
class PromiseCrossContextCallbackScope; | ||
|
||
diff --git a/src/objects/js-promise.h b/src/objects/js-promise.h | ||
index e4c1e42c63e..87bebb70d0d 100644 | ||
--- a/src/objects/js-promise.h | ||
+++ b/src/objects/js-promise.h | ||
@@ -75,6 +75,11 @@ class JSPromise | ||
static_assert(v8::Promise::kFulfilled == 1); | ||
static_assert(v8::Promise::kRejected == 2); | ||
|
||
+ static void ContinueTriggerPromiseReactions(Isolate* isolate, | ||
+ DirectHandle<Object> reactions, | ||
+ DirectHandle<Object> argument, | ||
+ PromiseReaction::Type type); | ||
+ | ||
private: | ||
// ES section #sec-triggerpromisereactions | ||
static Handle<Object> TriggerPromiseReactions(Isolate* isolate, | ||
diff --git a/src/objects/objects.cc b/src/objects/objects.cc | ||
index 76e284e19a2..32ccc84d8f8 100644 | ||
--- a/src/objects/objects.cc | ||
+++ b/src/objects/objects.cc | ||
@@ -4878,6 +4878,21 @@ Handle<Object> JSPromise::Fulfill(DirectHandle<JSPromise> promise, | ||
// 6. Set promise.[[PromiseState]] to "fulfilled". | ||
promise->set_status(Promise::kFulfilled); | ||
|
||
+ Tagged<Object> obj = promise->context_tag(); | ||
+ bool needs_promise_context_switch = | ||
+ !(obj == Smi::zero() || | ||
+ obj == isolate->promise_context_tag() || | ||
+ !isolate->has_promise_context_resolve_callback()); | ||
+ if (needs_promise_context_switch) { | ||
+ isolate->RunPromiseCrossContextResolveCallback( | ||
+ reinterpret_cast<v8::Isolate*>(isolate), | ||
+ Handle<JSObject>(Cast<JSObject>(obj), isolate), | ||
+ reactions, | ||
+ value, | ||
+ PromiseReaction::kFulfill); | ||
+ return isolate->factory()->undefined_value(); | ||
+ } | ||
+ | ||
// 7. Return TriggerPromiseReactions(reactions, value). | ||
return TriggerPromiseReactions(isolate, reactions, value, | ||
PromiseReaction::kFulfill); | ||
@@ -4933,6 +4948,21 @@ Handle<Object> JSPromise::Reject(Handle<JSPromise> promise, | ||
isolate->ReportPromiseReject(promise, reason, kPromiseRejectWithNoHandler); | ||
} | ||
|
||
+ Tagged<Object> obj = promise->context_tag(); | ||
+ bool needs_promise_context_switch = | ||
+ !(obj == Smi::zero() || | ||
+ obj == isolate->promise_context_tag() || | ||
+ !isolate->has_promise_context_resolve_callback()); | ||
+ if (needs_promise_context_switch) { | ||
+ isolate->RunPromiseCrossContextResolveCallback( | ||
+ reinterpret_cast<v8::Isolate*>(isolate), | ||
+ Handle<JSObject>(Cast<JSObject>(obj), isolate), | ||
+ reactions, | ||
+ reason, | ||
+ PromiseReaction::kReject); | ||
+ return isolate->factory()->undefined_value(); | ||
+ } | ||
+ | ||
// 8. Return TriggerPromiseReactions(reactions, reason). | ||
return TriggerPromiseReactions(isolate, reactions, reason, | ||
PromiseReaction::kReject); | ||
@@ -5035,6 +5065,15 @@ MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise, | ||
} | ||
|
||
// static | ||
+ | ||
+void JSPromise::ContinueTriggerPromiseReactions( | ||
+ Isolate* isolate, | ||
+ DirectHandle<Object> reactions, | ||
+ DirectHandle<Object> argument, | ||
+ PromiseReaction::Type type) { | ||
+ TriggerPromiseReactions(isolate, reactions, argument, type); | ||
+} | ||
+ | ||
Handle<Object> JSPromise::TriggerPromiseReactions( | ||
Isolate* isolate, DirectHandle<Object> reactions, | ||
DirectHandle<Object> argument, PromiseReaction::Type type) { | ||
diff --git a/src/runtime/runtime-promise.cc b/src/runtime/runtime-promise.cc | ||
index 4fbdf90319c..bb4e9c7fe41 100644 | ||
--- a/src/runtime/runtime-promise.cc | ||
+++ b/src/runtime/runtime-promise.cc | ||
@@ -251,5 +251,23 @@ RUNTIME_FUNCTION(Runtime_PromiseContextCheck) { | ||
return *result; | ||
} | ||
|
||
+RUNTIME_FUNCTION(Runtime_PromiseResolveContextCheck) { | ||
+ HandleScope scope(isolate); | ||
+ DCHECK_EQ(1, args.length()); | ||
+ Handle<JSPromise> promise = args.at<JSPromise>(0); | ||
+ // If promise.context_tag() is strict equal to isolate.promise_context_tag(), | ||
+ // or if the promise being checked does not have a context tag, or if the | ||
+ // resolve callback has not been set, we'll just return false here to indicate | ||
+ // that the default handling should be used. | ||
+ Tagged<Object> obj = promise->context_tag(); | ||
+ if (obj == Smi::zero() || | ||
+ obj == isolate->promise_context_tag() || | ||
+ !isolate->has_promise_context_resolve_callback()) { | ||
+ return isolate->heap()->ToBoolean(false); | ||
+ } | ||
+ | ||
+ return isolate->heap()->ToBoolean(true); | ||
+} | ||
+ | ||
} // namespace internal | ||
} // namespace v8 | ||
diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h | ||
index 9399509f690..bfca9182be0 100644 | ||
--- a/src/runtime/runtime.h | ||
+++ b/src/runtime/runtime.h | ||
@@ -403,7 +403,8 @@ namespace internal { | ||
F(ConstructAggregateErrorHelper, 4, 1) \ | ||
F(ConstructInternalAggregateErrorHelper, -1 /* <= 5*/, 1) \ | ||
F(PromiseContextInit, 1, 1) \ | ||
- F(PromiseContextCheck, 1, 1) | ||
+ F(PromiseContextCheck, 1, 1) \ | ||
+ F(PromiseResolveContextCheck, 1, 1) | ||
|
||
#define FOR_EACH_INTRINSIC_PROXY(F, I) \ | ||
F(CheckProxyGetSetTrapResult, 2, 1) \ | ||
-- | ||
2.34.1 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.