From 2180777f735f5ffb35027cd7511f12eef2097246 Mon Sep 17 00:00:00 2001 From: xtqqczze <45661989+xtqqczze@users.noreply.github.com> Date: Tue, 13 Jan 2026 18:45:26 +0000 Subject: [PATCH] use `nix::sys::signal::SigSet` Closes #12335 --- Cargo.toml | 3 +- src/input_common.rs | 58 ++++++++------------- src/proc.rs | 14 +++--- src/signal.rs | 12 ++--- src/threads/threads.rs | 111 ++++++++++++++++------------------------- 5 files changed, 74 insertions(+), 124 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index da77631cd..6faadea71 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,9 +41,10 @@ libc = "0.2.177" lru = "0.16.2" nix = { version = "0.30.1", default-features = false, features = [ "event", + "fs", "inotify", "resource", - "fs", + "signal", "term", ] } num-traits = "0.2.19" diff --git a/src/input_common.rs b/src/input_common.rs index 4d119d02a..c95ae9f33 100644 --- a/src/input_common.rs +++ b/src/input_common.rs @@ -19,10 +19,10 @@ use crate::universal_notifier::default_notifier; use crate::wutil::{fish_is_pua, fish_wcstol}; use fish_widestring::encode_byte_to_char; +use nix::sys::{select::FdSet, signal::SigSet, time::TimeSpec}; use std::cell::{RefCell, RefMut}; use std::collections::VecDeque; -use std::mem::MaybeUninit; -use std::os::fd::RawFd; +use std::os::fd::{BorrowedFd, RawFd}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::Duration; @@ -555,52 +555,34 @@ fn next_input_event(in_fd: RawFd, ioport_fd: RawFd, timeout: Timeout) -> InputEv } } -pub fn check_fd_readable(in_fd: RawFd, timeout: Duration) -> bool { - use std::ptr; +pub fn check_fd_readable(in_fd: BorrowedFd, timeout: Duration) -> bool { // We are not prepared to handle a signal immediately; we only want to know if we get input on // our fd before the timeout. Use pselect to block all signals; we will handle signals // before the next call to readch(). - let mut sigs = MaybeUninit::uninit(); - let mut sigs = unsafe { - libc::sigfillset(sigs.as_mut_ptr()); - sigs.assume_init() - }; - - // pselect expects timeouts in nanoseconds. - const NSEC_PER_MSEC: u64 = 1000 * 1000; - const NSEC_PER_SEC: u64 = NSEC_PER_MSEC * 1000; - let wait_nsec: u64 = (timeout.as_millis() as u64) * NSEC_PER_MSEC; - let timeout = libc::timespec { - tv_sec: (wait_nsec / NSEC_PER_SEC).try_into().unwrap(), - tv_nsec: (wait_nsec % NSEC_PER_SEC).try_into().unwrap(), - }; // We have one fd of interest. - let mut fdset = MaybeUninit::uninit(); - let mut fdset = unsafe { - libc::FD_ZERO(fdset.as_mut_ptr()); - fdset.assume_init() + let mut readfds = { + let mut set = FdSet::new(); + set.insert(in_fd); + set }; - unsafe { - libc::FD_SET(in_fd, &mut fdset); - } - let res = unsafe { - libc::pselect( - in_fd + 1, - &mut fdset, - ptr::null_mut(), - ptr::null_mut(), - &timeout, - &sigs, - ) - }; + let res = nix::sys::select::pselect( + None, + &mut readfds, + None, + None, + &TimeSpec::from_duration(timeout), + &SigSet::all(), + ) + .unwrap(); // Prevent signal starvation on WSL causing the `torn_escapes.py` test to fail if is_windows_subsystem_for_linux(WSL::V1) { // Merely querying the current thread's sigmask is sufficient to deliver a pending signal - let _ = unsafe { libc::pthread_sigmask(0, ptr::null(), &mut sigs) }; + _ = SigSet::thread_get_mask().expect("Failed to get sigmask!"); } + res > 0 } @@ -935,7 +917,7 @@ fn read_sequence_byte(&mut self, buffer: &mut Vec) -> Option { } }; if !check_fd_readable( - fd, + unsafe { BorrowedFd::borrow_raw(fd) }, if self.paste_is_buffering() || self.is_blocked_querying() { historical_millis(300) } else if buffer == b"\x1b" { @@ -1500,7 +1482,7 @@ fn readch_timed(&mut self, wait_time_ms: usize) -> Option { } check_fd_readable( - self.get_in_fd(), + unsafe { BorrowedFd::borrow_raw(self.get_in_fd()) }, Duration::from_millis(u64::try_from(wait_time_ms).unwrap()), ) .then(|| self.readch()) diff --git a/src/proc.rs b/src/proc.rs index e05b5c075..3a55b9ecb 100644 --- a/src/proc.rs +++ b/src/proc.rs @@ -25,10 +25,11 @@ use cfg_if::cfg_if; use fish_widestring::ToWString; use libc::{ - _SC_CLK_TCK, EXIT_SUCCESS, SIG_DFL, SIG_IGN, SIGABRT, SIGBUS, SIGCONT, SIGFPE, SIGHUP, SIGILL, - SIGINT, SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSYS, SIGTTOU, STDOUT_FILENO, WCONTINUED, - WEXITSTATUS, WIFCONTINUED, WIFEXITED, WIFSIGNALED, WIFSTOPPED, WNOHANG, WTERMSIG, WUNTRACED, + _SC_CLK_TCK, EXIT_SUCCESS, SIG_IGN, SIGABRT, SIGBUS, SIGCONT, SIGFPE, SIGHUP, SIGILL, SIGINT, + SIGKILL, SIGPIPE, SIGQUIT, SIGSEGV, SIGSYS, SIGTTOU, STDOUT_FILENO, WCONTINUED, WEXITSTATUS, + WIFCONTINUED, WIFEXITED, WIFSIGNALED, WIFSTOPPED, WNOHANG, WTERMSIG, WUNTRACED, }; +use nix::sys::signal::{SaFlags, SigAction, SigHandler, SigSet}; use std::cell::{Cell, Ref, RefCell, RefMut}; use std::fs; use std::io::{Read, Write}; @@ -1085,10 +1086,9 @@ fn handle_child_status(job: &Job, proc: &Process, status: ProcStatus) { job.group().cancel_with_signal(Signal::new(sig)); } else if !event::is_signal_observed(sig) { // Deliver the SIGINT or SIGQUIT signal to ourself since we're not interactive. - let mut act: libc::sigaction = unsafe { std::mem::zeroed() }; - unsafe { libc::sigemptyset(&mut act.sa_mask) }; - act.sa_flags = 0; - act.sa_sigaction = SIG_DFL; + let act = + SigAction::new(SigHandler::SigDfl, SaFlags::empty(), SigSet::empty()).into(); + unsafe { libc::sigaction(sig, &act, std::ptr::null_mut()); libc::kill(libc::getpid(), sig); diff --git a/src/signal.rs b/src/signal.rs index b0f0fabd7..b03028ce8 100644 --- a/src/signal.rs +++ b/src/signal.rs @@ -11,6 +11,7 @@ use crate::tty_handoff::{safe_deactivate_tty_protocols, safe_mark_tty_invalid}; use crate::wutil::{fish_wcstoi, perror}; use errno::{errno, set_errno}; +use nix::sys::signal::{SaFlags, SigAction, SigHandler, SigSet, SigmaskHow, sigprocmask}; use std::sync::{ LazyLock, atomic::{AtomicI32, Ordering}, @@ -127,10 +128,7 @@ extern "C" fn fish_signal_handler( /// Set all signal handlers to SIG_DFL. /// This is called after fork - it should be async signal safe. pub fn signal_reset_handlers() { - let mut act: libc::sigaction = unsafe { std::mem::zeroed() }; - unsafe { libc::sigemptyset(&mut act.sa_mask) }; - act.sa_flags = 0; - act.sa_sigaction = libc::SIG_DFL; + let act = SigAction::new(SigHandler::SigDfl, SaFlags::empty(), SigSet::empty()).into(); for data in SIGNAL_TABLE.iter() { if data.signal == libc::SIGHUP { @@ -302,11 +300,7 @@ pub fn signal_handle(sig: Signal) { /// Ensure we did not inherit any blocked signals. See issue #3964. pub fn signal_unblock_all() { - unsafe { - let mut iset = MaybeUninit::uninit(); - libc::sigemptyset(iset.as_mut_ptr()); - libc::sigprocmask(libc::SIG_SETMASK, iset.as_ptr(), std::ptr::null_mut()); - } + sigprocmask(SigmaskHow::SIG_SETMASK, Some(&SigSet::empty()), None).unwrap(); } /// A Sigchecker can be used to check if a SIGINT (or SIGHUP) has been delivered. diff --git a/src/threads/threads.rs b/src/threads/threads.rs index 3c6509458..847285cfe 100644 --- a/src/threads/threads.rs +++ b/src/threads/threads.rs @@ -1,7 +1,7 @@ //! Support for thread pools and thread management. use crate::flog::{FloggableDebug, flog}; +use nix::sys::signal::{SigSet, SigmaskHow, Signal}; use std::marker::PhantomData; -use std::mem::MaybeUninit; use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex, OnceLock}; use std::time::Duration; @@ -129,21 +129,21 @@ pub fn spawn(callback: F) -> bool { // then restore it. But we must not block SIGBUS, SIGFPE, SIGILL, or SIGSEGV; that's undefined // (#7837). Conservatively don't try to mask SIGKILL or SIGSTOP either; that's ignored on Linux // but maybe has an effect elsewhere. - let saved_set = unsafe { - let mut new_set = MaybeUninit::uninit(); - let new_set = new_set.as_mut_ptr(); - libc::sigfillset(new_set); - libc::sigdelset(new_set, libc::SIGILL); // bad jump - libc::sigdelset(new_set, libc::SIGFPE); // divide-by-zero - libc::sigdelset(new_set, libc::SIGBUS); // unaligned memory access - libc::sigdelset(new_set, libc::SIGSEGV); // bad memory access - libc::sigdelset(new_set, libc::SIGSTOP); // unblockable - libc::sigdelset(new_set, libc::SIGKILL); // unblockable + let saved_set = { + let new_set = { + let mut set = SigSet::all(); + set.remove(Signal::SIGILL); // bad jump + set.remove(Signal::SIGFPE); // divide-by-zero + set.remove(Signal::SIGBUS); // unaligned memory access + set.remove(Signal::SIGSEGV); // bad memory access + set.remove(Signal::SIGSTOP); // unblockable + set.remove(Signal::SIGKILL); // unblockable + set + }; - let mut saved_set: libc::sigset_t = std::mem::zeroed(); - let result = libc::pthread_sigmask(libc::SIG_BLOCK, new_set, &raw mut saved_set); - assert_eq!(result, 0, "Failed to override thread signal mask!"); - saved_set + new_set + .thread_swap_mask(SigmaskHow::SIG_BLOCK) + .expect("Failed to override thread signal mask!") }; // Spawn a thread. If this fails, it means there's already a bunch of threads; it is very @@ -166,14 +166,9 @@ pub fn spawn(callback: F) -> bool { }; // Restore our sigmask - unsafe { - let result = libc::pthread_sigmask( - libc::SIG_SETMASK, - &raw const saved_set, - std::ptr::null_mut(), - ); - assert_eq!(result, 0, "Failed to restore thread signal mask!"); - }; + saved_set + .thread_set_mask() + .expect("Failed to restore thread signal mask!"); result } @@ -383,9 +378,10 @@ fn dequeue_work_or_commit_to_exit(&self) -> Option { #[cfg(test)] mod tests { + use nix::sys::signal::{SigSet, SigmaskHow, Signal}; + use super::{spawn, thread_id}; - use std::mem::MaybeUninit; use std::sync::{ Arc, Condvar, Mutex, atomic::{AtomicI32, Ordering}, @@ -405,64 +401,41 @@ fn test_thread_ids() { /// sigmask to be inherited by the newly spawned thread. fn std_thread_inherits_sigmask() { // First change our own thread mask - let (saved_set, t1_set) = unsafe { - let mut new_set = MaybeUninit::uninit(); - let new_set = new_set.as_mut_ptr(); - libc::sigemptyset(new_set); - libc::sigaddset(new_set, libc::SIGILL); // mask bad jump + let (saved_set, t1_set) = { + let saved_set = { + let new_set = { + let mut set = SigSet::empty(); + set.add(Signal::SIGILL); // mask bad jump + set + }; - let mut saved_set: libc::sigset_t = std::mem::zeroed(); - let result = libc::pthread_sigmask(libc::SIG_BLOCK, new_set, &raw mut saved_set); - assert_eq!(result, 0, "Failed to set thread mask!"); + new_set + .thread_swap_mask(SigmaskHow::SIG_BLOCK) + .expect("Failed to set thread mask!") + }; // Now get the current set that includes the masked SIGILL - let mut t1_set: libc::sigset_t = std::mem::zeroed(); - let mut empty_set = MaybeUninit::uninit(); - let empty_set = empty_set.as_mut_ptr(); - libc::sigemptyset(empty_set); - let result = libc::pthread_sigmask(libc::SIG_UNBLOCK, empty_set, &raw mut t1_set); - assert_eq!(result, 0, "Failed to get own altered thread mask!"); + let t1_set = SigSet::empty() + .thread_swap_mask(SigmaskHow::SIG_UNBLOCK) + .expect("Failed to get own altered thread mask!"); (saved_set, t1_set) }; // Launch a new thread that can access existing variables let t2_set = std::thread::scope(|_| { - unsafe { - // Set a new thread sigmask and verify that the old one is what we expect it to be - let mut new_set = MaybeUninit::uninit(); - let new_set = new_set.as_mut_ptr(); - libc::sigemptyset(new_set); - let mut saved_set2: libc::sigset_t = std::mem::zeroed(); - let result = libc::pthread_sigmask(libc::SIG_BLOCK, new_set, &raw mut saved_set2); - assert_eq!(result, 0, "Failed to get existing sigmask for new thread"); - saved_set2 - } + // Set a new thread sigmask and verify that the old one is what we expect it to be + SigSet::empty() + .thread_swap_mask(SigmaskHow::SIG_BLOCK) + .expect("Failed to get existing sigmask for new thread") }); - // Compare the sigset_t values - unsafe { - let t1_sigset_slice = std::slice::from_raw_parts( - (&raw const t1_set).cast::(), - core::mem::size_of::(), - ); - let t2_sigset_slice = std::slice::from_raw_parts( - (&raw const t2_set).cast::(), - core::mem::size_of::(), - ); - - assert_eq!(t1_sigset_slice, t2_sigset_slice); - }; + assert_eq!(t1_set, t2_set); // Restore the thread sigset so we don't affect `cargo test`'s multithreaded test harnesses - unsafe { - let result = libc::pthread_sigmask( - libc::SIG_SETMASK, - &raw const saved_set, - core::ptr::null_mut(), - ); - assert_eq!(result, 0, "Failed to restore sigmask!"); - } + saved_set + .thread_set_mask() + .expect("Failed to restore sigmask!"); } #[test]