vorner/signal-hook

Allow turning off SA_RESTART

marcan opened this issue · 4 comments

marcan commented

Using a signal to trigger a custom exit path from a daemon/"infinite loop" app is a common pattern. However, if the app runs blocking I/O in the main thread, this requires turning off SA_RESTART: otherwise libc will automatically restart any blocking I/O calls after a signal, without giving the app a chance to react to the signal (e.g. check a flag). signal-hook right now hardcodes SA_RESTART.

In my case, the app reads data from a device which could be idle "forever", so there is no way to force it to exit out into user Rust code as long as SA_RESTART is set (without some ugly thing like closing the device directly in the signal handler, but that violates all the other layerings/abstractions so it kind of defeats the purpose).

Right now I am hacking it off after setting up the handler with signal-hook:

    unsafe {
        let mut act: libc::sigaction = core::mem::zeroed();
        assert!(libc::sigaction(signal_hook::consts::SIGQUIT, core::ptr::null(), &mut act) == 0);
        act.sa_flags &= !libc::SA_RESTART;
        assert!(libc::sigaction(signal_hook::consts::SIGQUIT, &mut act, core::ptr::null_mut()) == 0);
    }

... but it would be really nice if I didn't have to dig into libc to do that.

vorner commented

Hello

I've always found relying on interrupting system calls as error prone. Let's say you have something like this:

while (!terminate_flag.load()) {
    // But what if the signal arrives in here, before entering the read? Then the read will still get stuck forever.
    connection.read(buffer);
    handle_data(buffer);
}

Are you able to do such thing without this race condition?

Furthermore, what if the application spawns another thread (I'd say any dependency in Rust code can do that) and the signal happens to get delivered there instead of the main thread?

Or, can you provide a use case where it is actually correct without SA_RESTART and not with it, not merely works most of the times?

marcan commented

In this case it's a debug action, so "works almost all of the time" is certainly a lot more useful than "never works" ;)

The "correct" approach here would be to use a pipe to signal the main thread, so it can poll() on both it and the device. To implement that (in safe rust), signal_hook would have to offer that primitive (write to a file descriptor, or some higher level wrapper over the concept) as a signal handler.

vorner commented

I'm kind of reluctant to provide "works sometimes" solutions, the goal of the library is towards doing it correctly.

It does provide the primitive with the pipe (or, for technical reasons, a unix-domain socket): https://docs.rs/signal-hook/latest/signal_hook/low_level/pipe/index.html. You can also register both the flag and the pipe-write, the order of their execution is the same as the order of registration.

The iterator is just a wrapper over the pipe and flags for all the signals.

A completely different alternative would also be to set a timeout on the blocking file descriptor: https://doc.rust-lang.org/nightly/std/net/struct.TcpStream.html#method.set_read_timeout. That way if there are no data until the timeout happens, the one read fails, but next read can be executed in the next iteration. This is a bit non-elegant because it a) doesn't react immediately to the signal, but only after the timeout happens and b) introduces "activity" even if everything is idle. But might be good enough for debug purposes.

marcan commented

Heh, I somehow completely missed that module.

Feel free to close this then, I'm fine with continuing to use the libc hack (for my use case it really doesn't matter that it's not perfect, but I understand not wanting to implement potential footguns like that).