fixup new unsafe interrupt handler

Author: Byron

gix/examples/clone.rs

@@ -11,7 +11,10 @@ fn main() -> anyhow::Result<()> {
         .nth(2)
         .context("The second argument is the directory to clone the repository into")?;
 
-    gix::interrupt::init_handler(1, || {})?;
+    // SAFETY: The closure doesn't use mutexes or memory allocation, so it should be safe to call from a signal handler.
+    unsafe {
+        gix::interrupt::init_handler(1, || {})?;
+    }
     std::fs::create_dir_all(&dst)?;
     let url = gix::url::parse(repo_url.to_str().unwrap().into())?;
 

gix/examples/interrupt-handler-allows-graceful-shutdown.rs

@@ -6,7 +6,10 @@ fn main() -> anyhow::Result<()> {
 #[cfg(feature = "interrupt")]
 fn main() -> anyhow::Result<()> {
     use gix_tempfile::{AutoRemove, ContainingDirectory};
-    gix::interrupt::init_handler(1, || {})?;
+    // SAFETY: The closure doesn't use mutexes or memory allocation, so it should be safe to call from a signal handler.
+    unsafe {
+        gix::interrupt::init_handler(1, || {})?;
+    }
     eprintln!("About to emit the first term signal");
     let tempfile_path = std::path::Path::new("example-file.tmp");
     let _keep_tempfile = gix_tempfile::mark_at(tempfile_path, ContainingDirectory::Exists, AutoRemove::Tempfile)?;

gix/examples/reversible-interrupt-handlers.rs

@@ -6,7 +6,8 @@ fn main() -> anyhow::Result<()> {
 #[cfg(feature = "interrupt")]
 fn main() -> anyhow::Result<()> {
     {
-        let _deregister_on_drop = gix::interrupt::init_handler(1, || {})?.auto_deregister();
+        // SAFETY: The closure doesn't use mutexes or memory allocation, so it should be safe to call from a signal handler.
+        let _deregister_on_drop = unsafe { gix::interrupt::init_handler(1, || {})?.auto_deregister() };
     }
     eprintln!("About to emit the first term signal, which acts just like a normal one");
     signal_hook::low_level::raise(signal_hook::consts::SIGTERM)?;

gix/src/interrupt.rs

@@ -90,8 +90,8 @@ mod init {
     /// It will abort the process on second press and won't inform the user about this behaviour either as we are unable to do so without
     /// deadlocking even when trying to write to stderr directly.
     ///
-    /// SAFETY: `interrupt()` will be called from a signal handler. See `signal_hook::low_level::register()` for details about.
-    #[allow(unsafe_code)]
+    /// SAFETY: `interrupt()` will be called from a signal handler. See [`signal_hook::low_level::register()`] for details about.
+    #[allow(unsafe_code, clippy::missing_safety_doc)]
     pub unsafe fn init_handler(
         grace_count: usize,
         interrupt: impl Fn() + Send + Sync + Clone + 'static,

gix/tests/interrupt.rs

@@ -9,14 +9,18 @@ mod needs_feature {
         static V1: AtomicUsize = AtomicUsize::new(0);
         static V2: AtomicBool = AtomicBool::new(false);
 
-        let reg1 = gix::interrupt::init_handler(3, || {
-            V1.fetch_add(1, Ordering::SeqCst);
-        })
+        // SAFETY: The closure doesn't use mutexes or memory allocation, so it should be safe to call from a signal handler.
+        let reg1 = unsafe {
+            gix::interrupt::init_handler(3, || {
+                V1.fetch_add(1, Ordering::SeqCst);
+            })
+        }
         .expect("succeeds");
         assert!(!gix::interrupt::is_triggered());
         assert_eq!(V1.load(Ordering::Relaxed), 0);
-        let reg2 =
-            gix::interrupt::init_handler(2, || V2.store(true, Ordering::SeqCst)).expect("multi-initialization is OK");
+        // SAFETY: The closure doesn't use mutexes or memory allocation, so it should be safe to call from a signal handler.
+        let reg2 = unsafe { gix::interrupt::init_handler(2, || V2.store(true, Ordering::SeqCst)) }
+            .expect("multi-initialization is OK");
         assert!(!V2.load(Ordering::Relaxed));
 
         signal_hook::low_level::raise(SIGTERM).expect("signal can be raised");
@@ -36,13 +40,17 @@ mod needs_feature {
             "the deregistration succeeded and this is an optional side-effect"
         );
 
-        let reg1 = gix::interrupt::init_handler(3, || {
-            V1.fetch_add(1, Ordering::SeqCst);
-        })
+        // SAFETY: The closure doesn't use mutexes or memory allocation, so it should be safe to call from a signal handler.
+        let reg1 = unsafe {
+            gix::interrupt::init_handler(3, || {
+                V1.fetch_add(1, Ordering::SeqCst);
+            })
+        }
         .expect("succeeds");
         assert_eq!(V1.load(Ordering::Relaxed), 2, "nothing changed yet");
-        let reg2 =
-            gix::interrupt::init_handler(2, || V2.store(true, Ordering::SeqCst)).expect("multi-initialization is OK");
+        // SAFETY: The closure doesn't use mutexes or memory allocation, so it should be safe to call from a signal handler.
+        let reg2 = unsafe { gix::interrupt::init_handler(2, || V2.store(true, Ordering::SeqCst)) }
+            .expect("multi-initialization is OK");
         assert!(!V2.load(Ordering::Relaxed));
 
         signal_hook::low_level::raise(SIGTERM).expect("signal can be raised");

tests/snapshots/panic-behaviour/expected-failure

@@ -1,3 +1,3 @@
-thread 'main' panicked at src/porcelain/main.rs:41:42:
+thread 'main' panicked at src/porcelain/main.rs:45:42:
 something went very wrong
 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

tests/snapshots/panic-behaviour/expected-failure-in-thread

@@ -1,4 +1,4 @@
-thread 'main' panicked at src/porcelain/main.rs:41:42:
+thread 'main' panicked at src/porcelain/main.rs:45:42:
 something went very wrong
 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 

tests/snapshots/panic-behaviour/expected-failure-in-thread-with-progress

@@ -1,4 +1,4 @@
-[?1049h[?25lthread '<unnamed>' panicked at src/porcelain/main.rs:41:42:
+[?1049h[?25lthread '<unnamed>' panicked at src/porcelain/main.rs:45:42:
 something went very wrong
 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 [?25h[?1049l