Restoring system handler when slot empties

[shutton]

signal_hook::low_level::unregister() documents that the default handler isn't restored when the last action is removed. I couldn't find a justification for this. I'm trying to implement some temporary conditions where I want to trap signals, then restore the original behavior, but that appears to be unimplemented.

Did this cause issues? A quick 'n dirty (and likely naive) fix appears to simply be this:

--- a/signal-hook-registry/src/lib.rs
+++ b/signal-hook-registry/src/lib.rs
@@ -636,6 +636,11 @@ pub fn unregister(id: SigId) -> bool {
     let mut sigdata = SignalData::clone(&lock);
     if let Some(slot) = sigdata.signals.get_mut(&id.signal) {
         replace = slot.actions.remove(&id.action).is_some();
+        if slot.actions.is_empty()
+            && unsafe { libc::sigaction(id.signal, &slot.prev.info, ptr::null_mut()) } != 0
+        {
+            panic!("sigaction() returned OS Error {}", Error::last_os_error())
+        }
     }
     if replace {
         lock.store(sigdata);

Some quick sanity tests seem to work fine.

[vorner]

Hello

I'm pretty sure I've written the reasons somewhere, but in short, the library assumes:

• Running on arbitrary POSIX system, not only Linux.
• Rust programs are routinely multithreaded.
• Independent parts might want to add or remove hooks without knowing about each other.
• Other libraries might want to add their signal handlers even if they are not based on signal-hook. For that, the chaining (slot.prev.info ‒ equivalent) is used.

Your solution has several problems:

• This is racy in multi-threaded program. A signal might come in between removing the action and unregistering the handler (this can be fixed).
• The signal handler may be „burried“ below other signal handlers someone else might have registered in between and there's no way to remove us from the middle of the chain.
• The fact that the slot is or is not empty depends on behaviour of the other parts you don't know about, which results in sometimes restoring the original handler and sometimes not, because something else might have registered a debug print in the handler, or something.

These put together suggest the unregistering would behave quirkily at best, therefore the behaviour of just list of hooks that can even do nothing because of empty after the first takeover seemed much easier to reason about and make behave consistently when interacting with other parts, so it's not the thing it does by default.

But I believe the use case you describe should be covered by this function: https://docs.rs/signal-hook/0.3.8/signal_hook/flag/fn.register_conditional_default.html. You register this and then turn that on or off by setting that atomic bool. When it's off, the hook does nothing, when on, it invokes the equivalent of the default handler. If you need something more complex, you can invoke the default behaviour directly by https://docs.rs/signal-hook/0.3.8/signal_hook/low_level/fn.emulate_default_handler.html.

Configuring the slot to behave in the way you describe (more likely by checking if it's empty inside the handler and invoking the emulation, to avoid the burried-problem and racy-problem) is planned for the future, but I haven't gotten around to that yet. That's covered in #82.

As I believe the feature is already tracked and your use case should already be somehow covered, I'll close this issue.

[shutton]

I won't contest that -- at best, it's something I would have suggested in an environment where I already know what's going on. In my case, I needed to capture TERM signals within unit tests because I needed to do some teardown of an external mock environment before allowing the program to close, but I also wanted to be able to restore the environment for the next unit test (which would obviously have to operate serially).

In hindsight, this is already incredibly complex due to the fact that Rust unit tests generally run in parallel. Trying to do this via a signal handler probably doesn't even make sense. A watchdog process is probably the way to go.

Thanks for the commentary and explanation.

[vorner]

Hmm, I think I can post few tips around the use case of tests:

• In general, the „best practice“ is considered that tests clean up after themselves (maybe in some Drop implementation, so they clean up even if the test fails and panics). But they also clean up any leftovers after previous runs, because no matter how much careful cleanup you do, the cleanup process can always be killed or fail...
• There's https://crates.io/crates/serial_test for making tests run serially. Or, I've often used a global mutex that the test held during its run.