Commit fb3ea4c
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
2 files changed
Lines changed: 8 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -30,6 +30,7 @@ using v8::HandleScope; | |||
| 30 | 30 | using v8::Isolate; | |
| 31 | 31 | using v8::Local; | |
| 32 | 32 | using v8::Object; | |
| 33 | + using v8::Persistent; | ||
| 33 | 34 | using v8::Value; | |
| 34 | 35 | ||
| 35 | 36 | using v8_inspector::StringBuffer; | |
@@ -613,8 +614,7 @@ void Agent::RegisterAsyncHook(Isolate* isolate, | |||
| 613 | 614 | ||
| 614 | 615 | void Agent::EnableAsyncHook() { | |
| 615 | 616 | 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_); | ||
| 618 | 618 | } else if (pending_disable_async_hook_) { | |
| 619 | 619 | CHECK(!pending_enable_async_hook_); | |
| 620 | 620 | pending_disable_async_hook_ = false; | |
@@ -625,8 +625,7 @@ void Agent::EnableAsyncHook() { | |||
| 625 | 625 | ||
| 626 | 626 | void Agent::DisableAsyncHook() { | |
| 627 | 627 | 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_); | ||
| 630 | 629 | } else if (pending_enable_async_hook_) { | |
| 631 | 630 | CHECK(!pending_disable_async_hook_); | |
| 632 | 631 | pending_enable_async_hook_ = false; | |
@@ -635,10 +634,11 @@ void Agent::DisableAsyncHook() { | |||
| 635 | 634 | } | |
| 636 | 635 | } | |
| 637 | 636 | ||
| 638 | - void Agent::ToggleAsyncHook(Isolate* isolate, Local<Function> fn) { | ||
| 637 | + void Agent::ToggleAsyncHook(Isolate* isolate, const Persistent<Function>& fn) { | ||
| 639 | 638 | HandleScope handle_scope(isolate); | |
| 639 | + CHECK(!fn.IsEmpty()); | ||
| 640 | 640 | 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); | ||
| 642 | 642 | if (result.IsEmpty()) { | |
| 643 | 643 | FatalError( | |
| 644 | 644 | "node::inspector::Agent::ToggleAsyncHook", | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -96,7 +96,8 @@ class Agent { | |||
| 96 | 96 | void DisableAsyncHook(); | |
| 97 | 97 | ||
| 98 | 98 | 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); | ||
| 100 | 101 | ||
| 101 | 102 | node::Environment* parent_env_; | |
| 102 | 103 | std::unique_ptr<NodeInspectorClient> client_; | |
0 commit comments