Skip to content
Permalink
Browse files

inspector: properly shut down uv_async_t

Closing in the Agent destructor is too late, because that happens
when the Environment is destroyed, not when libuv handles are closed.

This fixes a situation in which the same libuv loop is re-used for
multiple Environment instances sequentially, e.g. in our cctest.

PR-URL: #30612
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
addaleax authored and MylesBorins committed Nov 19, 2019
1 parent 3bb085d commit 55e94cbba1ede5cc63eb30aed958e2f968c9d0e7
Showing with 21 additions and 8 deletions.
  1. +21 −8 src/inspector_agent.cc
@@ -60,6 +60,8 @@ static uv_async_t start_io_thread_async;
// This is just an additional check to make sure start_io_thread_async
// is not accidentally re-used or used when uninitialized.
static std::atomic_bool start_io_thread_async_initialized { false };
// Protects the Agent* stored in start_io_thread_async.data.
static Mutex start_io_thread_async_mutex;

class StartIoTask : public Task {
public:
@@ -97,6 +99,8 @@ static void StartIoThreadWakeup(int signo) {
inline void* StartIoThreadMain(void* unused) {
for (;;) {
uv_sem_wait(&start_io_thread_semaphore);
Mutex::ScopedLock lock(start_io_thread_async_mutex);

CHECK(start_io_thread_async_initialized);
Agent* agent = static_cast<Agent*>(start_io_thread_async.data);
if (agent != nullptr)
@@ -157,6 +161,7 @@ static int StartDebugSignalHandler() {

#ifdef _WIN32
DWORD WINAPI StartIoThreadProc(void* arg) {
Mutex::ScopedLock lock(start_io_thread_async_mutex);
CHECK(start_io_thread_async_initialized);
Agent* agent = static_cast<Agent*>(start_io_thread_async.data);
if (agent != nullptr)
@@ -748,14 +753,7 @@ Agent::Agent(Environment* env)
debug_options_(env->options()->debug_options()),
host_port_(env->inspector_host_port()) {}

Agent::~Agent() {
if (start_io_thread_async.data == this) {
CHECK(start_io_thread_async_initialized.exchange(false));
start_io_thread_async.data = nullptr;
// This is global, will never get freed
uv_close(reinterpret_cast<uv_handle_t*>(&start_io_thread_async), nullptr);
}
}
Agent::~Agent() {}

bool Agent::Start(const std::string& path,
const DebugOptions& options,
@@ -768,6 +766,7 @@ bool Agent::Start(const std::string& path,

client_ = std::make_shared<NodeInspectorClient>(parent_env_, is_main);
if (parent_env_->owns_inspector()) {
Mutex::ScopedLock lock(start_io_thread_async_mutex);
CHECK_EQ(start_io_thread_async_initialized.exchange(true), false);
CHECK_EQ(0, uv_async_init(parent_env_->event_loop(),
&start_io_thread_async,
@@ -776,6 +775,20 @@ bool Agent::Start(const std::string& path,
start_io_thread_async.data = this;
// Ignore failure, SIGUSR1 won't work, but that should not block node start.
StartDebugSignalHandler();

parent_env_->AddCleanupHook([](void* data) {
Environment* env = static_cast<Environment*>(data);

{
Mutex::ScopedLock lock(start_io_thread_async_mutex);
start_io_thread_async.data = nullptr;
}

// This is global, will never get freed
env->CloseHandle(&start_io_thread_async, [](uv_async_t*) {
CHECK(start_io_thread_async_initialized.exchange(false));
});
}, parent_env_);
}

AtExit(parent_env_, [](void* env) {

0 comments on commit 55e94cb

Please sign in to comment.
You can’t perform that action at this time.