Add signal_ignore() and signal_default()

[cpick]

What does this PR do

Provide safe mechanisms to ignore or reset a signal's action to the default.

Fixes #587.

Checklist:

• I have read CONTRIBUTING.md
• I have written necessary tests and rustdoc comments
• A change log has been added if this PR modifies nix's API

[asomers]

I can see the value of this PR; it allows something to be done safely that was previously only possible unsafely. It also avoids the problem that the existing sigaction function ignores the flags and mask field when setting SIG_DFL or SIG_IGN. But it creates a layering violation by layering one public API atop another. And I find the names confusing, too. Maybe this means that the original API wasn't well thought out? What would you think about making most of these functions into methods of Signal, like this:

pub struct MaybeSigAction {
    /// Cast into a `SigAction`.
    ///
    /// # Safety
    /// There is no guarantee that the old signal handler was installed correctly. If it was installed by this crate, it will be. But
    /// if it was installed by, for example, C code, then there is no guarantee its function pointer is valid. In that case, this
    /// function effectively dereferences a raw pointer of unknown provenance.
    unsafe fn into(self) -> SigAction {...}
}
pub enum Signal {SIGHUP, SIGINT, ...}
impl Signal {
    pub fn kill(...) {...}
    pub fn killpg(...) {...}
    pub fn is_default() {...}
    pub fn set_default(...) {...}
    pub fn set_handler(...) -> Result<MaybeSigAction> {...}
    pub fn get_handler() -> Result<MaybeSigAction> {...}
    ...
}

This API provides better namespacing than the original. With appropriately chosen definitions for types like SigAction and Handler it eliminates the unused mask and flags fields when setting the SIG_IGN or SIG_DFL. And it eliminates unsafety by moving it to a final casting step. So you can use it like this:

/// Set a signal handler safely, ignoring the old value
let _ = Signal::SIGINT.set_handler(...).unwrap();
/// Set a signal handler and unsafely dereference the old handler
let old: SigAction = unsafe{ Signal::SIGINT.set_handler(...).unwrap().into() }

What do you think, @SteveLauC ?

[cpick]

Thanks for having a look!

But it creates a layering violation by layering one public API atop another.

To my eye this is reasonable because the upper layer is adding the "safety functionality", but I'm looking at it from a pretty narrow scope.

And I find the names confusing, too.

My goal is to make it possible to ignore signals/reset actions safely, I'm definitely not precious about the method naming or implementation. Very happy to adjust for clarity, etc.

Maybe this means that the original API wasn't well thought out? What would you think about making most of these functions into methods of Signal, like this:
...

I can see how a wider re-work could end up in a better place overall.

/// Set a signal handler safely, ignoring the old value
let _ = Signal::SIGINT.set_handler(...).unwrap();

I believe this would need to remain unsafe because of all the regular restrictions that POSIX puts on signal handler behavior (reentrancy, minimal interactions with global state, async-signal-safe functions only, etc).

[SteveLauC]

Sorry for the late response, I think I accidentally ignored that ping.

---

I believe we should allow our users to do the following things in safe Rust:

1. set_default
2. set_ignored
3. is_default
4. is_ignored

For the interface, I slightly tend to the functions proposed in this PR, the impl Signal way IMHO kind of makes things a little bit complex.

4 functions may be a little more, we can probably do:

The naming is also bad, we should use some better ones

enum SafeSignalHanlder {
    Default,
    Ignore,
}

fn signal_is(sig: Signal, handler: SafeSignalHandler) -> Result<bool>;
fn signal_set(sig: Signal, handler: SafeSignalHandler) -> Result<()>;

---

For the mechanism to query existing signal handler, I tend to change the sigaction argument of sigaction() to be optional:

pub unsafe fn sigaction(signal: Signal, sigaction: Option<&SigAction>) -> Result<SigAction>

---

/// Set a signal handler safely, ignoring the old value
let _ = Signal::SIGINT.set_handler(...).unwrap();

I believe this would need to remain unsafe because of all the regular restrictions that POSIX puts on signal handler behavior (reentrancy, minimal interactions with global state, async-signal-safe functions only, etc).

I think @asomers' s proposal isolates all the unsafety into MaybeSigAction, and this function will take a SigAction, which has to be cast from a MaybeSigAction, when doing this, users should ensure that the signal handlers set by them are safe.