Repository: sharkdp/fd (language: rust)

Issue:
exit(130) is not the same as raise(SIGINT)
fd handles ^C by propagating `ExitCode::KilledBySigint` up to `main()` which does  `process::exit(exit_code.into())`.  But exit code 130 is a lie told by bash (this comment has a good explanation: https://github.com/rust-lang/rust/issues/62569#issuecomment-774546256).

This is observable from programs that check `WIFEXITED()`/`WIFSIGNALED()`.  Here's a crude example:

```
$ bfs -D exec /dev/null -exec fd . / \; >/dev/null & sleep 1; pkill -INT fd; wait %1
bfs: -D exec: -exec: Executing 'fd' ... [2 arguments]
bfs: warning: Command 'fd' terminated by signal 2 (Interrupt)
$ bfs -D exec /dev/null -exec fd --color=always . / \; >/dev/null & sleep 1; pkill -INT fd; wait %1
bfs: -D exec: -exec: Executing 'fd' ... [3 arguments]
bfs: -D exec: -exec: Command 'fd' failed with status 130
```

The right way to pretend to die by SIGINT is something like `signal(SIGINT, SIG_DFL); raise(SIGINT);`.
exit(130) is not the same as raise(SIGINT)
fd handles ^C by propagating `ExitCode::KilledBySigint` up to `main()` which does  `process::exit(exit_code.into())`.  But exit code 130 is a lie told by bash (this comment has a good explanation: https://github.com/rust-lang/rust/issues/62569#issuecomment-774546256).

This is observable from programs that check `WIFEXITED()`/`WIFSIGNALED()`.  Here's a crude example:

```
$ bfs -D exec /dev/null -exec fd . / \; >/dev/null & sleep 1; pkill -INT fd; wait %1
bfs: -D exec: -exec: Executing 'fd' ... [2 arguments]
bfs: warning: Command 'fd' terminated by signal 2 (Interrupt)
$ bfs -D exec /dev/null -exec fd --color=always . / \; >/dev/null & sleep 1; pkill -INT fd; wait %1
bfs: -D exec: -exec: Executing 'fd' ... [3 arguments]
bfs: -D exec: -exec: Command 'fd' failed with status 130
```

The right way to pretend to die by SIGINT is something like `signal(SIGINT, SIG_DFL); raise(SIGINT);`.


Discussion / hints:
Thank you for reporting this. I guess this should be easy to fix using sth. like [`nix::sys::signal::raise`](https://docs.rs/nix/0.23.0/nix/sys/signal/fn.raise.html)?

~~Or maybe we avoid that dependency and add some unsafe code using [`libc::raise`](https://docs.rs/libc/0.2.107/libc/fn.raise.html) directly?~~ `nix` is in the dependency tree via `ctrlc` anyway.
Yeah that's what I was thinking.  Didn't get around to it yet because I'm not sure what the right behavior is on Windows.  But I could at least fix it on *nix first
Thank you for reporting this. I guess this should be easy to fix using sth. like [`nix::sys::signal::raise`](https://docs.rs/nix/0.23.0/nix/sys/signal/fn.raise.html)?

~~Or maybe we avoid that dependency and add some unsafe code using [`libc::raise`](https://docs.rs/libc/0.2.107/libc/fn.raise.html) directly?~~ `nix` is in the dependency tree via `ctrlc` anyway.
Yeah that's what I was thinking.  Didn't get around to it yet because I'm not sure what the right behavior is on Windows.  But I could at least fix it on *nix first

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index c24c3a599..469adcfb5 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -175,6 +175,7 @@ dependencies = [
  "lazy_static",
  "libc",
  "lscolors",
+ "nix",
  "normpath",
  "num_cpus",
  "once_cell",
diff --git a/Cargo.toml b/Cargo.toml
index c89b2e97a..674772f47 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -57,6 +57,7 @@ features = ["suggestions", "color", "wrap_help"]
 
 [target.'cfg(unix)'.dependencies]
 users = "0.11.0"
+nix = "0.23.0"
 
 [target.'cfg(all(unix, not(target_os = "redox")))'.dependencies]
 libc = "0.2"
diff --git a/src/exit_codes.rs b/src/exit_codes.rs
index 4f8a9748d..e44111bb8 100644
--- a/src/exit_codes.rs
+++ b/src/exit_codes.rs
@@ -1,3 +1,8 @@
+use std::process;
+
+#[cfg(unix)]
+use nix::sys::signal::{raise, signal, SigHandler, Signal};
+
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub enum ExitCode {
     Success,
@@ -21,6 +26,21 @@ impl ExitCode {
     fn is_error(self) -> bool {
         i32::from(self) != 0
     }
+
+    /// Exit the process with the appropriate code.
+    pub fn exit(self) -> ! {
+        #[cfg(unix)]
+        if self == ExitCode::KilledBySigint {
+            // Get rid of the SIGINT handler, if present, and raise SIGINT
+            unsafe {
+                if signal(Signal::SIGINT, SigHandler::SigDfl).is_ok() {
+                    let _ = raise(Signal::SIGINT);
+                }
+            }
+        }
+
+        process::exit(self.into())
+    }
 }
 
 pub fn merge_exitcodes(results: impl IntoIterator<Item = ExitCode>) -> ExitCode {
diff --git a/src/main.rs b/src/main.rs
index 321df542d..172d570ed 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -12,7 +12,6 @@ mod walk;
 
 use std::env;
 use std::path::{Path, PathBuf};
-use std::process;
 use std::sync::Arc;
 use std::time;
 
@@ -54,11 +53,11 @@ fn main() {
     let result = run();
     match result {
         Ok(exit_code) => {
-            process::exit(exit_code.into());
+            exit_code.exit();
         }
         Err(err) => {
             eprintln!("[fd error]: {:#}", err);
-            process::exit(ExitCode::GeneralError.into());
+            ExitCode::GeneralError.exit();
         }
     }
 }
diff --git a/src/output.rs b/src/output.rs
index 828d99504..194cb2adc 100644
--- a/src/output.rs
+++ b/src/output.rs
@@ -1,7 +1,6 @@
 use std::borrow::Cow;
 use std::io::{self, StdoutLock, Write};
 use std::path::Path;
-use std::process;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::Arc;
 
@@ -38,10 +37,10 @@ pub fn print_entry(
     if let Err(e) = r {
         if e.kind() == ::std::io::ErrorKind::BrokenPipe {
             // Exit gracefully in case of a broken pipe (e.g. 'fd ... | head -n 3').
-            process::exit(0);
+            ExitCode::Success.exit();
         } else {
             print_error(format!("Could not write to output: {}", e));
-            process::exit(ExitCode::GeneralError.into());
+            ExitCode::GeneralError.exit();
         }
     }
 }
@@ -95,7 +94,7 @@ fn print_entry_colorized(
     }
 
     if wants_to_quit.load(Ordering::Relaxed) {
-        process::exit(ExitCode::KilledBySigint.into());
+        ExitCode::KilledBySigint.exit();
     }
 
     Ok(())
diff --git a/src/walk.rs b/src/walk.rs
index 789a5008a..ed3bf9f5e 100644
--- a/src/walk.rs
+++ b/src/walk.rs
@@ -3,7 +3,6 @@ use std::ffi::OsStr;
 use std::fs::{FileType, Metadata};
 use std::io;
 use std::path::{Path, PathBuf};
-use std::process;
 use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::mpsc::{channel, Receiver, Sender};
 use std::sync::{Arc, Mutex};
@@ -137,11 +136,9 @@ pub fn scan(path_vec: &[PathBuf], pattern: Arc<Regex>, config: Arc<Config>) -> R
     if config.ls_colors.is_some() && config.command.is_none() {
         let wq = Arc::clone(&wants_to_quit);
         ctrlc::set_handler(move || {
-            if wq.load(Ordering::Relaxed) {
+            if wq.fetch_or(true, Ordering::Relaxed) {
                 // Ctrl-C has been pressed twice, exit NOW
-                process::exit(ExitCode::KilledBySigint.into());
-            } else {
-                wq.store(true, Ordering::Relaxed);
+                ExitCode::KilledBySigint.exit();
             }
         })
         .unwrap();


Test patch: