Signal handling, part 2

[itizir]

It's interesting to see how the 'monitor' approach was suggested as an alternative way of handling signals. Still, regardless, there seem to be other ways to make things cleaner with few little steps.

The default behaviour regarding signal handling is a bit confusing: if ForwardSignals is not set, all signals get captured (thus ignored) by the parent and forwarded to the child. Not sure this is the intended behaviour, at least it wouldn't be very clear from the description. I would suggest that if ForwardSignals is nil, Notify shouldn't be called for the forwarding channel. Or the documentation should be clearer (and one would have to, for instance, make a list with just KILLSIG to effectively not forward any signals).

On top of that, there is a small issue when signals are both meant to be sent on the 'ignore' and 'forward' channels (it is arguably unnecessary/inconsistent to specify the same signals in both lists, but since the current default behaviour is to forward all messages...): because the channels are not buffered, such a signal will end up in only one of them. This at the very least should be fixed...

I'm opening a PR, starting with this latter change. Let me know what makes most sense! :)

[itizir]

By the way, noticed another issue: syscall.ExitStatus() may return -1, so using this convention to tell the code that it is running as the child and should continue with normal execution past the Wrap is problematic. :s

[brandonbloom]

Agreed that this behavior is very confusing and seems to not match the README.

Until the code is fixed, here's a workaround:

exitCode, err := panicwrap.Wrap(&panicwrap.WrapConfig{
        Handler: panicHandler,
        IgnoreSignals: []os.Signal{
                // Disable broken signal handling.
                // See <https://github.com/mitchellh/panicwrap/issues/19>.
                syscall.Signal(-1),
        },
        ForwardSignals: []os.Signal{
                os.Interrupt,
                os.Kill,
        },
})