1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src: fix missing handlescope bug in inspector

Browse Source 
Fix a regression that was introduced in commit 5886e204f0 ("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: https://github.com/nodejs/node/issues/17496
PR-URL: https://github.com/nodejs/node/pull/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>
This commit is contained in:
Ben Noordhuis 2017-12-07 23:55:23 +01:00
parent d865395b7d
commit df79b7d821
2 changed files with 8 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/inspector_agent.cc
View File

@ -30,6 +30,7 @@ using v8::HandleScope;
using v8::Isolate; using v8::Isolate;
using v8::Local; using v8::Local;
using v8::Object; using v8::Object;
using v8::Persistent;
using v8::Value; using v8::Value;
using v8_inspector::StringBuffer; using v8_inspector::StringBuffer;
@ -613,8 +614,7 @@ void Agent::RegisterAsyncHook(Isolate* isolate,
void Agent::EnableAsyncHook() { void Agent::EnableAsyncHook() {
if (!enable_async_hook_function_.IsEmpty()) { if (!enable_async_hook_function_.IsEmpty()) {
Isolate* isolate = parent_env_->isolate(); ToggleAsyncHook(parent_env_->isolate(), enable_async_hook_function_);
ToggleAsyncHook(isolate, enable_async_hook_function_.Get(isolate));
} else if (pending_disable_async_hook_) { } else if (pending_disable_async_hook_) {
CHECK(!pending_enable_async_hook_); CHECK(!pending_enable_async_hook_);
pending_disable_async_hook_ = false; pending_disable_async_hook_ = false;
@ -625,8 +625,7 @@ void Agent::EnableAsyncHook() {
void Agent::DisableAsyncHook() { void Agent::DisableAsyncHook() {
if (!disable_async_hook_function_.IsEmpty()) { if (!disable_async_hook_function_.IsEmpty()) {
Isolate* isolate = parent_env_->isolate(); ToggleAsyncHook(parent_env_->isolate(), disable_async_hook_function_);
ToggleAsyncHook(isolate, disable_async_hook_function_.Get(isolate));
} else if (pending_enable_async_hook_) { } else if (pending_enable_async_hook_) {
CHECK(!pending_disable_async_hook_); CHECK(!pending_disable_async_hook_);
pending_enable_async_hook_ = false; pending_enable_async_hook_ = false;
@ -635,10 +634,11 @@ void Agent::DisableAsyncHook() {
} }
} }
void Agent::ToggleAsyncHook(Isolate* isolate, Local<Function> fn) { void Agent::ToggleAsyncHook(Isolate* isolate, const Persistent<Function>& fn) {
HandleScope handle_scope(isolate); HandleScope handle_scope(isolate);
CHECK(!fn.IsEmpty());
auto context = parent_env_->context(); auto context = parent_env_->context();
auto result = fn->Call(context, Undefined(isolate), 0, nullptr); auto result = fn.Get(isolate)->Call(context, Undefined(isolate), 0, nullptr);
if (result.IsEmpty()) { if (result.IsEmpty()) {
FatalError( FatalError(
"node::inspector::Agent::ToggleAsyncHook", "node::inspector::Agent::ToggleAsyncHook",

3
src/inspector_agent.h
View File

@ -96,7 +96,8 @@ class Agent {
void DisableAsyncHook(); void DisableAsyncHook();
private: private:
void ToggleAsyncHook(v8::Isolate* isolate, v8::Local<v8::Function> fn); void ToggleAsyncHook(v8::Isolate* isolate,
const v8::Persistent<v8::Function>& fn);
node::Environment* parent_env_; node::Environment* parent_env_;
std::unique_ptr<NodeInspectorClient> client_; std::unique_ptr<NodeInspectorClient> client_;