Skip to content

Commit c5630ad

Browse files
targosRafaelGSS
targos
authored and
RafaelGSS
committed
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>
1 parent 4c33e5d commit c5630ad

File tree

9 files changed

+87
-6
lines changed

9 files changed

+87
-6
lines changed

‎common.gypi‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

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

4141
##### V8 defaults for Node.js #####
4242

‎deps/v8/src/builtins/builtins-microtask-queue-gen.cc‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -478,30 +478,38 @@ void MicrotaskQueueBuiltinsAssembler::RewindEnteredContext(
478478
void MicrotaskQueueBuiltinsAssembler::RunAllPromiseHooks(
479479
PromiseHookType type, TNode<Context> context,
480480
TNode<HeapObject> promise_or_capability) {
481-
Label hook(this, Label::kDeferred), done_hook(this);
482481
TNode<Uint32T> promiseHookFlags = PromiseHookFlags();
482+
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
483+
Label hook(this, Label::kDeferred), done_hook(this);
483484
Branch(NeedsAnyPromiseHooks(promiseHookFlags), &hook, &done_hook);
484485
BIND(&hook);
485486
{
487+
#endif
486488
switch (type) {
487489
case PromiseHookType::kBefore:
490+
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
488491
RunContextPromiseHookBefore(context, promise_or_capability,
489492
promiseHookFlags);
493+
#endif
490494
RunPromiseHook(Runtime::kPromiseHookBefore, context,
491495
promise_or_capability, promiseHookFlags);
492496
break;
493497
case PromiseHookType::kAfter:
498+
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
494499
RunContextPromiseHookAfter(context, promise_or_capability,
495500
promiseHookFlags);
501+
#endif
496502
RunPromiseHook(Runtime::kPromiseHookAfter, context,
497503
promise_or_capability, promiseHookFlags);
498504
break;
499505
default:
500506
UNREACHABLE();
501507
}
508+
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
502509
Goto(&done_hook);
503510
}
504511
BIND(&done_hook);
512+
#endif
505513
}
506514

507515
void MicrotaskQueueBuiltinsAssembler::RunPromiseHook(

‎deps/v8/src/d8/d8.cc‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2218,6 +2218,16 @@ void Shell::AsyncHooksTriggerAsyncId(
22182218
PerIsolateData::Get(isolate)->GetAsyncHooks()->GetTriggerAsyncId()));
22192219
}
22202220

2221+
static v8::debug::DebugDelegate dummy_delegate;
2222+
2223+
void Shell::EnableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args) {
2224+
v8::debug::SetDebugDelegate(args.GetIsolate(), &dummy_delegate);
2225+
}
2226+
2227+
void Shell::DisableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args) {
2228+
v8::debug::SetDebugDelegate(args.GetIsolate(), nullptr);
2229+
}
2230+
22212231
void Shell::SetPromiseHooks(const v8::FunctionCallbackInfo<v8::Value>& args) {
22222232
Isolate* isolate = args.GetIsolate();
22232233
if (i::FLAG_correctness_fuzzer_suppressions) {
@@ -3162,6 +3172,18 @@ Local<ObjectTemplate> Shell::CreateD8Template(Isolate* isolate) {
31623172
Local<Signature>(), 4));
31633173
d8_template->Set(isolate, "promise", promise_template);
31643174
}
3175+
{
3176+
Local<ObjectTemplate> debugger_template = ObjectTemplate::New(isolate);
3177+
debugger_template->Set(
3178+
isolate, "enable",
3179+
FunctionTemplate::New(isolate, EnableDebugger, Local<Value>(),
3180+
Local<Signature>(), 0));
3181+
debugger_template->Set(
3182+
isolate, "disable",
3183+
FunctionTemplate::New(isolate, DisableDebugger, Local<Value>(),
3184+
Local<Signature>(), 0));
3185+
d8_template->Set(isolate, "debugger", debugger_template);
3186+
}
31653187
return d8_template;
31663188
}
31673189

‎deps/v8/src/d8/d8.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,9 @@ class Shell : public i::AllStatic {
569569

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

572+
static void EnableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args);
573+
static void DisableDebugger(const v8::FunctionCallbackInfo<v8::Value>& args);
574+
572575
static void Print(const v8::FunctionCallbackInfo<v8::Value>& args);
573576
static void PrintErr(const v8::FunctionCallbackInfo<v8::Value>& args);
574577
static void WriteStdout(const v8::FunctionCallbackInfo<v8::Value>& args);

