Default kill does not kill whole process?

[pickfire]

I noticed that the default kill which is provided by low_level::raise or emulate_default_handler only sends the signal to the current process rather than to the whole process like kill(0, SIGSTOP) this seemed to have cause issue for suspend on tokio stuff that uses multiple threads.

For example, the usecase of suspend with ctrl-z in helix editor was using signal-hook to emulate default handler of SIGSTOP when SIGTSTP handler was raised. But when helix is used within git to edit git commit message, it does not suspend correctly, most likely is because some other background thread was still running. The fix was to switch from emulate_default_handler(SIGTSTP) to kill(0, SIGSTOP) which will let the whole process receive the event rather than just the running process. See helix-editor/helix#1843

I did two small examples locally, one with tokio (using crossterm) and one without tokio which does stuff in single thread (using termwiz) and found out that the one that use tokio does not suspend correctly, so I tried sending the signal to whole process using kill(0, SIGSTOP) and it worked, if you want I can send the examples.

Is there any reason why default kill is only for current process rather than all process that it can send signal to? Which will affect those that use tokio since it might use multiple process. My knowledge may be limited in this area, please correct me if I am wrong. Any suggestions are welcome.

[vorner]

Hello

First, low_level::raise is just a wrapper around libc::raise, nothing more. This is how raise behaves for some 50 years already and who would I be to try to change it. The emulate_default_handler works by resending the signal to itself and not other processes, because signal hook only receives signals for the same process.

kill(0, …) sends signals to whole process group, which means other processes (forked). If I catch process for myself and broadcast it to multiple processes, then I'm not simply emulating my own default handler that I've replaced.

I think you have a mixup in threads and processes somewhere in your reasoning. And I suspect shells or terminal emulators are supposed to catch and broadcast it or something, but that certainly is not how the default handler behaves.

You can try sending the examples, but I believe you'd have a similar problem with default handler.

[pickfire]

termwiz (single-thread)

// new project with these two dependencies
// signal-hook = "0.3"
// termwiz = "0.15"
use signal_hook::low_level;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::Arc;
use std::time::Duration;
use termwiz::caps::Capabilities;
use termwiz::input::{InputEvent, KeyCode, KeyEvent, Modifiers};
use termwiz::terminal::{new_terminal, Terminal};

const CTRL_C: KeyEvent = KeyEvent {
    key: KeyCode::Char('C'),
    modifiers: Modifiers::CTRL,
};

const CTRL_Z: KeyEvent = KeyEvent {
    key: KeyCode::Char('Z'),
    modifiers: Modifiers::CTRL,
};

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let tstp = Arc::new(AtomicBool::new(false));
    signal_hook::flag::register(signal_hook::consts::SIGTERM, tstp.clone())?;

    let caps = Capabilities::new_from_env()?;
    let mut terminal = new_terminal(caps)?;
    terminal.set_raw_mode()?;
    loop {
        if let Some(event) = terminal.poll_input(Some(Duration::from_millis(100)))? {
            println!("Event {event:?}\r");
            match event {
                InputEvent::Key(CTRL_C) => break,
                InputEvent::Key(CTRL_Z) => low_level::raise(signal_hook::consts::SIGTSTP)?,
                _ => {}
            }
        }

        if tstp.load(Ordering::Relaxed) {
            println!("Received signal SIGTSTP\r");
            terminal.set_cooked_mode()?;
            low_level::emulate_default_handler(signal_hook::consts::SIGTSTP)?;
            terminal.set_raw_mode()?;
        }
    }
    Ok(())
}

crossterm (multi-thread probably) modified from `examples/print.rs`

// add these to dev-dependencies and run `cargo run --example print`
// signal-hook-tokio = { version = "0.3", features = ["futures-v0_3"] }
// tokio = { package = "tokio", version = "~1", features = ["full"] }
// futures-util = "*"
// crossterm = { version = "0.23", features = ["event-stream"] }
use libc::c_int;
use signal_hook::consts::signal::*;
use signal_hook::low_level;

use crossterm::{event::EventStream, terminal};
use signal_hook_tokio::Signals;

#[tokio::main]
async fn main() -> Result<(), Box<dyn std::error::Error>> {
    const SIGNALS: &[c_int] = &[
        SIGTERM, SIGQUIT, SIGINT, SIGTSTP, SIGWINCH, SIGHUP, SIGCHLD, SIGCONT,
    ];
    let mut reader = EventStream::new();
    let mut sigs = Signals::new(SIGNALS)?;
    use futures_util::StreamExt;
    terminal::enable_raw_mode()?;
    loop {
        tokio::select! {
            maybe_event = reader.next() => {
                match maybe_event {
                    Some(Ok(event)) => println!("Event::{event:?}\r"),
                    Some(Err(e)) => println!("Error: {e:?}\r"),
                    None => break,
                }
            }
            Some(signal) = sigs.next() => {
                println!("Received signal {:?}\r", signal);
                // After printing it, do whatever the signal was supposed to do in the first place
                if let SIGTSTP = signal {
                    terminal::disable_raw_mode()?;
                    low_level::emulate_default_handler(signal)?;
                    terminal::enable_raw_mode()?;
                } else {
                    low_level::emulate_default_handler(signal)?;
                }
            }
        }
    }
    Ok(())
}

To reproduce the issue, run the application and ctrl-z when inside git, then the multi-thread version will get stuck, but if crossterm example was changed from low_level::emulate_default_handler(signal) to kill(0, SIGSTOP), then it seemed to be able to suspend correctly.

[archseer]

when inside git

You'll need to specify how to do that

[pickfire]

Sorry, didn't specify the clear behavior. When inside git, means git commit the application as editor, then ctrl-z.

[vorner]

Took me a bit longer to get around to this. I have not tried the first (but I suspect there's a mixup between registering SIGTERM and then handling/raising SIGTSTP.

Looking at the second one, are you sure it's the exact reproducer I should be trying? When I press CTRL+Z or CTRL+C (both directly in shell and under git), I'm getting only the Event: … messages, not Received signal ones. Strace agrees with them not being delivered. I suspect that by turning the terminal to raw mode, you claim you'll handle all these keyboard shortcuts yourself and they won't generate the signals by themselves (I think usually it's the shell that watches for them and sends them).

Anyway, two things that might help debugging the issue (because I'm still not convinced this is related to signal-hook):

• Run the tokio application as single threaded (#[tokio::main(flavor="current_thread")]). Check with something like pstree to see that there's indeed only one thread and only one process. Because signals are process-wide (not per thread) and the kill(0, …) is about other processes.
• Try running it with strace, to see what exactly happens around signals.

[pickfire]

I think can just close this for now, might take me some time to investigate in this.