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 <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>

Author: addaleax

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;
 
 classStartIoTask : publicTask{
 public:
@@ -97,6 +99,8 @@ static void StartIoThreadWakeup(int signo, siginfo_t* info, void* ucontext){
 inlinevoid* 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(){}
 
 boolAgent::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){