Skip to content

Commit fb3ea4c

Browse files
bnoordhuisMylesBorins
authored andcommitted
src: fix missing handlescope bug in inspector
Fix a regression that was introduced in commit 5886e20 ("inspector: track async stacks when necessary") and that I overlooked during review: the persistent handle with the callback must be rematerialized *after* the `v8::HandleScope` is created, not before. Apparently `test/sequential/test-inspector-async-call-stack.js` has no test coverage for this scenario and I'm out of good ideas on how to create a concise and reliable test case. Fixes: #17496 PR-URL: #17539 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Khaidi Chu <[email protected]> Reviewed-By: Timothy Gu <[email protected]>
1 parent 4bb27a2 commit fb3ea4c

File tree

2 files changed

+8
-7
lines changed

2 files changed

+8
-7
lines changed

src/inspector_agent.cc

+6-6
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ using v8::HandleScope;
3030
using v8::Isolate;
3131
using v8::Local;
3232
using v8::Object;
33+
using v8::Persistent;
3334
using v8::Value;
3435

3536
using v8_inspector::StringBuffer;
@@ -613,8 +614,7 @@ void Agent::RegisterAsyncHook(Isolate* isolate,
613614

614615
void Agent::EnableAsyncHook() {
615616
if (!enable_async_hook_function_.IsEmpty()) {
616-
Isolate* isolate = parent_env_->isolate();
617-
ToggleAsyncHook(isolate, enable_async_hook_function_.Get(isolate));
617+
ToggleAsyncHook(parent_env_->isolate(), enable_async_hook_function_);
618618
} else if (pending_disable_async_hook_) {
619619
CHECK(!pending_enable_async_hook_);
620620
pending_disable_async_hook_ = false;
@@ -625,8 +625,7 @@ void Agent::EnableAsyncHook() {
625625

626626
void Agent::DisableAsyncHook() {
627627
if (!disable_async_hook_function_.IsEmpty()) {
628-
Isolate* isolate = parent_env_->isolate();
629-
ToggleAsyncHook(isolate, disable_async_hook_function_.Get(isolate));
628+
ToggleAsyncHook(parent_env_->isolate(), disable_async_hook_function_);
630629
} else if (pending_enable_async_hook_) {
631630
CHECK(!pending_disable_async_hook_);
632631
pending_enable_async_hook_ = false;
@@ -635,10 +634,11 @@ void Agent::DisableAsyncHook() {
635634
}
636635
}
637636

638-
void Agent::ToggleAsyncHook(Isolate* isolate, Local<Function> fn) {
637+
void Agent::ToggleAsyncHook(Isolate* isolate, const Persistent<Function>& fn) {
639638
HandleScope handle_scope(isolate);
639+
CHECK(!fn.IsEmpty());
640640
auto context = parent_env_->context();
641-
auto result = fn->Call(context, Undefined(isolate), 0, nullptr);
641+
auto result = fn.Get(isolate)->Call(context, Undefined(isolate), 0, nullptr);
642642
if (result.IsEmpty()) {
643643
FatalError(
644644
"node::inspector::Agent::ToggleAsyncHook",

src/inspector_agent.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ class Agent {
9696
void DisableAsyncHook();
9797

9898
private:
99-
void ToggleAsyncHook(v8::Isolate* isolate, v8::Local<v8::Function> fn);
99+
void ToggleAsyncHook(v8::Isolate* isolate,
100+
const v8::Persistent<v8::Function>& fn);
100101

101102
node::Environment* parent_env_;
102103
std::unique_ptr<NodeInspectorClient> client_;

0 commit comments

Comments
 (0)