-
Notifications
You must be signed in to change notification settings - Fork 7.3k
Segfault on FreeBSD #9326
Comments
|
Have a same problem on FreeBSD 10.1-RELEASE-p5 with nodejs 0.12.0.
On Ubuntu 14.04 (3.13.0-45-generic) the same code works fine. |
I haven't had time to reproduce/investigate, but adding it to 0.12.1 to make sure we take care of it asap. |
@tjfontaine Assigning to you as discussed during the core team meeting. Please assign to @trevnorris when he can use a FreeBSD 10.1 VM somewhere. Thank you! |
Was finally able to reproduce. Though I'm not sure this is a Node thing. Might be a libuv problem? |
Here's a simplified test: require('http').createServer().listen(0, function() {
process.kill(process.pid, 'SIGINT');
}); Still narrowing down why this is happening. |
I've confirmed this is also an issue in io.js. Still investigating why this could possibly happen. |
More stack trace info using latest libuv:
|
I'm going to need more help to continue with this issue. /cc @joyent/node-coreteam |
Oh yeah, I seen it once. Looking... |
The problem disappears if I'll remove SA_RESETHAND... Looking further |
Looks like |
Ha, I think it is a bug in #include <assert.h>
#include <pthread.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
static void handler(int sig) {
fprintf(stderr, "oh\n");
fflush(stderr);
raise(sig);
}
int main() {
int r;
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sa.sa_handler = handler;
sa.sa_flags = SA_RESETHAND;
sigfillset(&sa.sa_mask);
r = sigaction(SIGINT, &sa, NULL);
assert(r == 0);
kill(getpid(), SIGINT);
return 0;
} When compiled without |
It seems that the problem happens somewhere in |
Just a short follow-up, the stack is in fact:
Took me awhile to disassemble and find proper chunks, the link for the crash line is: https://github.com/freebsd/freebsd/blob/master/lib/libthr/thread/thr_sig.c#L244 It seems that |
It is odd, because
0x44 flags are = SA_SIGINFO | SA_RESETHAND Going to try passing this without linking to libthr. |
Looks like a kernel bug to me: #include <assert.h>
#include <pthread.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
static void handler(int sig, siginfo_t* info, void* u) {
fprintf(stderr, "oh %d %p %p\n", sig, info, u);
fflush(stderr);
raise(sig);
}
int main() {
int r;
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sa.sa_sigaction = handler;
sa.sa_flags = SA_RESETHAND | SA_SIGINFO;
sigfillset(&sa.sa_mask);
r = sigaction(SIGINT, &sa, NULL);
assert(r == 0);
kill(getpid(), SIGINT);
return 0;
} Is producing:
/me is looking deeply into the kernel code now. |
It looks like it is using:
For the handler. |
Looks like a combination of |
Related FreeBSD discussion: https://lists.freebsd.org/pipermail/freebsd-threads/2014-November/005613.html . Looks like a kernel bug for me. |
And the relevant fix: freebsd/freebsd-src@4501dad . I'll ask on a mailing list about possibility of backporting it to 10.x FreeBSD. Looks like we'll need to work around it in io.js. |
And node.js for sure too :) |
FreeBSD has a nasty bug with SA_RESETHAND reseting the SA_SIGINFO, that is in turn set for a libthr wrapper. This leads to a crash. Work around the issue by manually setting SIG_DFL in the signal handler. Fix: nodejs/node-v0.x-archive#9326
Should be fixed by nodejs/node#1218 |
FreeBSD has a nasty bug with SA_RESETHAND reseting the SA_SIGINFO, that is in turn set for a libthr wrapper. This leads to a crash. Work around the issue by manually setting SIG_DFL in the signal handler. Fix: nodejs/node-v0.x-archive#9326 PR-URL: #1218 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The fix was landed in io.js nodejs/node@b64983d , please consider backporting. |
Many thanks @indutny. /cc @tjfontaine Let's back port this. |
This is a backport of b64983d. Original commit message: src: reset signal handler to SIG_DFL on FreeBSD FreeBSD has a nasty bug with SA_RESETHAND reseting the SA_SIGINFO, that is in turn set for a libthr wrapper. This leads to a crash. Work around the issue by manually setting SIG_DFL in the signal handler. Fix: nodejs#9326 PR-URL: nodejs/node#1218 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Fixes nodejs#9326.
This is a backport of b64983d. Original commit message: src: reset signal handler to SIG_DFL on FreeBSD FreeBSD has a nasty bug with SA_RESETHAND reseting the SA_SIGINFO, that is in turn set for a libthr wrapper. This leads to a crash. Work around the issue by manually setting SIG_DFL in the signal handler. Fix: nodejs#9326 PR-URL: nodejs/node#1218 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Fixes nodejs#9326.
This is a backport of b64983d. Original commit message: src: reset signal handler to SIG_DFL on FreeBSD FreeBSD has a nasty bug with SA_RESETHAND reseting the SA_SIGINFO, that is in turn set for a libthr wrapper. This leads to a crash. Work around the issue by manually setting SIG_DFL in the signal handler. Fix: #9326 PR-URL: nodejs/node#1218 Reviewed-By: Johan Bergström <bugs@bergstroem.nu> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Fixes #9326. Reviewed-By: Trevor Norris <trev.norris@gmail.com> PR-URL: #14184
Fixed by 61fe1fe. Thank you @indutny and @trevnorris! |
Problem with building: ../src/node.cc:2793:34: error: use of undeclared identifier 'nullptr'
CHECK_EQ(sigaction(signo, &sa, nullptr), 0);
^
../src/util.h:51:31: note: expanded from macro 'CHECK_EQ'
#define CHECK_EQ(a, b) CHECK((a) == (b))
^
../src/util.h:48:37: note: expanded from macro 'CHECK'
# define CHECK(expression) assert(expression)
^
/usr/include/assert.h:54:21: note: expanded from macro 'assert'
#define assert(e) ((e) ? (void)0 : __assert(__func__, __FILE__, \
^ I guess need to change "nullptr" to "NULL", or just make this include #include <cstddef> |
@vba-ds When integrating the change from io.js, I did not do my job correctly and did not actually test it. My apologies for that and thank you for letting us know! Since our code base still doesn't enable C++11 features, the change should use |
61fe1fe backported b64983d from io.js, but failed to change nullptr to NULL, which lead to a build break on FreeBSD since the current build system doesn't enable support for C++11. This change replaces nullptr by NULL, and has been tested on FreeBSD 10.1-RELEASE-p8. Fixes nodejs#9326.
61fe1fe backported b64983d from io.js, but failed to change nullptr to NULL, which lead to a build break on FreeBSD since the current build system doesn't enable support for C++11. This change replaces nullptr by NULL, and has been tested on FreeBSD 10.1-RELEASE-p8. Fixes #9326. Reviewed-By: Colin Ihrig <cjihrig@gmail.com> PR-URL: #14819
Should be fixed for good in f99eaef. Thank you and sorry for the confusion. |
I'm running this simple source:
It works fine until I hit Ctrl+C: it segfaults.
Here's the backtrace:
I'm running FreeBSD 10.1-RELEASE with node installed from the ports.
If I run a script that contains only a
console.log
it doesn't crash when it exits.The text was updated successfully, but these errors were encountered: