From edf8070ea6f1561e518d07901ba5e8c4ef14e88c Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 19 Oct 2019 00:53:38 +0200 Subject: [PATCH 1/3] 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). --- src/inspector/main_thread_interface.cc | 25 +++++++++++++++++-------- src/inspector/main_thread_interface.h | 3 ++- src/inspector_agent.cc | 8 ++++---- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/inspector/main_thread_interface.cc b/src/inspector/main_thread_interface.cc index ac4461baed4afd..de9f273e7a3151 100644 --- a/src/inspector/main_thread_interface.cc +++ b/src/inspector/main_thread_interface.cc @@ -87,15 +87,17 @@ class CallRequest : public Request { class DispatchMessagesTask : public v8::Task { public: - explicit DispatchMessagesTask(MainThreadInterface* thread) + explicit DispatchMessagesTask(std::weak_ptr thread) : thread_(thread) {} void Run() override { - thread_->DispatchMessages(); + std::shared_ptr thread = thread_.lock(); + if (thread) + thread->DispatchMessages(); } private: - MainThreadInterface* thread_; + std::weak_ptr thread_; }; template @@ -231,11 +233,18 @@ void MainThreadInterface::Post(std::unique_ptr request) { if (needs_notify) { if (isolate_ != nullptr && platform_ != nullptr) { std::shared_ptr taskrunner = - platform_->GetForegroundTaskRunner(isolate_); - taskrunner->PostTask(std::make_unique(this)); - isolate_->RequestInterrupt([](v8::Isolate* isolate, void* thread) { - static_cast(thread)->DispatchMessages(); - }, this); + platform_->GetForegroundTaskRunner(isolate_); + std::weak_ptr* interface_ptr = + new std::weak_ptr(shared_from_this()); + taskrunner->PostTask( + std::make_unique(*interface_ptr)); + isolate_->RequestInterrupt([](v8::Isolate* isolate, void* opaque) { + std::unique_ptr> interface_ptr { + static_cast*>(opaque) }; + std::shared_ptr interface = interface_ptr->lock(); + if (interface) + interface->DispatchMessages(); + }, static_cast(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 { friend class MainThreadInterface; }; -class MainThreadInterface { +class MainThreadInterface : + public std::enable_shared_from_this { 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 getThreadHandle() { - if (interface_ == nullptr) { - interface_.reset(new MainThreadInterface( + if (!interface_) { + interface_ = std::make_shared( 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 interface_; + std::shared_ptr interface_; std::shared_ptr worker_manager_; }; From 521ce6a9ae82e61c84fac48a1090fd2b8de620e3 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 21 Oct 2019 14:13:09 +0200 Subject: [PATCH 2/3] fixup! inspector: turn platform tasks that outlive Agent into no-ops --- src/inspector/main_thread_interface.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/inspector/main_thread_interface.cc b/src/inspector/main_thread_interface.cc index de9f273e7a3151..3935dbdfa7c6c4 100644 --- a/src/inspector/main_thread_interface.cc +++ b/src/inspector/main_thread_interface.cc @@ -241,9 +241,9 @@ void MainThreadInterface::Post(std::unique_ptr request) { isolate_->RequestInterrupt([](v8::Isolate* isolate, void* opaque) { std::unique_ptr> interface_ptr { static_cast*>(opaque) }; - std::shared_ptr interface = interface_ptr->lock(); - if (interface) - interface->DispatchMessages(); + std::shared_ptr iface = interface_ptr->lock(); + if (iface) + iface->DispatchMessages(); }, static_cast(interface_ptr)); } } From 54a475bc5a78f2ced305ecc766f9d46221d80757 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 23 Oct 2019 00:04:06 +0200 Subject: [PATCH 3/3] fixup! inspector: turn platform tasks that outlive Agent into no-ops --- src/inspector/main_thread_interface.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/inspector/main_thread_interface.cc b/src/inspector/main_thread_interface.cc index 3935dbdfa7c6c4..48a964d564fec2 100644 --- a/src/inspector/main_thread_interface.cc +++ b/src/inspector/main_thread_interface.cc @@ -91,9 +91,7 @@ class DispatchMessagesTask : public v8::Task { : thread_(thread) {} void Run() override { - std::shared_ptr thread = thread_.lock(); - if (thread) - thread->DispatchMessages(); + if (auto thread = thread_.lock()) thread->DispatchMessages(); } private: @@ -241,9 +239,7 @@ void MainThreadInterface::Post(std::unique_ptr request) { isolate_->RequestInterrupt([](v8::Isolate* isolate, void* opaque) { std::unique_ptr> interface_ptr { static_cast*>(opaque) }; - std::shared_ptr iface = interface_ptr->lock(); - if (iface) - iface->DispatchMessages(); + if (auto iface = interface_ptr->lock()) iface->DispatchMessages(); }, static_cast(interface_ptr)); } }