Skip to content

Commit

Permalink
inspector: turn platform tasks that outlive Agent into no-ops
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-By: Shelley Vohr <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
  • Loading branch information
addaleax authored and targos committed Oct 24, 2019
1 parent 5ca5864 commit f01c5c5
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 13 deletions.
21 changes: 13 additions & 8 deletions src/inspector/main_thread_interface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/inspector/main_thread_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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_;
};

Expand Down

0 comments on commit f01c5c5

Please sign in to comment.