From b4667794d40ff814ce12a1c06e371a42200b7748 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Thu, 7 Dec 2017 23:55:23 +0100
Subject: [PATCH] src: fix missing handlescope bug

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
---
 src/inspector_agent.cc | 12 ++++++------
 src/inspector_agent.h  |  3 ++-
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
index 48cc29324fbdbb..9677dca37b5939 100644
--- a/src/inspector_agent.cc
+++ b/src/inspector_agent.cc
@@ -30,6 +30,7 @@ using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
 using v8::Object;
+using v8::Persistent;
 using v8::Value;
 
 using v8_inspector::StringBuffer;
@@ -613,8 +614,7 @@ void Agent::RegisterAsyncHook(Isolate* isolate,
 
 void Agent::EnableAsyncHook() {
   if (!enable_async_hook_function_.IsEmpty()) {
-    Isolate* isolate = parent_env_->isolate();
-    ToggleAsyncHook(isolate, enable_async_hook_function_.Get(isolate));
+    ToggleAsyncHook(parent_env_->isolate(), enable_async_hook_function_);
   } else if (pending_disable_async_hook_) {
     CHECK(!pending_enable_async_hook_);
     pending_disable_async_hook_ = false;
@@ -625,8 +625,7 @@ void Agent::EnableAsyncHook() {
 
 void Agent::DisableAsyncHook() {
   if (!disable_async_hook_function_.IsEmpty()) {
-    Isolate* isolate = parent_env_->isolate();
-    ToggleAsyncHook(isolate, disable_async_hook_function_.Get(isolate));
+    ToggleAsyncHook(parent_env_->isolate(), disable_async_hook_function_);
   } else if (pending_enable_async_hook_) {
     CHECK(!pending_disable_async_hook_);
     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);
+  CHECK(!fn.IsEmpty());
   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()) {
     FatalError(
         "node::inspector::Agent::ToggleAsyncHook",
diff --git a/src/inspector_agent.h b/src/inspector_agent.h
index 7211f5a2a49a95..29b9546b514aea 100644
--- a/src/inspector_agent.h
+++ b/src/inspector_agent.h
@@ -96,7 +96,8 @@ class Agent {
   void DisableAsyncHook();
 
  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_;
   std::unique_ptr<NodeInspectorClient> client_;