3493473 · helix · git.iko.soy

git.iko.soy/helix

helix-term: send the STOP signal to all processes in the process group (#3546)

* helix-term: send the STOP signal to all processes in the process group From kill(3p): If pid is 0, sig shall be sent to all processes (excluding an unspecified set of system processes) whose process group ID is equal to the process group ID of the sender, and for which the process has permission to send a signal. This fixes the issue of running `git commit`, attempting to suspend helix with ^Z, and then not regaining control over the terminal and having to press ^Z again. * helix-term: use libc directly to send STOP signal * helix-term: document safety of libc::kill * helix-term: properly handle libc::kill's failure I misread the manpage for POSIX `kill` -- it returns `-1` in the failure case, and sets `errno`, which is retrieved via `std::io::Error::last_os_error()`, has its string representation printed out, and then exits with the matching status code (or 1 if, for whatever reason, there is no matching status code). * helix-term: expand upon why we need to SIGSTOP the entire process group Also add a link back to one of the upstream issues.

commit34934733b364ed460c647f1775f5865048833f54 authorCole Helbling <cole.e.helbling@outlook.com> dateMon 13 Mar 2023, 05:08 (4 years ago) parent4f066b1

3 files changed+29−5

compare with master

unifiedsplit

Cargo.lock+1 helix-term/Cargo.toml+1 helix-term/src/application.rs+27−5

Cargo.lock+1

@@ -1177,6 +1177,7 @@ dependencies = [

11771177 "helix-view",

11781178 "ignore",

11791179 "indoc",

1180+ "libc",

11801181 "log",

11811182 "once_cell",

11821183 "pulldown-cmark",

helix-term/Cargo.toml+1

@@ -68,6 +68,7 @@ grep-searcher = "0.1.11"

6868

6969[target.'cfg(not(windows))'.dependencies] # https://github.com/vorner/signal-hook/issues/100

7070signal-hook-tokio = { version = "0.3", features = ["futures-v0_3"] }

71+libc = "0.2.132"

7172

7273[build-dependencies]

7374helix-loader = { version = "0.6", path = "../helix-loader" }

helix-term/src/application.rs+27−5

@@ -40,10 +40,7 @@ use anyhow::{Context, Error};

4040

4141use crossterm::{event::Event as CrosstermEvent, tty::IsTty};

4242#[cfg(not(windows))]

43−use {

44− signal_hook::{consts::signal, low_level},

45− signal_hook_tokio::Signals,

46−};

43+use {signal_hook::consts::signal, signal_hook_tokio::Signals};

4744#[cfg(windows)]

4845type Signals = futures_util::stream::Empty<()>;

4946

@@ -447,7 +444,32 @@ impl Application {

447444 match signal {

448445 signal::SIGTSTP => {

449446 self.restore_term().unwrap();

450− low_level::emulate_default_handler(signal::SIGTSTP).unwrap();

447+

448+ // SAFETY:

449+ //

450+ // - helix must have permissions to send signals to all processes in its signal

451+ // group, either by already having the requisite permission, or by having the

452+ // user's UID / EUID / SUID match that of the receiving process(es).

453+ let res = unsafe {

454+ // A pid of 0 sends the signal to the entire process group, allowing the user to

455+ // regain control of their terminal if the editor was spawned under another process

456+ // (e.g. when running `git commit`).

457+ //

458+ // We have to send SIGSTOP (not SIGTSTP) to the entire process group, because,

459+ // as mentioned above, the terminal will get stuck if `helix` was spawned from

460+ // an external process and that process waits for `helix` to complete. This may

461+ // be an issue with signal-hook-tokio, but the author of signal-hook believes it

462+ // could be a tokio issue instead:

463+ // https://github.com/vorner/signal-hook/issues/132

464+ libc::kill(0, signal::SIGSTOP)

465+ };

466+

467+ if res != 0 {

468+ let err = std::io::Error::last_os_error();

469+ eprintln!("{}", err);

470+ let res = err.raw_os_error().unwrap_or(1);

471+ std::process::exit(res);

472+ }

451473 }

452474 signal::SIGCONT => {

453475 self.claim_term().await.unwrap();