diff --git a/shell/common/api/electron_bindings.cc b/shell/common/api/electron_bindings.cc index d078f37700512..afecb9f5d3699 100644 --- a/shell/common/api/electron_bindings.cc +++ b/shell/common/api/electron_bindings.cc @@ -56,14 +56,12 @@ void V8FatalErrorCallback(const char* location, const char* message) { } // namespace ElectronBindings::ElectronBindings(uv_loop_t* loop) { - uv_async_init(loop, &call_next_tick_async_, OnCallNextTick); - call_next_tick_async_.data = this; + uv_async_init(loop, call_next_tick_async_.get(), OnCallNextTick); + call_next_tick_async_.get()->data = this; metrics_ = base::ProcessMetrics::CreateCurrentProcessMetrics(); } -ElectronBindings::~ElectronBindings() { - uv_close(reinterpret_cast(&call_next_tick_async_), nullptr); -} +ElectronBindings::~ElectronBindings() {} // static void ElectronBindings::BindProcess(v8::Isolate* isolate, @@ -131,7 +129,7 @@ void ElectronBindings::ActivateUVLoop(v8::Isolate* isolate) { return; pending_next_ticks_.push_back(env); - uv_async_send(&call_next_tick_async_); + uv_async_send(call_next_tick_async_.get()); } // static diff --git a/shell/common/api/electron_bindings.h b/shell/common/api/electron_bindings.h index d489d3feb6390..79a68fcd5a2b2 100644 --- a/shell/common/api/electron_bindings.h +++ b/shell/common/api/electron_bindings.h @@ -14,6 +14,7 @@ #include "base/process/process_metrics.h" #include "base/strings/string16.h" #include "shell/common/gin_helper/promise.h" +#include "shell/common/node_bindings.h" #include "uv.h" // NOLINT(build/include_directory) namespace gin_helper { @@ -74,7 +75,7 @@ class ElectronBindings { bool success, std::unique_ptr dump); - uv_async_t call_next_tick_async_; + UvHandle call_next_tick_async_; std::list pending_next_ticks_; std::unique_ptr metrics_; diff --git a/shell/common/node_bindings.cc b/shell/common/node_bindings.cc index e1e72dc4f5012..08dd113a5b644 100644 --- a/shell/common/node_bindings.cc +++ b/shell/common/node_bindings.cc @@ -97,24 +97,26 @@ namespace { void stop_and_close_uv_loop(uv_loop_t* loop) { uv_stop(loop); - int error = uv_loop_close(loop); - - while (error) { - uv_run(loop, UV_RUN_DEFAULT); - uv_stop(loop); - uv_walk( - loop, - [](uv_handle_t* handle, void*) { - if (!uv_is_closing(handle)) { - uv_close(handle, nullptr); - } - }, - nullptr); - uv_run(loop, UV_RUN_DEFAULT); - error = uv_loop_close(loop); - } - DCHECK_EQ(error, 0); + auto const ensure_closing = [](uv_handle_t* handle, void*) { + // We should be using the UvHandle wrapper everywhere, in which case + // all handles should already be in a closing state... + DCHECK(uv_is_closing(handle)); + // ...but if a raw handle got through, through, do the right thing anyway + if (!uv_is_closing(handle)) { + uv_close(handle, nullptr); + } + }; + + uv_walk(loop, ensure_closing, nullptr); + + // All remaining handles are in a closing state now. + // Pump the event loop so that they can finish closing. + for (;;) + if (uv_run(loop, UV_RUN_DEFAULT) == 0) + break; + + DCHECK_EQ(0, uv_loop_alive(loop)); } bool g_is_initialized = false; @@ -282,7 +284,7 @@ NodeBindings::~NodeBindings() { // Clear uv. uv_sem_destroy(&embed_sem_); - uv_close(reinterpret_cast(&dummy_uv_handle_), nullptr); + dummy_uv_handle_.reset(); // Clean up worker loop if (in_worker_loop()) @@ -460,7 +462,7 @@ void NodeBindings::LoadEnvironment(node::Environment* env) { void NodeBindings::PrepareMessageLoop() { // Add dummy handle for libuv, otherwise libuv would quit when there is // nothing to do. - uv_async_init(uv_loop_, &dummy_uv_handle_, nullptr); + uv_async_init(uv_loop_, dummy_uv_handle_.get(), nullptr); // Start worker that will interrupt main loop when having uv events. uv_sem_init(&embed_sem_, 0); @@ -517,8 +519,7 @@ void NodeBindings::WakeupMainThread() { } void NodeBindings::WakeupEmbedThread() { - if (!in_worker_loop()) - uv_async_send(&dummy_uv_handle_); + uv_async_send(dummy_uv_handle_.get()); } // static diff --git a/shell/common/node_bindings.h b/shell/common/node_bindings.h index bf737a51ddffb..c33c32eaeb281 100644 --- a/shell/common/node_bindings.h +++ b/shell/common/node_bindings.h @@ -5,6 +5,8 @@ #ifndef SHELL_COMMON_NODE_BINDINGS_H_ #define SHELL_COMMON_NODE_BINDINGS_H_ +#include + #include "base/files/file_path.h" #include "base/macros.h" #include "base/memory/weak_ptr.h" @@ -24,6 +26,54 @@ class IsolateData; namespace electron { +// A helper class to manage uv_handle_t types, e.g. uv_async_t. +// +// As per the uv docs: "uv_close() MUST be called on each handle before +// memory is released. Moreover, the memory can only be released in +// close_cb or after it has returned." This class encapsulates the work +// needed to follow those requirements. +template ::value || + std::is_same::value || + std::is_same::value || + std::is_same::value || + std::is_same::value || + std::is_same::value || + std::is_same::value || + std::is_same::value || + std::is_same::value || + std::is_same::value || + std::is_same::value || + std::is_same::value || + std::is_same::value || + std::is_same::value || + std::is_same::value>::type* = nullptr> +class UvHandle { + public: + UvHandle() : t_(new T) {} + ~UvHandle() { reset(); } + T* get() { return t_; } + uv_handle_t* handle() { return reinterpret_cast(t_); } + + void reset() { + auto* h = handle(); + if (h != nullptr) { + DCHECK_EQ(0, uv_is_closing(h)); + uv_close(h, OnClosed); + t_ = nullptr; + } + } + + private: + static void OnClosed(uv_handle_t* handle) { + delete reinterpret_cast(handle); + } + + T* t_ = {}; +}; + class NodeBindings { public: enum class BrowserEnvironment { BROWSER, RENDERER, WORKER }; @@ -95,7 +145,7 @@ class NodeBindings { uv_loop_t worker_loop_; // Dummy handle to make uv's loop not quit. - uv_async_t dummy_uv_handle_; + UvHandle dummy_uv_handle_; // Thread for polling events. uv_thread_t embed_thread_;