Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit fb3ea4c

Browse filesBrowse 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 <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Timothy Gu <timothygu99@gmail.com>
1 parent 4bb27a2 commit fb3ea4c
Copy full SHA for fb3ea4c

File tree

Expand file treeCollapse file tree

2 files changed

+8
-7
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

2 files changed

+8
-7
lines changed
Open diff view settings
Collapse file

‎src/inspector_agent.cc‎

Copy file name to clipboardExpand all lines: src/inspector_agent.cc
+6-6Lines changed: 6 additions & 6 deletions
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",
Collapse file

‎src/inspector_agent.h‎

Copy file name to clipboardExpand all lines: src/inspector_agent.h
+2-1Lines changed: 2 additions & 1 deletion
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.