node: reset signal handler to SIG_DFL on FreeBSD

[indutny]

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

R=@bnoordhuis

[indutny]

Also, cc @trevnorris @jbergstroem

[jbergstroem]

Can confirm that it fixes the segfault. I need to read up in the manpages before calling it a bug though.

[bnoordhuis]

@indutny You mention SA_SIGINFO. Does that mean that with SA_SIGINFO | SA_RESETHAND it does work?

[indutny]

Yeah, basically SA_RESETHAND resets to default signal implementation and this resets all other flags, including SA_SIGINFO. So the handler is invoked as there was no SA_SIGINFO.

Considering that this behavior is fixed in FreeBSD master branch - I suppose it should be a bug after all :)

Looks like I forgot to give a link: freebsd/freebsd-src@4501dad

[indutny]

And a test case from node.js issue:

#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;
}

[indutny]

NOTE: SA_SIGINFO is used internally in libthr, and libthr overrides libc's sigaction when linking with -pthread.

[indutny]

@bnoordhuis sorry, got your question work. It is the SA_SIGINFO | SA_RESETHAND that does not work, both of them works fine when using separately.

[bnoordhuis]

Right, I understand now. I assume this is going to be fixed in 10.2? Perhaps we can simply wait it out and note it in the known issues list until then. I'm not really a fan of adding hacks for OS bugs, they tend to stick around for a long time.

[bnoordhuis]

I don't think you have to call sigfillset() here.

[indutny]

Fixed.

[indutny]

@bnoordhuis the thing is that io.js is crashing at SIGINT on FreeBSD at the moment. I'm quite sure we should fix this and revert once FreeBSD guys will reply to this and if they'll tell us that they are going to backport the fix.

[bnoordhuis]

Well, alright. LGTM with nit.

[indutny]

@bnoordhuis may I ask you to land it? I'm on a terrible internet atm.

[jbergstroem]

@indutny thanks for the link. What about FreeBSD systems that has the patch (assume it lands or you're on -current)? I have a -current but it actually fails on v8; need to investigate.

Edit: another v8 bug. I'll send it upstream.
Edit2: After fixing the bug, I can say that it returns a similar interrupt.

[Fishrock123]

missing .js

[indutny]

Fixed.

[indutny]

CI: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/350/ with a fixes

[indutny]

it seems that the patch is already in stable/10, shame on me :( But it wasn't released yet.

@jbergstroem what do you mean by "similar interrupt"? Works or broken?

[indutny]

Huh, crashes on CI... WTF, it works just fine in a vbox. @jbergstroem does this patch work for you?

[indutny]

Oh, very interesting. It crashes when I run it with gmake test, works just fine when I run directly.

[indutny]

Ahaha, I think it is just related to the way test runner checks the exit code :) No actual crash is happening.

[indutny]

Another CI run: https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/351/ should work fine now.

[indutny]

Yay, appears to be passing now!

[indutny]

@jbergstroem @bnoordhuis : could you PTAL and LGTY?

[jbergstroem]

@indutny What I meant was that the signal emitted was similar between a "fixed" and "broken" FreeBSD: exiting non-null. With catching exit code in the test it LGTM.