https://codereview.chromium.org/11195045/diff/9001/src/platform-linux.cc File src/platform-linux.cc (right):
https://codereview.chromium.org/11195045/diff/9001/src/platform-linux.cc#newcode1074 src/platform-linux.cc:1074: old_signal_handler_.sa_sigaction(signal, info, context); If you want to do this right, things get a lot more complicated. In practice, you can probably skip some of the more complicated corner-cases and you'll be OK. You ideally should check the signal mask first. If the signal was originally masked, then no matter what you do, it would never be raised. Of course, inside of the signal handler, this is all too late. The signal was obviously unmasked, because otherwise you would never be inside your own signal handler. It might in fact be masked right now, though, as that happens unless you set the SA_NODEFER flag. Once you know that signal delivery was in fact expected, you now have to determine what the signal disposition is. For that, you should check whether SA_SIGINFO was set in sa_flags. If so, then your code is correct. You can just call the function in sa_sigaction. If it wasn't set though, you are looking at a signal that behaves as if it had been set with the old-school signal(2) function. This means a) you have to look at sa_handler rather than at sa_sigaction, and b) you have to also be prepared to handle SIG_IGN and SIG_DFL. These are magic constants and you can just compare them directly against the value of sa_handler. If it is SIG_IGN, things are easy. You don't need to do anything. If it is SIG_DFL, things are a little more complicated. You now need to check what the default signal disposition is; the man page for signal(7) tells you. In the case of SIGPROF, it terminates the program. You could just call _exit() (please note the underscore!). But the exit code won't be quite right. If you need the correct exit code, make sure that your SIGPROF signal is masked (as it would be unless you set SA_NODEFER) and then raise SIGPROF again. This will result in the expect exit code. Only, there is another complication. I don't believe that SIGPROF is a synchronous signal, and signal masks are a per-thread property. So, SIGPROF is most likely unmasked in another thread and the kernel could very well decide to deliver it there. Rather than calling "raise(SIGPROF)", you might want to do "tgkill(-1, gettid(), SIGPROF)". I don't know whether we currently expose "gettid()"; if we don't, then use "syscall(__NR_gettid)". Now that you know how to handle SIG_IGN and SIG_DFL, the remaining question is what to do if sa_handler points to an actual function. This is the most difficult problem to solve, as the two different types of signal handlers expect completely different stack frames (at least on some architectures). You can (often) convert one into the other; if I recall correctly, a sigaction-style stack frame is technically a superset of a sighandler-style stack frame. This is all highly specific to the target platform though. And it is of course Linux specific; POSIX doesn't allow for any of what you are doing here. But I suspect we are running into some diminishing returns. As you mentioned earlier, it is hopefully safe to assume that both handlers (if present at all) are sigaction-style handlers. I'll leave it up to you to decide what to do if this last assumption doesn't hold. You could fail with a CHECK() or at least a DCHECK(). Or you could simply decide to do what we did before and ignore the old handler. You could also do crazy things with repointing the signal handler and then rethrowing the signal; POSIX would be happier if you did that. But again, that's probably more trouble than what it's worth. And I don't even want to know what the associated unittest is going to look like (you do plan on testing this, don't you?). Talking about CHECK() and DCHECK(). Neither one of them is technically safe to be called at this time and could quite conceivably result in a dead lock or worse. Hopefully, it's a rare case and we are OK with very rare inexplicable bugs; but you might still want to document this. And just for completeness sake, there is one last bit of information. This is probably not something that you'll run into though. Chaining of signal handlers interposes an additional stack frame. This break any call to sigreturn(), and it could also break other low-level code. Fortunately, you are unlikely to see that in any signal handler that is written in a high-level language. But if you wanted to be really precise, you would pop your own stack frame prior to chaining. https://codereview.chromium.org/11195045/diff/9001/src/platform-linux.cc#newcode1212 src/platform-linux.cc:1212: if (signal != SIGPROF) return; This seems somewhat bogus. That really should never happen. And if it does, then why do decide to skip calling the old signal handler. Isn't that just papering over bugs. Maybe a CHECK() or at least a DCHECK() would be more appropriate. https://codereview.chromium.org/11195045/diff/9001/src/platform-linux.cc#newcode1263 src/platform-linux.cc:1263: sampler->Tick(sample); I don't see any code here that preserves "errno". That is broken. You will almost certainly clobber this value and cause very subtle bugs. All of that is ignoring the fact that SIGPROF is an asynchronous signal and I suspect at least some of what you are calling is not async signal safe ... but I haven't bothered going over that code with a fine-toothed comb. Maybe, somebody else already did and it is in fact OK. In general though, writing signal handlers in C++ is almost impossible to get right. https://codereview.chromium.org/11195045/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
