inspector: turn platform tasks that outlive Agent into no-ops

addaleax · targos · commit 4277066afd4d · 2019-11-11T10:24:51.000+01:00
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: #30031 Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com> Reviewed-By: Shelley Vohr <codebytere@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
diff --git 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
@@ -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
@@ -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_;
 };
 
