diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index c2c9c9c..7f1f11a 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -119,6 +119,7 @@ jobs: cd signal-hook-registry sed -i -e '/signal-hook =/d' Cargo.toml sed -i -e 's/libc = "^0.2"/libc = "=0.2.156"/' Cargo.toml + sed -i -e 's/errno = ">=0.2, <0.4"/errno = "0.2"/' Cargo.toml cargo check ancient: @@ -144,6 +145,7 @@ jobs: run: | rm Cargo.lock sed -i -e 's/libc = "^0.2"/libc = "=0.2.156"/' Cargo.toml + sed -i -e 's/errno = ">=0.2, <0.4"/errno = "0.2"/' signal-hook-registry/Cargo.toml cargo update cargo check --no-default-features diff --git a/Cargo.lock b/Cargo.lock index 2f89342..b761b1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -826,6 +826,7 @@ dependencies = [ name = "signal-hook-registry" version = "1.4.7" dependencies = [ + "errno", "libc", "signal-hook", ] diff --git a/ci-check.sh b/ci-check.sh index fc418c2..8faae4f 100755 --- a/ci-check.sh +++ b/ci-check.sh @@ -12,6 +12,7 @@ rm -f Cargo.lock if [ "$RUST_VERSION" = 1.36.0 ] || [ "$RUST_VERSION" = 1.40.0 ] ; then sed -i -e 's/libc = "^0.2"/libc = "=0.2.156"/' Cargo.toml sed -i -e 's/cc = { version = "^1"/cc = { version = "=1.0.79"/' Cargo.toml + sed -i -e 's/errno = ">=0.2, <0.4"/errno = "0.2"/' signal-hook-registry/Cargo.toml fi cargo build --all --exclude signal-hook-async-std --exclude signal-hook-tokio diff --git a/signal-hook-registry/Cargo.toml b/signal-hook-registry/Cargo.toml index 46919f4..cfee92e 100644 --- a/signal-hook-registry/Cargo.toml +++ b/signal-hook-registry/Cargo.toml @@ -20,6 +20,7 @@ maintenance = { status = "actively-developed" } [dependencies] libc = "^0.2" +errno = ">=0.2, <0.4" [dev-dependencies] signal-hook = { version = "~0.3", path = ".." } diff --git a/signal-hook-registry/src/lib.rs b/signal-hook-registry/src/lib.rs index 1ba8d43..e791813 100644 --- a/signal-hook-registry/src/lib.rs +++ b/signal-hook-registry/src/lib.rs @@ -62,6 +62,7 @@ //! [signal-hook]: https://docs.rs/signal-hook //! [async-signal-safe]: http://www.man7.org/linux/man-pages/man7/signal-safety.7.html +extern crate errno; extern crate libc; mod half_lock; @@ -78,6 +79,7 @@ use std::sync::atomic::{AtomicPtr, Ordering}; use std::sync::ONCE_INIT; use std::sync::{Arc, Once}; +use errno::Errno; #[cfg(not(windows))] use libc::{c_int, c_void, sigaction, siginfo_t}; #[cfg(windows)] @@ -338,6 +340,8 @@ impl GlobalData { #[cfg(windows)] extern "C" fn handler(sig: c_int) { + let _errno = ErrnoGuard::new(); + if sig != SIGFPE { // Windows CRT `signal` resets handler every time, unless for SIGFPE. // Reregister the handler to retain maximal compatibility. @@ -385,6 +389,8 @@ extern "C" fn handler(sig: c_int) { // cfg_attr is needed because the `allow(clippy::lint)` syntax was added in Rust 1.31 #[cfg_attr(clippy, allow(clippy::incompatible_msrv))] extern "C" fn handler(sig: c_int, info: *mut siginfo_t, data: *mut c_void) { + let _errno = ErrnoGuard::new(); + let globals = GlobalData::get(); let fallback = globals.race_fallback.read(); let sigdata = globals.data.read(); @@ -422,6 +428,20 @@ extern "C" fn handler(sig: c_int, info: *mut siginfo_t, data: *mut c_void) { } } +struct ErrnoGuard(Errno); + +impl ErrnoGuard { + fn new() -> Self { + ErrnoGuard(errno::errno()) + } +} + +impl Drop for ErrnoGuard { + fn drop(&mut self) { + errno::set_errno(self.0); + } +} + /// List of forbidden signals. /// /// Some signals are impossible to replace according to POSIX and some are so special that this @@ -834,4 +854,38 @@ mod tests { // The next time unregistering does nothing and tells us so. assert!(!unregister(signal)); } + + /// Check that errno is not clobbered by the signal handler. + #[test] + fn save_restore_errno() { + const MAGIC_ERRNO: i32 = 123456; + let status = Arc::new(AtomicUsize::new(0)); + let action = { + let status = Arc::clone(&status); + move || { + errno::set_errno(Errno(MAGIC_ERRNO)); + status.store(1, Ordering::Relaxed); + } + }; + unsafe { + register(SIGUSR1, action).unwrap(); + libc::raise(SIGUSR1); + } + for _ in 0..10 { + // We can't rule out that the raise() or sleep() clobbers errno, so this test isn't + // waterproof. But it fails at least sometimes on some platforms if the errno + // save/restore is removed. We assert both before and after the sleep for good luck. + assert!(errno::errno().0 != MAGIC_ERRNO); + thread::sleep(Duration::from_millis(100)); + assert!(errno::errno().0 != MAGIC_ERRNO); + let current = status.load(Ordering::Relaxed); + match current { + // Not yet + 0 => continue, + // Good, we are done with the correct result + _ if current == 1 => return, + _ => panic!("Wrong status value {}", current), + } + } + } }