‎deps/v8/src/execution/isolate.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4958,9 +4958,11 @@ void Isolate::SetPromiseHook(PromiseHook hook) {
49584958
void Isolate::RunAllPromiseHooks(PromiseHookType type,
49594959
Handle<JSPromise> promise,
49604960
Handle<Object> parent) {
4961+
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
49614962
if (HasContextPromiseHooks()) {
49624963
native_context()->RunPromiseHook(type, promise, parent);
49634964
}
4965+
#endif
49644966
if (HasIsolatePromiseHooks() || HasAsyncEventDelegate()) {
49654967
RunPromiseHook(type, promise, parent);
49664968
}
@@ -4977,7 +4979,7 @@ void Isolate::RunPromiseHook(PromiseHookType type, Handle<JSPromise> promise,
49774979
void Isolate::OnAsyncFunctionSuspended(Handle<JSPromise> promise,
49784980
Handle<JSPromise> parent) {
49794981
DCHECK_EQ(0, promise->async_task_id());
4980-
RunPromiseHook(PromiseHookType::kInit, promise, parent);
4982+
RunAllPromiseHooks(PromiseHookType::kInit, promise, parent);
49814983
if (HasAsyncEventDelegate()) {
49824984
DCHECK_NE(nullptr, async_event_delegate_);
49834985
promise->set_async_task_id(++async_task_count_);

‎deps/v8/src/objects/contexts.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,7 @@ STATIC_ASSERT(NativeContext::kSize ==
551551
(Context::SizeFor(NativeContext::NATIVE_CONTEXT_SLOTS) +
552552
kSystemPointerSize));
553553

554+
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
554555
void NativeContext::RunPromiseHook(PromiseHookType type,
555556
Handle<JSPromise> promise,
556557
Handle<Object> parent) {
@@ -606,6 +607,7 @@ void NativeContext::RunPromiseHook(PromiseHookType type,
606607
isolate->clear_pending_exception();
607608
}
608609
}
610+
#endif
609611

610612
} // namespace internal
611613
} // namespace v8

‎deps/v8/src/objects/contexts.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,8 +781,10 @@ class NativeContext : public Context {
781781
void IncrementErrorsThrown();
782782
int GetErrorsThrown();
783783

784+
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
784785
void RunPromiseHook(PromiseHookType type, Handle<JSPromise> promise,
785786
Handle<Object> parent);
787+
#endif
786788

787789
private:
788790
STATIC_ASSERT(OffsetOfElementAt(EMBEDDER_DATA_INDEX) ==

‎deps/v8/src/objects/objects.cc‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5398,6 +5398,14 @@ Handle<Object> JSPromise::Fulfill(Handle<JSPromise> promise,
53985398
Handle<Object> value) {
53995399
Isolate* const isolate = promise->GetIsolate();
54005400

5401+
#ifdef V8_ENABLE_JAVASCRIPT_PROMISE_HOOKS
5402+
if (isolate->HasContextPromiseHooks()) {
5403+
isolate->raw_native_context().RunPromiseHook(
5404+
PromiseHookType::kResolve, promise,
5405+
isolate->factory()->undefined_value());
5406+
}
5407+
#endif
5408+
54015409
// 1. Assert: The value of promise.[[PromiseState]] is "pending".
54025410
CHECK_EQ(Promise::kPending, promise->status());
54035411

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

5480-
isolate->RunAllPromiseHooks(PromiseHookType::kResolve, promise,
5481-
isolate->factory()->undefined_value());
5488+
isolate->RunPromiseHook(PromiseHookType::kResolve, promise,
5489+
isolate->factory()->undefined_value());
54825490

54835491
// 7. If SameValue(resolution, promise) is true, then
54845492
if (promise.is_identical_to(resolution)) {

‎deps/v8/test/mjsunit/promise-hooks.js‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ function optimizerBailout(test, verify) {
220220
d8.promise.setHooks();
221221
}
222222

223-
if (has_promise_hooks) {
223+
function doTest () {
224224
optimizerBailout(async () => {
225225
await Promise.resolve();
226226
}, () => {
@@ -234,6 +234,19 @@ if (has_promise_hooks) {
234234
assertNextEvent('after', [ 3 ]);
235235
assertEmptyLog();
236236
});
237+
optimizerBailout(async () => {
238+
await Promise.reject();
239+
}, () => {
240+
assertNextEvent('init', [ 1 ]);
241+
assertNextEvent('init', [ 2 ]);
242+
assertNextEvent('resolve', [ 2 ]);
243+
assertNextEvent('init', [ 3, 2 ]);
244+
assertNextEvent('before', [ 3 ]);
245+
assertNextEvent('resolve', [ 1 ]);
246+
assertNextEvent('resolve', [ 3 ]);
247+
assertNextEvent('after', [ 3 ]);
248+
assertEmptyLog();
249+
});
237250
optimizerBailout(async () => {
238251
await { then (cb) { cb() } };
239252
}, () => {
@@ -249,6 +262,21 @@ if (has_promise_hooks) {
249262
assertNextEvent('after', [ 3 ]);
250263
assertEmptyLog();
251264
});
265+
optimizerBailout(async () => {
266+
await { then (_, cb) { cb() } };
267+
}, () => {
268+
assertNextEvent('init', [ 1 ]);
269+
assertNextEvent('init', [ 2, 1 ]);
270+
assertNextEvent('init', [ 3, 2 ]);
271+
assertNextEvent('before', [ 2 ]);
272+
assertNextEvent('resolve', [ 2 ]);
273+
assertNextEvent('after', [ 2 ]);
274+
assertNextEvent('before', [ 3 ]);
275+
assertNextEvent('resolve', [ 1 ]);
276+
assertNextEvent('resolve', [ 3 ]);
277+
assertNextEvent('after', [ 3 ]);
278+
assertEmptyLog();
279+
});
252280
basicTest();
253281
exceptions();
254282

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

294322
}
323+
324+
if (has_promise_hooks) {
325+
doTest();
326+
d8.debugger.enable();
327+
doTest();
328+
}

0 commit comments

Comments
 (0)