Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invoking previously registered action #130

Open
hniksic opened this issue Feb 21, 2022 · 2 comments
Open

Invoking previously registered action #130

hniksic opened this issue Feb 21, 2022 · 2 comments

Comments

@hniksic
Copy link

hniksic commented Feb 21, 2022

I have a stats library which I'd like to have the option of printing its stats on ^C. So I wrote something like this:

pub fn report_on_interrupt() {
    let mut signals = Signals::new(&[SIGINT]).unwrap();
    thread::Builder::new()
        .name("SIGINT helper".to_string())
        .spawn(move || {
            for _ in signals.forever() {
                dump_stat_reports();
                let _ = signal_hook::low_level::emulate_default_handler(SIGINT);
                std::process::exit(255); // just in case
            }
        })
        .unwrap();
}

This works nicely - if you interrupt the program with ^C, a report gets printed, and the program exists as if ^C were pressed (the shell reports an exit status of 130).

But this approach will clobber someone else who installs a SIGINT hook - my cleanup code just executes the default action, where I'd like it to remember the previously installed action and execute that. So I wrote something like this:

pub fn report_on_interrupt() {
    // safety: we don't install a useful action, we call this for side effect of getting the previous action
    let orig_action = unsafe {
        signal_hook::low_level::register(SIGINT, || ()).unwrap()
    };
    let mut signals = Signals::new(&[SIGINT]).unwrap();
    thread::Builder::new()
        .name("SIGINT helper".to_string())
        .spawn(move || {
            for _ in signals.forever() {
                dump_stat_reports();
                signal_hook::low_level::unregister(orig_action);
                let _ = signal_hook::low_level::raise(SIGINT);
                std::process::exit(255); // just in case
            }
        })
        .unwrap();
}

Of course, the above doesn't work, It always exits with 255 - first, because unregister doesn't restore the previous action, it just removes the dummy one we've installed. And second because unregister docs explicitly specify that it will never restore the default terminating action.

Is it possible to implement the behavior I'd like using signal_hook?

@vorner
Copy link
Owner

vorner commented Mar 1, 2022

Sorry for the late response.

First, the more direct answer to your question. If the previous one registered a signal action/signal handler the unix way (libc::signal or similar), then their handler is executed first, before anything that is done through signal-hook:

https://github.com/vorner/signal-hook/blob/master/signal-hook-registry/src/lib.rs#L324

Then, the hooks by signal-hook are all executed in the order they were registered, one by one.

However, a lot of these hooks just postpone the actual stuff for later ‒ for example, what you use has some small code that runs inside the signal handler. That small code sets few flags and wakes up the blocked thread. Then that thread executes. If some other thread does similar thing, the order in which they wake up is not specified and one or the other may win. By the way, at the time you call unregister here, all the other hooks were already called (because unregister actually waits for any running hooks to finish first, it can't manipulate the internal structures).

So, it is somewhat possible, yes, but probably not easy.

The less direct answer is, the API you provide seems subtly bit wrong to me ‒ it actually does two unrelated things. Prints the statistics and terminates. That API is very much „global“ (but what manipulating signal handlers isn't). I'd try to find out some way to either take a cleanup callback to get called, or leave the „wiring“ of signal to print up to the user to have under control.

As a shameless plug, I also maintain the spirit libraries that handle all bunch of application lifetime things (reading configuration, handling signals, etc). You probably could just use that / suggest to use that and install an on_terminate callback and do the statistics printing from there.

@hniksic
Copy link
Author

hniksic commented Mar 1, 2022

First, the more direct answer to your question. If the previous one registered a signal action/signal handler the unix way (libc::signal or similar), then their handler is executed first, before anything that is done through signal-hook

Ok, that wasn't obvious to me. That means that my code is correct wrt handlers registered from outside signal-hook. But I'm still unclear on how to get the effect of performing an action SIGINT and then exiting as if SIGINT wasn't handled, while allowing others (including signal-hook users) the same privilege.

The less direct answer is, the API you provide seems subtly bit wrong to me ‒ it actually does two unrelated things. Prints the statistics and terminates.

The idea is to just print the statistics. But if I didn't terminate, the program would just keep running as if ^C wasn't pressed, right? Explicit termination is only needed to counter-act the fact that SIGINT is handled.

As a shameless plug, I also maintain the spirit libraries that handle all bunch of application lifetime things

Thanks, I'll look into it - but in this case the API is provided by a library, which can't depend on the program it runs in using a particular application framework.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants