Skip to content
Permalink
Browse files
deps: V8: backport ff8d67c88449
Original commit message:

    Reland "Fix Context PromiseHook behaviour with debugger enabled"

    This is a reland of commit 872b7faa32d837f9b166d750328357f856168e72

    Original change's description:
    > Fix Context PromiseHook behaviour with debugger enabled
    >
    > This is a solution for #43148.
    >
    > Due to differences in behaviour between code with and without the debugger enabled, some promise lifecycle events were being missed and some extra ones were being added. This change resolves this and verifies the event sequence is consistent between code with and without the debugger.
    >
    > Change-Id: I3dabf1dceb14233226b1752083d659f1c2f97966
    > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3779922
    > Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    > Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    > Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    > Cr-Commit-Position: refs/heads/main@{#82132}

    Change-Id: Ifdd407261c793887fbd012d5a04ba36b3744c349
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3805979
    Reviewed-by: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Clemens Backes <clemensb@chromium.org>
    Commit-Queue: Camillo Bruni <cbruni@chromium.org>
    Reviewed-by: Victor Gomes <victorgomes@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#82575}

Refs: v8/v8@ff8d67c
Fixes: #43148
Fixes: #44415
PR-URL: #44423
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
  • Loading branch information
targos authored and RafaelGSS committed Sep 7, 2022
1 parent 4c33e5d commit c5630ad1a79cce11e7351ebe8d6b4844d9b50f08
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 6 deletions.
@@ -36,7 +36,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.11',
'v8_embedder_string': '-node.12',

##### V8 defaults for Node.js #####

@@ -478,30 +478,38 @@ void MicrotaskQueueBuiltinsAssembler::RewindEnteredContext(
void MicrotaskQueueBuiltinsAssembler::RunAllPromiseHooks(
PromiseHookType type, TNode<Context> context,
TNode<HeapObject> promise_or_capability) {
Label hook(this, Label::kDeferred), done_hook(this);
TNode<Uint32T> promiseHookFlags = PromiseHookFlags();
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
Label hook(this, Label::kDeferred), done_hook(this);
Branch(NeedsAnyPromiseHooks(promiseHookFlags), &hook, &done_hook);
BIND(&hook);
{
#endif
switch (type) {
case PromiseHookType::kBefore:
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
RunContextPromiseHookBefore(context, promise_or_capability,
promiseHookFlags);
#endif
RunPromiseHook(Runtime::kPromiseHookBefore, context,
promise_or_capability, promiseHookFlags);
break;
case PromiseHookType::kAfter:
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
RunContextPromiseHookAfter(context, promise_or_capability,
promiseHookFlags);
#endif
RunPromiseHook(Runtime::kPromiseHookAfter, context,
promise_or_capability, promiseHookFlags);
break;
default:
UNREACHABLE();
}
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
Goto(&done_hook);
}
BIND(&done_hook);
#endif
}

void MicrotaskQueueBuiltinsAssembler::RunPromiseHook(
@@ -2218,6 +2218,16 @@ void Shell::AsyncHooksTriggerAsyncId(
PerIsolateData::Get(isolate)->GetAsyncHooks()->GetTriggerAsyncId()));
}

static v8::debug::DebugDelegate dummy_delegate;

void Shell::EnableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args) {
v8::debug::SetDebugDelegate(args.GetIsolate(), &dummy_delegate);
}

void Shell::DisableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args) {
v8::debug::SetDebugDelegate(args.GetIsolate(), nullptr);
}

void Shell::SetPromiseHooks(const v8::FunctionCallbackInfo<v8::Value>& args) {
Isolate* isolate = args.GetIsolate();
if (i::FLAG_correctness_fuzzer_suppressions) {
@@ -3162,6 +3172,18 @@ Local<ObjectTemplate> Shell::CreateD8Template(Isolate* isolate) {
Local<Signature>(), 4));
d8_template->Set(isolate, "promise", promise_template);
}
{
Local<ObjectTemplate> debugger_template = ObjectTemplate::New(isolate);
debugger_template->Set(
isolate, "enable",
FunctionTemplate::New(isolate, EnableDebugger, Local<Value>(),
Local<Signature>(), 0));
debugger_template->Set(
isolate, "disable",
FunctionTemplate::New(isolate, DisableDebugger, Local<Value>(),
Local<Signature>(), 0));
d8_template->Set(isolate, "debugger", debugger_template);
}
return d8_template;
}

@@ -569,6 +569,9 @@ class Shell : public i::AllStatic {

static void SetPromiseHooks(const v8::FunctionCallbackInfo<v8::Value>& args);

static void EnableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args);
static void DisableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args);

