Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions binding.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@
"src/segfault-handler.cpp"
],
"defines": [ "DEBUG", "_DEBUG" ],
"cflags": [ "-O0" ],
"cflags": [ "-O0", "-g" ],
"xcode_settings": {
"OTHER_CFLAGS": [ "-O0" ]
"OTHER_CFLAGS": [ "-O0", "-g"],
"OTHER_LFLAGS": [ "-O0", "-g"]
}
}
]
Expand Down
68 changes: 45 additions & 23 deletions src/segfault-handler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,35 +18,37 @@ using namespace node;

#define STDERR_FD 2

static Persistent<Function> callback;
static bool handlersSet = false;

static void segfault_handler(int sig, siginfo_t *si, void *unused) {
void *array[32]; // Array to store backtrace symbols
size_t size; // To store the size of the stack backtrace
char sbuff[128];
int n; // chars written to buffer
int fd;
time_t now;
int pid;

// Construct a filename
time(&now);
pid = getpid();
snprintf(sbuff, sizeof(sbuff), "stacktrace-%d-%d.log", (int)now, pid );

// Open the File
fd = open(sbuff, O_CREAT | O_APPEND | O_WRONLY, S_IRUSR | S_IRGRP | S_IROTH);
// Write the header line
n = snprintf(sbuff, sizeof(sbuff), "PID %d received SIGSEGV for address: 0x%lx\n", pid, (long) si->si_addr);
if(fd > 0) write(fd, sbuff, n);
n = snprintf(sbuff, sizeof(sbuff), "PID %d received SIGSEGV/SIGBUS (%i) for address: 0x%lx\n", pid, si->si_signo, (long)si->si_addr);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer not to remove these codepaths entirely. Perhaps there could be a config value to enable the default writing behavior, which would execute first, followed by the callback.

My concern here is that after a SIGSEGV and more importantly, the interrupt of V8, the execution state is not trustable. For example, the SIGSEGV might have occurred in the middle of some complicated operation that was itself in the middle of a deep stack of JS methods. V8 could have locks that it is holding or some other transient state. Calling back into the V8 interpreter at that point could produce an unpredictable result and result in the original error not being logged properly. On the other hand, presumably, the SIGSEGV is happening inside of a module and that module would have the ability to call a callback, so maybe the danger isn't as much as I fear. Still, at a minimum, I'd want to have a fallback plan with a stupid simple codepath like the one here - no allocations, no external state, no calls into V8, just a dirt-simple dump of the stacktrace.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do get the value of a JS callback - being able to report the segfault to a central logging service would be super cool. I'm just paranoid and want a second copy written on the local filesystem incase everything goes sideways before the log line gets written out. The issue I was debugging when I wrote this code originally only reproduced after a few days of production usage, so I needed to be certain I'd capture the stack the next time it happened, even if it required SSHing into the box that died.

write(STDERR_FD, sbuff, n);

// Write the Backtrace
size = backtrace(array, 32);
if(fd > 0) backtrace_symbols_fd(array, size, fd);
backtrace_symbols_fd(array, size, STDERR_FD);

// Exit violently
close(fd);
exit(-1);
if (!callback.IsEmpty()) {
char **stack = backtrace_symbols(array, size);
Local<Array> argStack = Local<Array>::New(Array::New(size));
for (size_t i = 0; i < size; i++) {
argStack->Set(i, String::New(stack[i]));
}
Local<Value> argv[3] = {argStack, Local<Value>::New(Number::New(si->si_signo)), Local<Value>::New(Number::New((long)si->si_addr))};
callback->Call(Context::GetCurrent()->Global(), 3, argv);
free(stack);
}

// Re-send the signal, this time a default handler will be called
kill(pid, si->si_signo);
}

// create some stack frames to inspect from CauseSegfault
Expand All @@ -62,7 +64,6 @@ void segfault_stack_frame_1()
int *foo = (int*)1;
printf("NodeSegfaultHandlerNative: about to dereference NULL (will cause a SIGSEGV)\n");
*foo = 78; // trigger a SIGSEGV

}

__attribute__ ((noinline))
Expand All @@ -80,13 +81,34 @@ Handle<Value> CauseSegfault(const Arguments& args) {
}

Handle<Value> RegisterHandler(const Arguments& args) {
struct sigaction sa;
memset(&sa, 0, sizeof(struct sigaction));
sigemptyset(&sa.sa_mask);
sa.sa_sigaction = segfault_handler;
sa.sa_flags = SA_SIGINFO;
sigaction(SIGSEGV, &sa, NULL);
return Undefined();
HandleScope scope;

if (args.Length() > 0) {
if (!args[0]->IsFunction()) {
ThrowException(Exception::TypeError(String::New("Invalid callback argument")));
return scope.Close(Undefined());
}

if (!callback.IsEmpty()) {
callback.Dispose();
callback.Clear();
}
callback = Persistent<Function>::New(Handle<Function>::Cast(args[0]));
}

// Set our handler only once
if (!handlersSet) {
struct sigaction sa;
memset(&sa, 0, sizeof(struct sigaction));
sigemptyset(&sa.sa_mask);
sa.sa_sigaction = segfault_handler;
sa.sa_flags = SA_SIGINFO | SA_RESETHAND; // We set SA_RESETHAND so that our handler is called only once
sigaction(SIGSEGV, &sa, NULL);
sigaction(SIGBUS, &sa, NULL);
handlersSet = true;
}

return scope.Close(Undefined());
}

extern "C" {
Expand Down