inspector: fix abort when two Environments own the inspector · nodejs/node@fe61478 · GitHub
Skip to content

Commit fe61478

Browse files
codebytereaduh95
authored andcommitted
inspector: fix abort when two Environments own the inspector
Two Environments with default flags alive at the same time (for example the embedding.md example run on two threads, or two `CommonEnvironmentSetup`s) aborted the process: `Agent::Start()` bound one file-level static `uv_async_t` to the current Environment's loop for every Environment with `kOwnsInspector`, which `kDefaultFlags` implies, and CHECKed that nobody else had. Environments created one after another did not abort, but each ran `StartDebugSignalHandler()` again, which re-initialized the semaphore the watchdog waits on and spawned another detached watchdog thread, leaking one thread per Environment. Give every Agent that asks for the debug signal handler its own async handle, keep those Agents in a mutex-protected list that the watchdog (or the Windows remote thread) walks, and set the watchdog up once per process while still unblocking SIGUSR1 on each Environment's thread. The handle is heap-allocated, closed by the cleanup hook or `~Agent()`, whichever runs first, and freed by its close callback. A SIGUSR1 now reaches every Environment that asked for the handler, and no longer starts the inspector of one that passed `kNoStartDebugSignalHandler`. Refs: #25777 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: #65877 Refs: #44121 Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 7ae0855 commit fe61478

3 files changed

Lines changed: 81 additions & 61 deletions

File tree

‎src/inspector_agent.cc‎

Lines changed: 71 additions & 58 deletions

‎src/inspector_agent.h‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#endif
99

1010
#include "node_options.h"
11+
#include "uv.h"
1112
#include "v8.h"
1213

1314
#include <cstddef>
@@ -117,7 +118,8 @@ class Agent {
117118
// Can only be called from the main thread.
118119
bool StartIoThread();
119120

120-
// Calls StartIoThread() from off the main thread.
121+
// Calls StartIoThread() from off the main thread. Only valid while the
122+
// Environment owns the inspector and has not started cleanup.
121123
void RequestIoThreadStart();
122124

123125
const DebugOptions& options() { return debug_options_; }
@@ -156,6 +158,12 @@ class Agent {
156158
bool async_hook_enabled_ = false;
157159
bool syncing_async_hook_state_ = false;
158160

161+
// Woken by the SIGUSR1 watchdog; closed by the cleanup hook or ~Agent(),
162+
// whichever runs first, and freed by its close callback.
163+
uv_async_t* start_io_thread_async_ = nullptr;
164+
void StopAcceptingIoThreadStarts();
165+
static void StopAcceptingIoThreadStartsHook(void* agent);
166+
159167
bool network_tracking_enabled_ = false;
160168
bool pending_enable_network_tracking = false;
161169
bool pending_disable_network_tracking = false;

‎test/cctest/test_environment.cc‎

Lines changed: 1 addition & 2 deletions

0 commit comments

Comments
 (0)