Error on multiple signal handlers on unix

Cargo.toml

@@ -27,5 +27,13 @@ termination = []
 
 [[test]]
 harness = false
-name = "tests"
-path = "src/tests.rs"
+name = "main"
+path = "tests/main/mod.rs"
+
+[[test]]
+harness = false
+name = "issue_97"
+path = "tests/main/issue_97.rs"
+
+[dev-dependencies]
+signal-hook = "0.3"

src/error.rs

@@ -24,6 +24,11 @@ impl Error {
 
 impl From<platform::Error> for Error {
     fn from(e: platform::Error) -> Error {
+        #[cfg(not(windows))]
+        if e == platform::Error::EEXIST {
+            return Error::MultipleHandlers;
+        }
+
         let system_error = std::io::Error::new(std::io::ErrorKind::Other, e);
         Error::System(system_error)
     }

src/lib.rs

@@ -70,8 +70,8 @@ static INIT: AtomicBool = AtomicBool::new(false);
 /// ```
 ///
 /// # Warning
-/// On Unix, any existing `SIGINT`, `SIGTERM` and `SIGHUP` (if termination feature is enabled) or `SA_SIGINFO`
-/// posix signal handlers will be overwritten. On Windows, multiple handler routines are allowed,
+/// On Unix, the handler registration for `SIGINT`, (`SIGTERM` and `SIGHUP` if termination feature is enabled) or `SA_SIGINFO`
+/// posix signal handlers will fail if a signal handler is already present. On Windows, multiple handler routines are allowed,
 /// but they are called on a last-registered, first-called basis until the signal is handled.
 ///
 /// On Unix, signal dispositions and signal handlers are inherited by child processes created via

src/platform/unix/mod.rs

@@ -109,11 +109,13 @@ pub unsafe fn init_os_handler() -> Result<(), Error> {
         signal::SigSet::empty(),
     );
 
-    #[allow(unused_variables)]
     let sigint_old = match signal::sigaction(signal::Signal::SIGINT, &new_action) {
         Ok(old) => old,
         Err(e) => return Err(close_pipe(e)),
     };
+    if sigint_old.handler() != signal::SigHandler::SigDfl {
+        return Err(close_pipe(nix::Error::EEXIST));
+    }
 
     #[cfg(feature = "termination")]
     {
@@ -124,18 +126,25 @@ pub unsafe fn init_os_handler() -> Result<(), Error> {
                 return Err(close_pipe(e));
             }
         };
-        match signal::sigaction(signal::Signal::SIGHUP, &new_action) {
-            Ok(_) => {}
+        if sigterm_old.handler() != signal::SigHandler::SigDfl {
+            signal::sigaction(signal::Signal::SIGINT, &sigint_old).unwrap();
+            return Err(close_pipe(nix::Error::EEXIST));
+        }
+        let sighup_old = match signal::sigaction(signal::Signal::SIGHUP, &new_action) {
+            Ok(old) => old,
             Err(e) => {
                 signal::sigaction(signal::Signal::SIGINT, &sigint_old).unwrap();
                 signal::sigaction(signal::Signal::SIGTERM, &sigterm_old).unwrap();
                 return Err(close_pipe(e));
             }
+        };
+        if sighup_old.handler() != signal::SigHandler::SigDfl {
+            signal::sigaction(signal::Signal::SIGINT, &sigint_old).unwrap();
+            signal::sigaction(signal::Signal::SIGTERM, &sigterm_old).unwrap();
+            return Err(close_pipe(nix::Error::EEXIST));
         }
     }
 
-    // TODO: Maybe throw an error if old action is not SigDfl.
-
     Ok(())
 }
 

src/tests.rs → tests/main/harness.rs

@@ -1,4 +1,4 @@
-// Copyright (c) 2017 CtrlC developers
+// Copyright (c) 2023 CtrlC developers
 // Licensed under the Apache License, Version 2.0
 // <LICENSE-APACHE or
 // http://www.apache.org/licenses/LICENSE-2.0> or the MIT
@@ -8,7 +8,7 @@
 // according to those terms.
 
 #[cfg(unix)]
-mod platform {
+pub mod platform {
     use std::io;
 
     pub unsafe fn setup() -> io::Result<()> {
@@ -31,7 +31,7 @@ mod platform {
 }
 
 #[cfg(windows)]
-mod platform {
+pub mod platform {
     use std::io;
     use std::ptr;
     use windows_sys::Win32::Foundation::{
@@ -216,41 +216,19 @@ mod platform {
     }
 }
 
-fn test_set_handler() {
-    let (tx, rx) = ::std::sync::mpsc::channel();
-    ctrlc::set_handler(move || {
-        tx.send(true).unwrap();
-    })
-    .unwrap();
-
-    unsafe {
-        platform::raise_ctrl_c();
-    }
-
-    rx.recv_timeout(::std::time::Duration::from_secs(10))
-        .unwrap();
-
-    match ctrlc::set_handler(|| {}) {
-        Err(ctrlc::Error::MultipleHandlers) => {}
-        ret => panic!("{:?}", ret),
-    }
-}
-
 macro_rules! run_tests {
     ( $($test_fn:ident),* ) => {
         unsafe {
-            platform::print(format_args!("\n"));
             $(
-                platform::print(format_args!("test tests::{} ... ", stringify!($test_fn)));
+                harness::platform::print(format_args!("test {} ... ", stringify!($test_fn)));
                 $test_fn();
-                platform::print(format_args!("ok\n"));
+                harness::platform::print(format_args!("ok\n"));
             )*
-            platform::print(format_args!("\n"));
         }
     }
 }
 
-fn main() {
+pub fn run_harness(f: fn()) {
     unsafe {
         platform::setup().unwrap();
     }
@@ -263,7 +241,9 @@ fn main() {
         (default)(info);
     }));
 
-    run_tests!(test_set_handler);
+    println!("");
+    f();
+    println!("");
 
     unsafe {
         platform::cleanup().unwrap();

tests/main/issue_97.rs

@@ -0,0 +1,32 @@
+// Copyright (c) 2023 CtrlC developers
+// Licensed under the Apache License, Version 2.0
+// <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT
+// license <LICENSE-MIT or http://opensource.org/licenses/MIT>,
+// at your option. All files in the project carrying such
+// notice may not be copied, modified, or distributed except
+// according to those terms.
+
+#[macro_use]
+mod harness;
+use harness::{platform, run_harness};
+
+mod test_signal_hook;
+use test_signal_hook::run_signal_hook;
+
+fn expect_multiple_handlers() {
+    #[cfg(not(windows))]
+    match ctrlc::set_handler(|| {}) {
+        Err(ctrlc::Error::MultipleHandlers) => {}
+        _ => panic!("Expected Error::MultipleHandlers"),
+    }
+}
+
+fn tests() {
+    run_tests!(run_signal_hook);
+    run_tests!(expect_multiple_handlers);
+}
+
+fn main() {
+    run_harness(tests);
+}

tests/main/mod.rs

@@ -0,0 +1,50 @@
+// Copyright (c) 2023 CtrlC developers
+// Licensed under the Apache License, Version 2.0
+// <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT
+// license <LICENSE-MIT or http://opensource.org/licenses/MIT>,
+// at your option. All files in the project carrying such
+// notice may not be copied, modified, or distributed except
+// according to those terms.
+
+#[macro_use]
+mod harness;
+use harness::{platform, run_harness};
+
+mod test_signal_hook;
+use test_signal_hook::run_signal_hook;
+
+use std::sync::{
+    atomic::{AtomicBool, Ordering},
+    Arc,
+};
+
+fn test_set_handler() {
+    let flag = Arc::new(AtomicBool::new(false));
+    let flag_handler = Arc::clone(&flag);
+    ctrlc::set_handler(move || {
+        flag_handler.store(true, Ordering::SeqCst);
+    })
+    .unwrap();
+
+    unsafe {
+        platform::raise_ctrl_c();
+    }
+
+    std::thread::sleep(std::time::Duration::from_millis(100));
+    assert!(flag.load(Ordering::SeqCst));
+
+    match ctrlc::set_handler(|| {}) {
+        Err(ctrlc::Error::MultipleHandlers) => {}
+        ret => panic!("{:?}", ret),
+    }
+}
+
+fn tests() {
+    run_tests!(test_set_handler);
+    run_tests!(run_signal_hook);
+}
+
+fn main() {
+    run_harness(tests);
+}

tests/main/test_signal_hook.rs

@@ -0,0 +1,26 @@
+// Copyright (c) 2023 CtrlC developers
+// Licensed under the Apache License, Version 2.0
+// <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT
+// license <LICENSE-MIT or http://opensource.org/licenses/MIT>,
+// at your option. All files in the project carrying such
+// notice may not be copied, modified, or distributed except
+// according to those terms.
+
+use std::sync::{
+    atomic::{AtomicBool, Ordering},
+    Arc,
+};
+
+pub fn run_signal_hook() {
+    let hook = Arc::new(AtomicBool::new(false));
+
+    signal_hook::flag::register(signal_hook::consts::SIGINT, Arc::clone(&hook)).unwrap();
+
+    unsafe {
+        super::platform::raise_ctrl_c();
+    }
+
+    std::thread::sleep(std::time::Duration::from_millis(100));
+    assert!(hook.load(Ordering::SeqCst));
+}