static void Print(const v8::FunctionCallbackInfo<v8::Value>& args);
static void PrintErr(const v8::FunctionCallbackInfo<v8::Value>& args);
static void WriteStdout(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -4958,9 +4958,11 @@ void Isolate::SetPromiseHook(PromiseHook hook) {
void Isolate::RunAllPromiseHooks(PromiseHookType type,
Handle<JSPromise> promise,
Handle<Object> parent) {
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
if (HasContextPromiseHooks()) {
native_context()->RunPromiseHook(type, promise, parent);
}
#endif
if (HasIsolatePromiseHooks() || HasAsyncEventDelegate()) {
RunPromiseHook(type, promise, parent);
}
@@ -4977,7 +4979,7 @@ void Isolate::RunPromiseHook(PromiseHookType type, Handle<JSPromise> promise,
void Isolate::OnAsyncFunctionSuspended(Handle<JSPromise> promise,
Handle<JSPromise> parent) {
DCHECK_EQ(0, promise->async_task_id());
RunPromiseHook(PromiseHookType::kInit, promise, parent);
RunAllPromiseHooks(PromiseHookType::kInit, promise, parent);
if (HasAsyncEventDelegate()) {
DCHECK_NE(nullptr, async_event_delegate_);
promise->set_async_task_id(++async_task_count_);
@@ -551,6 +551,7 @@ STATIC_ASSERT(NativeContext::kSize ==
(Context::SizeFor(NativeContext::NATIVE_CONTEXT_SLOTS) +
kSystemPointerSize));

#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
void NativeContext::RunPromiseHook(PromiseHookType type,
Handle<JSPromise> promise,
Handle<Object> parent) {
@@ -606,6 +607,7 @@ void NativeContext::RunPromiseHook(PromiseHookType type,
isolate->clear_pending_exception();
}
}
#endif

} // namespace internal
} // namespace v8
@@ -781,8 +781,10 @@ class NativeContext : public Context {
void IncrementErrorsThrown();
int GetErrorsThrown();

#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
void RunPromiseHook(PromiseHookType type, Handle<JSPromise> promise,
Handle<Object> parent);
#endif

private:
STATIC_ASSERT(OffsetOfElementAt(EMBEDDER_DATA_INDEX) ==
@@ -5398,6 +5398,14 @@ Handle<Object> JSPromise::Fulfill(Handle<JSPromise> promise,
Handle<Object> value) {
Isolate* const isolate = promise->GetIsolate();

#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
if (isolate->HasContextPromiseHooks()) {
isolate->raw_native_context().RunPromiseHook(
PromiseHookType::kResolve, promise,
isolate->factory()->undefined_value());
}
#endif

// 1. Assert: The value of promise.[[PromiseState]] is "pending".
CHECK_EQ(Promise::kPending, promise->status());

@@ -5477,8 +5485,8 @@ MaybeHandle<Object> JSPromise::Resolve(Handle<JSPromise> promise,
DCHECK(
!reinterpret_cast<v8::Isolate*>(isolate)->GetCurrentContext().IsEmpty());

isolate->RunAllPromiseHooks(PromiseHookType::kResolve, promise,
isolate->factory()->undefined_value());
isolate->RunPromiseHook(PromiseHookType::kResolve, promise,
isolate->factory()->undefined_value());

// 7. If SameValue(resolution, promise) is true, then
if (promise.is_identical_to(resolution)) {
@@ -220,7 +220,7 @@ function optimizerBailout(test, verify) {
d8.promise.setHooks();
}

if (has_promise_hooks) {
function doTest () {
optimizerBailout(async () => {
await Promise.resolve();
}, () => {
@@ -234,6 +234,19 @@ if (has_promise_hooks) {
assertNextEvent('after', [ 3 ]);
assertEmptyLog();
});
optimizerBailout(async () => {
await Promise.reject();
}, () => {
assertNextEvent('init', [ 1 ]);
assertNextEvent('init', [ 2 ]);
assertNextEvent('resolve', [ 2 ]);
assertNextEvent('init', [ 3, 2 ]);
assertNextEvent('before', [ 3 ]);
assertNextEvent('resolve', [ 1 ]);
assertNextEvent('resolve', [ 3 ]);
assertNextEvent('after', [ 3 ]);
assertEmptyLog();
});
optimizerBailout(async () => {
await { then (cb) { cb() } };
}, () => {
@@ -249,6 +262,21 @@ if (has_promise_hooks) {
assertNextEvent('after', [ 3 ]);
assertEmptyLog();
});
optimizerBailout(async () => {
await { then (_, cb) { cb() } };
}, () => {
assertNextEvent('init', [ 1 ]);
assertNextEvent('init', [ 2, 1 ]);
assertNextEvent('init', [ 3, 2 ]);
assertNextEvent('before', [ 2 ]);
assertNextEvent('resolve', [ 2 ]);
assertNextEvent('after', [ 2 ]);
assertNextEvent('before', [ 3 ]);
assertNextEvent('resolve', [ 1 ]);
assertNextEvent('resolve', [ 3 ]);
assertNextEvent('after', [ 3 ]);
assertEmptyLog();
});
basicTest();
exceptions();

@@ -292,3 +320,9 @@ if (has_promise_hooks) {
});

}

if (has_promise_hooks) {
doTest();
d8.debugger.enable();
doTest();
}

0 comments on commit c5630ad

Please sign in to comment.