From 7a82e5ee62e5a3a29059581f208d960ff3ec8696 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 19 Oct 2019 00:53:38 +0200
Subject: [PATCH] inspector: turn platform tasks that outlive Agent into no-ops

Turn tasks scheduled on the `v8::Isolate` or on the given platform
into no-ops if the underlying `MainThreadInterface` has gone away
before the task could be run (which would happen when the
`Environment` instance and with it the `inspector::Agent` instance
are destroyed).

This addresses an issue that Electron has been having with
inspector support, and generally just seems like the right thing
to do, as we may not fully be in control of the relative timing
of Environment teardown, platform tasksexecution, and the
execution of `RequestInterrupt()` callbacks (although
the former two always happen in the same order in our own code).

PR-URL: https://github.com/nodejs/node/pull/30031
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
---
 src/inspector/main_thread_interface.cc | 21 +++++++++++++--------
 src/inspector/main_thread_interface.h  |  3 ++-
 src/inspector_agent.cc                 |  8 ++++----
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/src/inspector/main_thread_interface.cc b/src/inspector/main_thread_interface.cc
index ac4461baed4afd..48a964d564fec2 100644
--- a/src/inspector/main_thread_interface.cc
+++ b/src/inspector/main_thread_interface.cc
@@ -87,15 +87,15 @@ class CallRequest : public Request {
 
 class DispatchMessagesTask : public v8::Task {
  public:
-  explicit DispatchMessagesTask(MainThreadInterface* thread)
+  explicit DispatchMessagesTask(std::weak_ptr<MainThreadInterface> thread)
                                 : thread_(thread) {}
 
   void Run() override {
-    thread_->DispatchMessages();
+    if (auto thread = thread_.lock()) thread->DispatchMessages();
   }
 
  private:
-  MainThreadInterface* thread_;
+  std::weak_ptr<MainThreadInterface> thread_;
 };
 
 template <typename T>
@@ -231,11 +231,16 @@ void MainThreadInterface::Post(std::unique_ptr<Request> request) {
   if (needs_notify) {
     if (isolate_ != nullptr && platform_ != nullptr) {
       std::shared_ptr<v8::TaskRunner> taskrunner =
-        platform_->GetForegroundTaskRunner(isolate_);
-      taskrunner->PostTask(std::make_unique<DispatchMessagesTask>(this));
-      isolate_->RequestInterrupt([](v8::Isolate* isolate, void* thread) {
-        static_cast<MainThreadInterface*>(thread)->DispatchMessages();
-      }, this);
+          platform_->GetForegroundTaskRunner(isolate_);
+      std::weak_ptr<MainThreadInterface>* interface_ptr =
+          new std::weak_ptr<MainThreadInterface>(shared_from_this());
+      taskrunner->PostTask(
+          std::make_unique<DispatchMessagesTask>(*interface_ptr));
+      isolate_->RequestInterrupt([](v8::Isolate* isolate, void* opaque) {
+        std::unique_ptr<std::weak_ptr<MainThreadInterface>> interface_ptr {
+          static_cast<std::weak_ptr<MainThreadInterface>*>(opaque) };
+        if (auto iface = interface_ptr->lock()) iface->DispatchMessages();
+      }, static_cast<void*>(interface_ptr));
     }
   }
   incoming_message_cond_.Broadcast(scoped_lock);
diff --git a/src/inspector/main_thread_interface.h b/src/inspector/main_thread_interface.h
index 9a48192cd374e5..bcea19f3f3937e 100644
--- a/src/inspector/main_thread_interface.h
+++ b/src/inspector/main_thread_interface.h
@@ -70,7 +70,8 @@ class MainThreadHandle : public std::enable_shared_from_this<MainThreadHandle> {
   friend class MainThreadInterface;
 };
 
-class MainThreadInterface {
+class MainThreadInterface :
+    public std::enable_shared_from_this<MainThreadInterface> {
  public:
   MainThreadInterface(Agent* agent, uv_loop_t*, v8::Isolate* isolate,
                       v8::Platform* platform);
diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc
index 424b09d6e1d22a..469e0b4f8f335a 100644
--- a/src/inspector_agent.cc
+++ b/src/inspector_agent.cc
@@ -665,10 +665,10 @@ class NodeInspectorClient : public V8InspectorClient {
   }
 
   std::shared_ptr<MainThreadHandle> getThreadHandle() {
-    if (interface_ == nullptr) {
-      interface_.reset(new MainThreadInterface(
+    if (!interface_) {
+      interface_ = std::make_shared<MainThreadInterface>(
           env_->inspector_agent(), env_->event_loop(), env_->isolate(),
-          env_->isolate_data()->platform()));
+          env_->isolate_data()->platform());
     }
     return interface_->GetHandle();
   }
@@ -739,7 +739,7 @@ class NodeInspectorClient : public V8InspectorClient {
   bool waiting_for_frontend_ = false;
   bool waiting_for_sessions_disconnect_ = false;
   // Allows accessing Inspector from non-main threads
-  std::unique_ptr<MainThreadInterface> interface_;
+  std::shared_ptr<MainThreadInterface> interface_;
   std::shared_ptr<WorkerManager> worker_manager_;
 };