From 34934733b364ed460c647f1775f5865048833f54 Mon Sep 17 00:00:00 2001
From: Cole Helbling <cole.e.helbling@outlook.com>
Date: Mon, 13 Mar 2023 05:08:57 -0700
Subject: [PATCH] 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.
---
 Cargo.lock                    |  1 +
 helix-term/Cargo.toml         |  1 +
 helix-term/src/application.rs | 32 +++++++++++++++++++++++++++-----
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index de985bca1..70d3eaef2 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1177,6 +1177,7 @@ dependencies = [
  "helix-view",
  "ignore",
  "indoc",
+ "libc",
  "log",
  "once_cell",
  "pulldown-cmark",
diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml
index bca567c28..279fdcf8b 100644
--- a/helix-term/Cargo.toml
+++ b/helix-term/Cargo.toml
@@ -68,6 +68,7 @@ grep-searcher = "0.1.11"
 
 [target.'cfg(not(windows))'.dependencies]  # https://github.com/vorner/signal-hook/issues/100
 signal-hook-tokio = { version = "0.3", features = ["futures-v0_3"] }
+libc = "0.2.132"
 
 [build-dependencies]
 helix-loader = { version = "0.6", path = "../helix-loader" }
diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs
index 781e6fa71..1cec213cb 100644
--- a/helix-term/src/application.rs
+++ b/helix-term/src/application.rs
@@ -40,10 +40,7 @@
 
 use crossterm::{event::Event as CrosstermEvent, tty::IsTty};
 #[cfg(not(windows))]
-use {
-    signal_hook::{consts::signal, low_level},
-    signal_hook_tokio::Signals,
-};
+use {signal_hook::consts::signal, signal_hook_tokio::Signals};
 #[cfg(windows)]
 type Signals = futures_util::stream::Empty<()>;
 
@@ -447,7 +444,32 @@ pub async fn handle_signals(&mut self, signal: i32) {
         match signal {
             signal::SIGTSTP => {
                 self.restore_term().unwrap();
-                low_level::emulate_default_handler(signal::SIGTSTP).unwrap();
+
+                // SAFETY:
+                //
+                // - helix must have permissions to send signals to all processes in its signal
+                //   group, either by already having the requisite permission, or by having the
+                //   user's UID / EUID / SUID match that of the receiving process(es).
+                let res = unsafe {
+                    // A pid of 0 sends the signal to the entire process group, allowing the user to
+                    // regain control of their terminal if the editor was spawned under another process
+                    // (e.g. when running `git commit`).
+                    //
+                    // We have to send SIGSTOP (not SIGTSTP) to the entire process group, because,
+                    // as mentioned above, the terminal will get stuck if `helix` was spawned from
+                    // an external process and that process waits for `helix` to complete. This may
+                    // be an issue with signal-hook-tokio, but the author of signal-hook believes it
+                    // could be a tokio issue instead:
+                    // https://github.com/vorner/signal-hook/issues/132
+                    libc::kill(0, signal::SIGSTOP)
+                };
+
+                if res != 0 {
+                    let err = std::io::Error::last_os_error();
+                    eprintln!("{}", err);
+                    let res = err.raw_os_error().unwrap_or(1);
+                    std::process::exit(res);
+                }
             }
             signal::SIGCONT => {
                 self.claim_term().await.unwrap();