Browse Source

debugger: fix SIGUSR1 bootstrap race condition

Before this commit, the SIGUSR1 signal handler wasn't installed until
late in the bootstrapping process and we were prone to miss signals
sent by other processes.

This commit installs an early-boot signal handler that merely records
the fact that we received a signal.  Once the debugger infrastructure
is in place, the signal is re-raised, kickstarting the debugger.

Among other things, this means that simple/test-debugger-client is
now _much_ less likely to fail.
v0.11.8-release
Ben Noordhuis 11 years ago
parent
commit
4234bcce48
  1. 24
      src/node.cc
  2. 16
      test/simple/test-debugger-client.js

24
src/node.cc

@ -2874,6 +2874,22 @@ static void DispatchDebugMessagesAsyncCallback(uv_async_t* handle, int status) {
#ifdef __POSIX__
static volatile sig_atomic_t caught_early_debug_signal;
static void EarlyDebugSignalHandler(int signo) {
caught_early_debug_signal = 1;
}
static void InstallEarlyDebugSignalHandler() {
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sa.sa_handler = EarlyDebugSignalHandler;
sigaction(SIGUSR1, &sa, NULL);
}
static void EnableDebugSignalHandler(int signo) {
// Call only async signal-safe functions here!
v8::Debug::DebugBreak(*static_cast<Isolate* volatile*>(&node_isolate));
@ -2911,6 +2927,9 @@ void DebugProcess(const FunctionCallbackInfo<Value>& args) {
static int RegisterDebugSignalHandler() {
// FIXME(bnoordhuis) Should be per-isolate or per-context, not global.
RegisterSignalHandler(SIGUSR1, EnableDebugSignalHandler);
// If we caught a SIGUSR1 during the bootstrap process, re-raise it
// now that the debugger infrastructure is in place.
if (caught_early_debug_signal) raise(SIGUSR1);
return 0;
}
#endif // __POSIX__
@ -3270,6 +3289,11 @@ Environment* CreateEnvironment(Isolate* isolate,
int Start(int argc, char** argv) {
#if !defined(_WIN32)
// Try hard not to lose SIGUSR1 signals during the bootstrap process.
InstallEarlyDebugSignalHandler();
#endif
assert(argc > 0);
// Hack around with the argv pointer. Used for process.title = "blah".

16
test/simple/test-debugger-client.js

@ -181,11 +181,15 @@ function doTest(cb, done) {
console.error('got stderr data %j', data);
nodeProcess.stderr.resume();
b += data;
if (didTryConnect == false &&
b.match(/Debugger listening on port/)) {
if (didTryConnect === false && b.match(/Debugger listening on port/)) {
didTryConnect = true;
setTimeout(function() {
// The timeout is here to expose a race in the bootstrap process.
// Without the early SIGUSR1 debug handler, it effectively results
// in an infinite ECONNREFUSED loop.
setTimeout(tryConnect, 100);
function tryConnect() {
// Wait for some data before trying to connect
var c = new debug.Client();
console.error('>>> connecting...');
@ -202,7 +206,11 @@ function doTest(cb, done) {
done();
});
});
});
c.on('error', function(err) {
if (err.code !== 'ECONNREFUSED') throw err;
setTimeout(tryConnect, 10);
});
}
}
});
}

Loading…
Cancel
Save