From 7ce5376067cd080c2d46de9e174ab1915960f792 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Thu, 11 Jun 2026 12:45:49 +0300 Subject: [PATCH 01/11] Newtype SigalNum --- Lib/test/test_signal.py | 8 +++ crates/host_env/src/signal.rs | 11 +--- crates/vm/src/signal.rs | 100 +++++++++++++++++++++++------ crates/vm/src/stdlib/_signal.rs | 109 ++++++++++++++++---------------- crates/vm/src/stdlib/_thread.rs | 6 +- 5 files changed, 151 insertions(+), 83 deletions(-) diff --git a/Lib/test/test_signal.py b/Lib/test/test_signal.py index 07fc97cb6a1..a4af92d52eb 100644 --- a/Lib/test/test_signal.py +++ b/Lib/test/test_signal.py @@ -93,6 +93,7 @@ def test_setting_signal_handler_to_none_raises_error(self): self.assertRaises(TypeError, signal.signal, signal.SIGUSR1, None) + @unittest.expectedFailureIf(sys.platform == "android", "TODO: RUSTPYTHON; AssertionError: None is not an instance of ") def test_getsignal(self): hup = signal.signal(signal.SIGHUP, self.trivial_signal_handler) self.assertIsInstance(hup, signal.Handlers) @@ -101,6 +102,7 @@ def test_getsignal(self): signal.signal(signal.SIGHUP, hup) self.assertEqual(signal.getsignal(signal.SIGHUP), hup) + @unittest.expectedFailureIf(sys.platform == "android", "TODO: RUSTPYTHON") def test_no_repr_is_called_on_signal_handler(self): # See https://github.com/python/cpython/issues/112559. @@ -778,6 +780,7 @@ def test_siginterrupt_off(self): self.assertFalse(interrupted) +@unittest.skipIf(sys.platform == "android", "TODO: RUSTPYTHON; Error during teardown") @unittest.skipIf(sys.platform == "win32", "Not valid on Windows") @unittest.skipUnless(hasattr(signal, 'getitimer') and hasattr(signal, 'setitimer'), "needs signal.getitimer() and signal.setitimer()") @@ -1264,6 +1267,7 @@ def decide_itimer_count(self): "(> 10 ms.) on this platform (or system too busy)" % (reso,)) + @unittest.skipIf(sys.platform == "android", "TODO: RUSTPYTHON; TypeError: signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object") @unittest.skipUnless(hasattr(signal, "setitimer"), "test needs setitimer()") def test_stress_delivery_dependent(self): @@ -1310,6 +1314,7 @@ def second_handler(signum=None, frame=None): # Python handler self.assertEqual(len(sigs), N, "Some signals were lost") + @unittest.skipIf(sys.platform == "android", "TODO: RUSTPYTHON; TypeError: signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object") @unittest.skipUnless(hasattr(signal, "setitimer"), "test needs setitimer()") def test_stress_delivery_simultaneous(self): @@ -1409,6 +1414,7 @@ def cycle_handlers(): class RaiseSignalTest(unittest.TestCase): + @unittest.expectedFailureIf(sys.platform == "android", "TODO: RUSTPYTHON; AssertionError: KeyboardInterrupt not raised") def test_sigint(self): with self.assertRaises(KeyboardInterrupt): signal.raise_signal(signal.SIGINT) @@ -1425,6 +1431,7 @@ def test_invalid_argument(self): else: raise + @unittest.skipIf(sys.platform == "android", "TODO: RUSTPYTHON; TypeError: signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object") def test_handler(self): is_ok = False def handler(a, b): @@ -1455,6 +1462,7 @@ def __del__(self): class PidfdSignalTest(unittest.TestCase): + @unittest.expectedFailureIf(sys.platform == "android", "TODO: RUSTPYTHON; AssertionError: KeyboardInterrupt not raised") @unittest.skipUnless( hasattr(signal, "pidfd_send_signal"), "pidfd support not built in", diff --git a/crates/host_env/src/signal.rs b/crates/host_env/src/signal.rs index f9f2a8206b9..ab1974f37df 100644 --- a/crates/host_env/src/signal.rs +++ b/crates/host_env/src/signal.rs @@ -55,10 +55,10 @@ mod ffi { } } -#[cfg(any(unix, windows))] /// # Safety /// /// The caller must ensure `signalnum` is a valid platform signal number. +#[cfg(any(unix, windows))] pub unsafe fn probe_handler(signalnum: i32) -> Option { let handler = unsafe { libc::signal(signalnum, libc::SIG_IGN) }; if handler == libc::SIG_ERR as sighandler_t { @@ -69,11 +69,11 @@ pub unsafe fn probe_handler(signalnum: i32) -> Option { } } -#[cfg(any(unix, windows))] /// # Safety /// /// The caller must ensure `signalnum` is a valid platform signal number and /// `handler` is accepted by the platform signal ABI. +#[cfg(any(unix, windows))] pub unsafe fn install_handler(signalnum: i32, handler: sighandler_t) -> io::Result { let old = unsafe { libc::signal(signalnum, handler) }; if old == libc::SIG_ERR as sighandler_t { @@ -158,7 +158,7 @@ pub fn pthread_sigmask(how: i32, set: &libc::sigset_t) -> io::Result io::Result<()> { let ret = unsafe { libc::syscall( @@ -199,11 +199,6 @@ pub const CTRL_BREAK_EVENT: u32 = 1; #[cfg(windows)] pub const INVALID_SOCKET: libc::SOCKET = windows_sys::Win32::Networking::WinSock::INVALID_SOCKET; -#[cfg(windows)] -pub fn is_valid_signal(signalnum: i32) -> bool { - VALID_SIGNALS.contains(&signalnum) -} - #[cfg(windows)] fn init_winsock() { static WSA_INIT: Once = Once::new(); diff --git a/crates/vm/src/signal.rs b/crates/vm/src/signal.rs index 50a887c1435..388d5f79fc3 100644 --- a/crates/vm/src/signal.rs +++ b/crates/vm/src/signal.rs @@ -1,16 +1,20 @@ -use crate::{PyObjectRef, PyResult, VirtualMachine}; -use alloc::fmt; -use core::cell::{Cell, RefCell}; -use core::sync::atomic::{AtomicBool, Ordering}; +use core::{ + cell::{Cell, RefCell}, + fmt, + ops::Range, + sync::atomic::{AtomicBool, Ordering}, +}; use std::sync::mpsc; #[cfg(windows)] use core::sync::atomic::AtomicIsize; -static ANY_TRIGGERED: AtomicBool = AtomicBool::new(false); +use crate::{PyObjectRef, PyResult, TryFromBorrowedObject, TryFromObject, VirtualMachine}; pub(crate) const NSIG: usize = 64; +static ANY_TRIGGERED: AtomicBool = AtomicBool::new(false); + #[expect( clippy::declare_interior_mutable_const, reason = "workaround for const array repeat limitation (rust issue #79270)" @@ -22,16 +26,16 @@ pub(crate) static TRIGGERS: [AtomicBool; NSIG] = [ATOMIC_FALSE; NSIG]; #[cfg(windows)] static SIGINT_EVENT: AtomicIsize = AtomicIsize::new(0); -pub(crate) fn new_signal_handlers() -> Box; NSIG]>> { - Box::new(const { RefCell::new([const { None }; NSIG]) }) -} - thread_local! { /// Prevent recursive signal handler invocation. When a Python signal /// handler is running, new signals are deferred until it completes. static IN_SIGNAL_HANDLER: Cell = const { Cell::new(false) }; } +pub(crate) fn new_signal_handlers() -> Box; NSIG]>> { + Box::new(const { RefCell::new([const { None }; NSIG]) }) +} + struct SignalHandlerGuard; impl Drop for SignalHandlerGuard { @@ -110,28 +114,86 @@ pub(crate) fn clear_after_fork() { } } -pub fn assert_in_range(signum: i32, vm: &VirtualMachine) -> PyResult<()> { - if (1..NSIG as i32).contains(&signum) { - Ok(()) - } else { - Err(vm.new_value_error("signal number out of range")) +/// A valid signal number. +#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd, Ord)] +pub struct SignalNum(i32); + +impl SignalNum { + const VALID_RANGE: Range = 1..NSIG as i32; + + /// [`libc::SIGINT`] converted to [`Self`]. + pub(crate) const SIGINT: Self = Self(libc::SIGINT); + + /// Construct [`Self`] without any validation on the signalnum value. + /// + /// SAFETY: + /// Caller's responsibility to ensure the signal num is valid. + #[must_use] + pub const unsafe fn new_unchecked(value: i32) -> Self { + Self(value) + } + + /// Get the self as an [`i32`]. + #[must_use] + pub const fn as_i32(&self) -> i32 { + self.0 + } + + /// Get the self as an [`usize`]. + #[must_use] + pub const fn as_usize(&self) -> usize { + self.0 as usize + } +} + +impl fmt::Display for SignalNum { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.0, f) + } +} + +impl From for i32 { + fn from(signalnum: SignalNum) -> Self { + signalnum.as_i32() + } +} + +impl TryFrom for SignalNum { + type Error = String; + + fn try_from(value: i32) -> Result { + let bounds = cfg_select! { + all(windows, feature = "host_env") => rustpython_host_env::signal::VALID_SIGNALS, + _ => Self::VALID_RANGE, + }; + + if bounds.contains(&value) { + return Ok(Self(value)); + } else { + Err("signal number out of range".into()) + } + } +} + +impl TryFromObject for SignalNum { + fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + Self::try_from(i32::try_from_borrowed_object(vm, &obj)?) + .map_err(|msg| vm.new_value_error(msg)) } } /// Similar to `PyErr_SetInterruptEx` in CPython /// /// Missing signal handler for the given signal number is silently ignored. -#[allow(dead_code)] #[cfg(all(not(target_arch = "wasm32"), feature = "host_env"))] -pub fn set_interrupt_ex(signum: i32, vm: &VirtualMachine) -> PyResult<()> { +pub fn set_interrupt_ex(signum: SignalNum) -> PyResult<()> { use crate::stdlib::_signal::_signal::{SIG_DFL, SIG_IGN, run_signal}; - assert_in_range(signum, vm)?; - match signum as usize { + match signum.as_usize() { SIG_DFL | SIG_IGN => Ok(()), _ => { // interrupt the main thread with given signal number - run_signal(signum); + run_signal(signum.into()); Ok(()) } } diff --git a/crates/vm/src/stdlib/_signal.rs b/crates/vm/src/stdlib/_signal.rs index 191f67d090f..d6c4be98973 100644 --- a/crates/vm/src/stdlib/_signal.rs +++ b/crates/vm/src/stdlib/_signal.rs @@ -6,7 +6,10 @@ pub(crate) use _signal::module_def; pub(crate) mod _signal { #![allow(unreachable_pub)] - use crate::{Py, PyObjectRef, PyResult, VirtualMachine, signal}; + use crate::{ + Py, PyObjectRef, PyResult, VirtualMachine, + signal::{self, SignalNum}, + }; use core::{ ops::Range, sync::atomic::{self, Ordering}, @@ -91,9 +94,11 @@ pub(crate) mod _signal { #[cfg(not(unix))] #[pyattr] pub const SIG_DFL: sighandler_t = 0; + #[cfg(not(unix))] #[pyattr] pub const SIG_IGN: sighandler_t = 1; + #[cfg(not(unix))] #[allow(dead_code)] pub const SIG_ERR: sighandler_t = -1 as _; @@ -165,7 +170,8 @@ pub(crate) mod _signal { #[cfg(unix)] fn new_itimer_error(msg: &str, vm: &VirtualMachine) -> PyBaseExceptionRef { - vm.new_exception_msg(itimer_error(vm), msg.into()) + vm.new_os_subtype_error(itimer_error(vm).to_owned(), None, msg) + .upcast() } const _: () = assert!(SIGNUM_RANGE.start.is_positive()); @@ -200,7 +206,8 @@ pub(crate) mod _signal { let int_handler = module .get_attr("default_int_handler", vm) .expect("_signal does not have this attr?"); - signal(libc::SIGINT, int_handler, vm).expect("Failed to set sigint handler"); + + signal(SignalNum::SIGINT, int_handler, vm).expect("Failed to set sigint handler"); } } @@ -217,46 +224,43 @@ pub(crate) mod _signal { #[cfg(any(unix, windows))] #[pyfunction] pub fn signal( - signalnum: i32, + signalnum: SignalNum, handler: PyObjectRef, vm: &VirtualMachine, ) -> PyResult> { - signal::assert_in_range(signalnum, vm)?; - - #[cfg(windows)] - if !host_signal::is_valid_signal(signalnum) { - return Err(vm.new_value_error(format!("signal number {signalnum} out of range"))); - } - if !vm.is_main_thread() { - return Err(vm.new_value_error("signal only works in main thread")); + return Err( + vm.new_value_error("signal only works in main thread of the main interpreter") + ); } - let sig_handler = - match usize::try_from_borrowed_object(vm, &handler).ok() { - Some(SIG_DFL) => SIG_DFL, - Some(SIG_IGN) => SIG_IGN, - None if handler.is_callable() => run_signal as *const () as sighandler_t, - _ => return Err(vm.new_type_error( - "signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object", - )), - }; + let sig_handler = if handler.is_callable() { + run_signal as *const () as sighandler_t + } else { + const MSG: &str = + "signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object"; + + usize::try_from_borrowed_object(vm, &handler) + .ok() + .filter(|&v| matches!(v, SIG_DFL | SIG_IGN)) + .ok_or_else(|| vm.new_type_error(MSG))? + }; signal::check_signals(vm)?; - unsafe { host_signal::install_handler(signalnum, sig_handler) } + unsafe { host_signal::install_handler(signalnum.into(), sig_handler) } .map_err(|_| vm.new_os_error("Failed to set signal"))?; let signal_handlers = vm.signal_handlers.get_or_init(signal::new_signal_handlers); - let old_handler = signal_handlers.borrow_mut()[signalnum as usize].replace(handler); + let old_handler = signal_handlers.borrow_mut()[signalnum.as_usize()].replace(handler); + Ok(old_handler) } #[pyfunction] - fn getsignal(signalnum: i32, vm: &VirtualMachine) -> PyResult { - signal::assert_in_range(signalnum, vm)?; + fn getsignal(signalnum: SignalNum, vm: &VirtualMachine) -> PyResult { let signal_handlers = vm.signal_handlers.get_or_init(signal::new_signal_handlers); - let handler = signal_handlers.borrow()[signalnum as usize] + let handler = signal_handlers.borrow()[signalnum.as_usize()] .clone() .unwrap_or_else(|| vm.ctx.none()); Ok(handler) @@ -271,7 +275,7 @@ pub(crate) mod _signal { #[cfg(unix)] #[pyfunction] fn pause(vm: &VirtualMachine) -> PyResult<()> { - host_signal::pause(); + vm.allow_threads(|| host_signal::pause()); signal::check_signals(vm)?; Ok(()) } @@ -329,7 +333,9 @@ pub(crate) mod _signal { }; if !vm.is_main_thread() { - return Err(vm.new_value_error("set_wakeup_fd only works in main thread")); + return Err(vm.new_value_error( + "set_wakeup_fd only works in main thread of the main interpreter", + )); } #[cfg(windows)] @@ -368,16 +374,15 @@ pub(crate) mod _signal { Ok(old_fd as i64) } - #[cfg(target_os = "linux")] + #[cfg(any(target_os = "android", target_os = "linux"))] #[pyfunction] fn pidfd_send_signal( pidfd: i32, - sig: i32, + sig: SignalNum, siginfo: OptionalArg, flags: OptionalArg, vm: &VirtualMachine, ) -> PyResult<()> { - signal::assert_in_range(sig, vm)?; if let OptionalArg::Present(obj) = siginfo && !vm.is_none(&obj) { @@ -385,33 +390,35 @@ pub(crate) mod _signal { } let flags = flags.unwrap_or(0); - host_signal::pidfd_send_signal(pidfd, sig, flags).map_err(|_| vm.new_last_errno_error()) + host_signal::pidfd_send_signal(pidfd, sig.into(), flags) + .map_err(|_| vm.new_last_errno_error()) } #[cfg(all(unix, not(target_os = "redox")))] #[pyfunction(name = "siginterrupt")] - fn py_siginterrupt(signum: i32, flag: i32, vm: &VirtualMachine) -> PyResult<()> { - signal::assert_in_range(signum, vm)?; - host_signal::siginterrupt(signum, flag).map_err(|_| vm.new_last_errno_error()) + fn py_siginterrupt(signum: SignalNum, flag: i32, vm: &VirtualMachine) -> PyResult<()> { + host_signal::siginterrupt(signum.into(), flag).map_err(|_| vm.new_last_errno_error()) } #[cfg(any(unix, windows))] #[pyfunction] fn raise_signal(signalnum: i32, vm: &VirtualMachine) -> PyResult<()> { - signal::assert_in_range(signalnum, vm)?; - - // On Windows, only certain signals are supported - #[cfg(windows)] - if !host_signal::is_valid_signal(signalnum) { - return Err(vm - .new_errno_error(libc::EINVAL, "Invalid argument") - .upcast()); - } + let signalnum = SignalNum::try_from(signalnum).map_err(|msg| { + cfg_select! { + windows => { + vm.new_errno_error(libc::EINVAL, "Invalid argument").upcast() + }, + _ => vm.new_value_error(msg) + } + })?; - host_signal::raise_signal(signalnum) - .map_err(|_| vm.new_os_error(format!("raise_signal failed for signal {signalnum}")))?; + vm.allow_threads(|| { + host_signal::raise_signal(signalnum.into()) + .map_err(|_| vm.new_os_error(format!("raise_signal failed for signal {signalnum}"))) + })?; // Check if a signal was triggered and handle it + signal::check_signals(vm)?; Ok(()) @@ -419,12 +426,8 @@ pub(crate) mod _signal { #[cfg(any(unix, windows))] #[pyfunction] - fn strsignal(signalnum: i32, vm: &VirtualMachine) -> PyResult> { - if !SIGNUM_RANGE.contains(&signalnum) { - return Err(vm.new_value_error(format!("signal number {signalnum} out of range"))); - } - - Ok(host_signal::strsignal(signalnum)) + fn strsignal(signalnum: SignalNum) -> PyResult> { + Ok(host_signal::strsignal(signalnum.into())) } #[pyfunction] @@ -464,8 +467,6 @@ pub(crate) mod _signal { mask: crate::function::ArgIterable, vm: &VirtualMachine, ) -> PyResult { - use crate::convert::IntoPyException; - // Initialize sigset let mut sigset = host_signal::sigemptyset().map_err(|e| e.into_pyexception(vm))?; diff --git a/crates/vm/src/stdlib/_thread.rs b/crates/vm/src/stdlib/_thread.rs index 93d42676b4e..834c9c52f39 100644 --- a/crates/vm/src/stdlib/_thread.rs +++ b/crates/vm/src/stdlib/_thread.rs @@ -19,6 +19,7 @@ pub(crate) mod _thread { common::wtf8::Wtf8Buf, frame::FrameRef, function::{ArgCallable, FuncArgs, KwArgs, OptionalArg, PySetterValue, TimeoutSeconds}, + signal::SignalNum, types::{Constructor, GetAttr, Representable, SetAttr}, }; use alloc::{ @@ -614,8 +615,9 @@ pub(crate) mod _thread { #[cfg(all(not(target_arch = "wasm32"), feature = "host_env"))] #[pyfunction] - fn interrupt_main(signum: OptionalArg, vm: &VirtualMachine) -> PyResult<()> { - crate::signal::set_interrupt_ex(signum.unwrap_or(libc::SIGINT), vm) + fn interrupt_main(signum: OptionalArg) -> PyResult<()> { + let sig = signum.unwrap_or(SignalNum::SIGINT); + crate::signal::set_interrupt_ex(sig) } #[pyfunction] From 86383c66528e15c062768db2517066e2fbad2a4b Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sun, 14 Jun 2026 09:20:00 +0300 Subject: [PATCH 02/11] Improved doc --- crates/vm/src/signal.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/vm/src/signal.rs b/crates/vm/src/signal.rs index c367b0b3d87..3a7849b4a34 100644 --- a/crates/vm/src/signal.rs +++ b/crates/vm/src/signal.rs @@ -121,7 +121,11 @@ pub struct SignalNum(i32); impl SignalNum { const VALID_RANGE: Range = 1..NSIG as i32; - /// [`libc::SIGINT`] converted to [`Self`]. + /// Alias for: + /// ```rust + /// unsafe { SignalNum::new_unchecked(libc::SIGINT) } + /// ``` + #[cfg(any(unix, windows))] pub(crate) const SIGINT: Self = Self(libc::SIGINT); /// Construct [`Self`] without any validation on the signalnum value. From 19fbd50968da72643478495b2dd992525dbfde97 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sun, 14 Jun 2026 09:57:39 +0300 Subject: [PATCH 03/11] impl Index for SignalHandlersInner --- crates/vm/src/signal.rs | 41 +++++++++++++++++++++++++-------- crates/vm/src/stdlib/_signal.rs | 11 +++++---- 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/crates/vm/src/signal.rs b/crates/vm/src/signal.rs index 3a7849b4a34..12a40370a90 100644 --- a/crates/vm/src/signal.rs +++ b/crates/vm/src/signal.rs @@ -1,7 +1,7 @@ use core::{ cell::{Cell, RefCell}, fmt, - ops::{Deref, DerefMut, Range}, + ops::{Deref, DerefMut, Index, IndexMut, Range}, sync::atomic::{AtomicBool, Ordering}, }; use std::sync::mpsc; @@ -32,10 +32,6 @@ thread_local! { static IN_SIGNAL_HANDLER: Cell = const { Cell::new(false) }; } -pub(crate) fn new_signal_handlers() -> Box; NSIG]>> { - Box::new(const { RefCell::new([const { None }; NSIG]) }) -} - struct SignalHandlerGuard; impl Drop for SignalHandlerGuard { @@ -79,11 +75,16 @@ fn trigger_signals(vm: &VirtualMachine) -> PyResult<()> { let signal_handlers = vm.signal_handlers.get().unwrap().borrow(); for (signum, trigger) in TRIGGERS.iter().enumerate().skip(1) { let triggered = trigger.swap(false, Ordering::Relaxed); + + let signum = (signum as i32) + .try_into() + .expect("TRIGGERS has the same length as the signal_handlers"); + if triggered && let Some(handler) = &signal_handlers[signum] && let Some(callable) = handler.to_callable() { - callable.invoke((signum, vm.ctx.none()), vm)?; + callable.invoke((signum.as_i32(), vm.ctx.none()), vm)?; } } if let Some(signal_rx) = &vm.signal_rx { @@ -257,16 +258,38 @@ pub fn get_sigint_event() -> Option { if handle == 0 { None } else { Some(handle) } } -pub struct SignalHandlers(Box; NSIG]>>); +pub struct SignalHandlersInner([Option; NSIG]); + +impl Default for SignalHandlersInner { + fn default() -> Self { + Self([const { None }; NSIG]) + } +} + +impl Index for SignalHandlersInner { + type Output = Option; + + fn index(&self, index: SignalNum) -> &Self::Output { + &self.0[index.as_usize()] + } +} + +impl IndexMut for SignalHandlersInner { + fn index_mut(&mut self, index: SignalNum) -> &mut Self::Output { + &mut self.0[index.as_usize()] + } +} + +pub struct SignalHandlers(Box>); impl Default for SignalHandlers { fn default() -> Self { - Self(Box::new(const { RefCell::new([const { None }; NSIG]) })) + Self(Box::new(RefCell::new(SignalHandlersInner::default()))) } } impl Deref for SignalHandlers { - type Target = Box; NSIG]>>; + type Target = Box>; fn deref(&self) -> &Self::Target { &self.0 diff --git a/crates/vm/src/stdlib/_signal.rs b/crates/vm/src/stdlib/_signal.rs index 0a8bf2c6006..0316ce5ab3b 100644 --- a/crates/vm/src/stdlib/_signal.rs +++ b/crates/vm/src/stdlib/_signal.rs @@ -8,7 +8,7 @@ pub(crate) mod _signal { use crate::{ Py, PyObjectRef, PyResult, VirtualMachine, - signal::{self, SignalNum}, + signal::{self, SignalHandlers, SignalNum}, }; use core::{ ops::Range, @@ -198,9 +198,12 @@ pub(crate) mod _signal { None }; + // SAFETY: Trust `SIGNUM_RANGE` + let signum = unsafe { SignalNum::new_unchecked(signum) }; + vm.signal_handlers .get_or_init(SignalHandlers::default) - .borrow_mut()[signum as usize] = py_handler; + .borrow_mut()[signum] = py_handler; } let int_handler = module @@ -252,14 +255,14 @@ pub(crate) mod _signal { .map_err(|_| vm.new_os_error("Failed to set signal"))?; let signal_handlers = vm.signal_handlers.get_or_init(SignalHandlers::default); - let old_handler = signal_handlers.borrow_mut()[signalnum as usize].replace(handler); + let old_handler = signal_handlers.borrow_mut()[signalnum].replace(handler); Ok(old_handler) } #[pyfunction] fn getsignal(signalnum: SignalNum, vm: &VirtualMachine) -> PyResult { let signal_handlers = vm.signal_handlers.get_or_init(SignalHandlers::default); - let handler = signal_handlers.borrow()[signalnum.as_usize()] + let handler = signal_handlers.borrow()[signalnum] .clone() .unwrap_or_else(|| vm.ctx.none()); Ok(handler) From d2a73313e1a40727b1193de51efda11e5303b20b Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sun, 14 Jun 2026 10:20:47 +0300 Subject: [PATCH 04/11] clippy --- crates/vm/src/signal.rs | 7 +++++-- crates/vm/src/stdlib/_signal.rs | 15 +++++++-------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/crates/vm/src/signal.rs b/crates/vm/src/signal.rs index 12a40370a90..126f50c5126 100644 --- a/crates/vm/src/signal.rs +++ b/crates/vm/src/signal.rs @@ -124,6 +124,8 @@ impl SignalNum { /// Alias for: /// ```rust + /// # use rustpython_vm::signal::SigalNum; + /// /// unsafe { SignalNum::new_unchecked(libc::SIGINT) } /// ``` #[cfg(any(unix, windows))] @@ -131,7 +133,8 @@ impl SignalNum { /// Construct [`Self`] without any validation on the signalnum value. /// - /// SAFETY: + /// # Safety + /// /// Caller's responsibility to ensure the signal num is valid. #[must_use] pub const unsafe fn new_unchecked(value: i32) -> Self { @@ -173,7 +176,7 @@ impl TryFrom for SignalNum { }; if bounds.contains(&value) { - return Ok(Self(value)); + Ok(Self(value)) } else { Err("signal number out of range".into()) } diff --git a/crates/vm/src/stdlib/_signal.rs b/crates/vm/src/stdlib/_signal.rs index 0316ce5ab3b..d30b1c9a94a 100644 --- a/crates/vm/src/stdlib/_signal.rs +++ b/crates/vm/src/stdlib/_signal.rs @@ -170,7 +170,7 @@ pub(crate) mod _signal { #[cfg(unix)] fn new_itimer_error(msg: &str, vm: &VirtualMachine) -> PyBaseExceptionRef { - vm.new_os_subtype_error(itimer_error(vm).to_owned(), None, msg) + vm.new_os_subtype_error(itimer_error(vm), None, msg) .upcast() } @@ -260,12 +260,11 @@ pub(crate) mod _signal { } #[pyfunction] - fn getsignal(signalnum: SignalNum, vm: &VirtualMachine) -> PyResult { + fn getsignal(signalnum: SignalNum, vm: &VirtualMachine) -> PyObjectRef { let signal_handlers = vm.signal_handlers.get_or_init(SignalHandlers::default); - let handler = signal_handlers.borrow()[signalnum] + signal_handlers.borrow()[signalnum] .clone() - .unwrap_or_else(|| vm.ctx.none()); - Ok(handler) + .unwrap_or_else(|| vm.ctx.none()) } #[cfg(unix)] @@ -277,7 +276,7 @@ pub(crate) mod _signal { #[cfg(unix)] #[pyfunction] fn pause(vm: &VirtualMachine) -> PyResult<()> { - vm.allow_threads(|| host_signal::pause()); + vm.allow_threads(host_signal::pause); signal::check_signals(vm)?; Ok(()) } @@ -428,8 +427,8 @@ pub(crate) mod _signal { #[cfg(any(unix, windows))] #[pyfunction] - fn strsignal(signalnum: SignalNum) -> PyResult> { - Ok(host_signal::strsignal(signalnum.into())) + fn strsignal(signalnum: SignalNum) -> Option { + host_signal::strsignal(signalnum.into()) } #[pyfunction] From 7689418ac028ed0749e9c71e92274c504687df1c Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sun, 14 Jun 2026 10:28:05 +0300 Subject: [PATCH 05/11] Allow dead code --- crates/vm/src/signal.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/vm/src/signal.rs b/crates/vm/src/signal.rs index 126f50c5126..4cbb40e3c61 100644 --- a/crates/vm/src/signal.rs +++ b/crates/vm/src/signal.rs @@ -129,6 +129,7 @@ impl SignalNum { /// unsafe { SignalNum::new_unchecked(libc::SIGINT) } /// ``` #[cfg(any(unix, windows))] + #![allow(dead_code, reason="Not used on all platforms")] pub(crate) const SIGINT: Self = Self(libc::SIGINT); /// Construct [`Self`] without any validation on the signalnum value. From eef6f26f26e1609a9ee41425ba3a99479658c557 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sun, 14 Jun 2026 11:28:44 +0300 Subject: [PATCH 06/11] fix lint --- crates/vm/src/signal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vm/src/signal.rs b/crates/vm/src/signal.rs index 4cbb40e3c61..5ce99744dc1 100644 --- a/crates/vm/src/signal.rs +++ b/crates/vm/src/signal.rs @@ -129,7 +129,7 @@ impl SignalNum { /// unsafe { SignalNum::new_unchecked(libc::SIGINT) } /// ``` #[cfg(any(unix, windows))] - #![allow(dead_code, reason="Not used on all platforms")] + #[allow(dead_code, reason = "Not used on all platforms")] pub(crate) const SIGINT: Self = Self(libc::SIGINT); /// Construct [`Self`] without any validation on the signalnum value. From b9ed125fe2fe1ead1f51587ca56d26d2c49f3651 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sun, 14 Jun 2026 12:23:54 +0300 Subject: [PATCH 07/11] fix typo --- crates/vm/src/signal.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vm/src/signal.rs b/crates/vm/src/signal.rs index 5ce99744dc1..1a5a1a9ca99 100644 --- a/crates/vm/src/signal.rs +++ b/crates/vm/src/signal.rs @@ -124,7 +124,7 @@ impl SignalNum { /// Alias for: /// ```rust - /// # use rustpython_vm::signal::SigalNum; + /// # use rustpython_vm::signal::SignalNum; /// /// unsafe { SignalNum::new_unchecked(libc::SIGINT) } /// ``` From 52a1efdaf3dc4047869ad1a569eff4e87260efc9 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sun, 14 Jun 2026 13:14:45 +0300 Subject: [PATCH 08/11] More fixes --- crates/vm/src/signal.rs | 4 ++-- crates/vm/src/stdlib/_signal.rs | 28 +++++++++++++--------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/crates/vm/src/signal.rs b/crates/vm/src/signal.rs index 1a5a1a9ca99..aae19e5c933 100644 --- a/crates/vm/src/signal.rs +++ b/crates/vm/src/signal.rs @@ -120,13 +120,13 @@ pub(crate) fn clear_after_fork() { pub struct SignalNum(i32); impl SignalNum { - const VALID_RANGE: Range = 1..NSIG as i32; + pub(crate) const VALID_RANGE: Range = 1..NSIG as i32; /// Alias for: /// ```rust /// # use rustpython_vm::signal::SignalNum; /// - /// unsafe { SignalNum::new_unchecked(libc::SIGINT) } + /// unsafe { SignalNum::new_unchecked(libc::SIGINT) }; /// ``` #[cfg(any(unix, windows))] #[allow(dead_code, reason = "Not used on all platforms")] diff --git a/crates/vm/src/stdlib/_signal.rs b/crates/vm/src/stdlib/_signal.rs index d30b1c9a94a..b7fbf9a04a7 100644 --- a/crates/vm/src/stdlib/_signal.rs +++ b/crates/vm/src/stdlib/_signal.rs @@ -42,8 +42,6 @@ pub(crate) mod _signal { _ => usize, }; - const SIGNUM_RANGE: Range = 1..signal::NSIG as i32; - cfg_select! { windows => { type WakeupFdRaw = libc::SOCKET; @@ -174,8 +172,8 @@ pub(crate) mod _signal { .upcast() } - const _: () = assert!(SIGNUM_RANGE.start.is_positive()); - const _: () = assert!(SIGNUM_RANGE.end.is_positive()); + const _: () = assert!(SignalNum::VALID_RANGE.start.is_positive()); + const _: () = assert!(SignalNum::VALID_RANGE.end.is_positive()); #[cfg(any(unix, windows))] pub(super) fn init_signal_handlers( @@ -186,7 +184,7 @@ pub(crate) mod _signal { let sig_dfl = vm.new_pyobj(SIG_DFL as u8); let sig_ign = vm.new_pyobj(SIG_IGN as u8); - for signum in SIGNUM_RANGE { + for signum in SignalNum::VALID_RANGE { let Some(handler) = (unsafe { host_signal::probe_handler(signum) }) else { continue; }; @@ -198,7 +196,7 @@ pub(crate) mod _signal { None }; - // SAFETY: Trust `SIGNUM_RANGE` + // SAFETY: Trust `SignalNum::VALID_RANGE` let signum = unsafe { SignalNum::new_unchecked(signum) }; vm.signal_handlers @@ -404,12 +402,12 @@ pub(crate) mod _signal { #[cfg(any(unix, windows))] #[pyfunction] fn raise_signal(signalnum: i32, vm: &VirtualMachine) -> PyResult<()> { - let signalnum = SignalNum::try_from(signalnum).map_err(|msg| { - cfg_select! { - windows => { - vm.new_errno_error(libc::EINVAL, "Invalid argument").upcast() - }, - _ => vm.new_value_error(msg) + let signalnum = SignalNum::try_from(signalnum).map_err(cfg_select! { + windows => { + |_| vm.new_errno_error(libc::EINVAL, "Invalid argument").upcast() + }, + _ => { + |msg| vm.new_value_error(msg) } })?; @@ -453,7 +451,7 @@ pub(crate) mod _signal { use crate::PyPayload; use crate::builtins::PySet; let set = PySet::default().into_ref(&vm.ctx); - for signum in SIGNUM_RANGE { + for signum in SignalNum::VALID_RANGE { if host_signal::sigset_contains(mask, signum) { set.add(vm.ctx.new_int(signum).into(), vm)?; } @@ -480,11 +478,11 @@ pub(crate) mod _signal { let signum = sig .try_to_value::(vm) .ok() - .filter(|v| SIGNUM_RANGE.contains(v)) + .filter(|v| SignalNum::VALID_RANGE.contains(v)) .ok_or_else(|| { vm.new_value_error(format!( "signal number out of range [1, {}]", - SIGNUM_RANGE.end - 1 + SignalNum::VALID_RANGE.end - 1 )) })?; From 68f8a3a289856ffd009e237bc8ebc9fa41e305fd Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sun, 14 Jun 2026 13:52:53 +0300 Subject: [PATCH 09/11] fix --- crates/vm/src/stdlib/_signal.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/vm/src/stdlib/_signal.rs b/crates/vm/src/stdlib/_signal.rs index b7fbf9a04a7..38137af1192 100644 --- a/crates/vm/src/stdlib/_signal.rs +++ b/crates/vm/src/stdlib/_signal.rs @@ -10,10 +10,7 @@ pub(crate) mod _signal { Py, PyObjectRef, PyResult, VirtualMachine, signal::{self, SignalHandlers, SignalNum}, }; - use core::{ - ops::Range, - sync::atomic::{self, Ordering}, - }; + use core::sync::atomic::{self, Ordering}; cfg_select! { any(unix, windows) => { From a7a355a778433144ffdc01175dd1c1728a5e472b Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sun, 14 Jun 2026 14:45:00 +0300 Subject: [PATCH 10/11] Fix for windows --- crates/vm/src/signal.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/crates/vm/src/signal.rs b/crates/vm/src/signal.rs index aae19e5c933..eea42f4a87e 100644 --- a/crates/vm/src/signal.rs +++ b/crates/vm/src/signal.rs @@ -71,14 +71,17 @@ fn trigger_signals(vm: &VirtualMachine) -> PyResult<()> { } let _guard = SignalHandlerGuard; - // unwrap should never fail since we check above - let signal_handlers = vm.signal_handlers.get().unwrap().borrow(); + let signal_handlers = vm + .signal_handlers + .get() + .expect("should never fail since we check above") + .borrow(); + for (signum, trigger) in TRIGGERS.iter().enumerate().skip(1) { let triggered = trigger.swap(false, Ordering::Relaxed); - let signum = (signum as i32) - .try_into() - .expect("TRIGGERS has the same length as the signal_handlers"); + // SAFETY: TRIGGERS has the same length as the signal_handlers + let signum = unsafe { SignalNum::new_unchecked(signum as i32) }; if triggered && let Some(handler) = &signal_handlers[signum] @@ -87,11 +90,13 @@ fn trigger_signals(vm: &VirtualMachine) -> PyResult<()> { callable.invoke((signum.as_i32(), vm.ctx.none()), vm)?; } } + if let Some(signal_rx) = &vm.signal_rx { for f in signal_rx.rx.try_iter() { f(vm)?; } } + Ok(()) } From bbf319b13d99a9817024b05b49b63d7796a46949 Mon Sep 17 00:00:00 2001 From: ShaharNaveh <50263213+ShaharNaveh@users.noreply.github.com> Date: Sun, 14 Jun 2026 18:54:02 +0300 Subject: [PATCH 11/11] coderabbit suggestions --- crates/vm/src/stdlib/_signal.rs | 6 ++---- crates/vm/src/stdlib/_thread.rs | 35 +++++++++++++++++---------------- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/crates/vm/src/stdlib/_signal.rs b/crates/vm/src/stdlib/_signal.rs index 38137af1192..e3d12568d26 100644 --- a/crates/vm/src/stdlib/_signal.rs +++ b/crates/vm/src/stdlib/_signal.rs @@ -408,10 +408,8 @@ pub(crate) mod _signal { } })?; - vm.allow_threads(|| { - host_signal::raise_signal(signalnum.into()) - .map_err(|_| vm.new_os_error(format!("raise_signal failed for signal {signalnum}"))) - })?; + vm.allow_threads(|| host_signal::raise_signal(signalnum.into())) + .map_err(|_| vm.new_os_error(format!("raise_signal failed for signal {signalnum}")))?; // Check if a signal was triggered and handle it diff --git a/crates/vm/src/stdlib/_thread.rs b/crates/vm/src/stdlib/_thread.rs index 834c9c52f39..ae612e4d539 100644 --- a/crates/vm/src/stdlib/_thread.rs +++ b/crates/vm/src/stdlib/_thread.rs @@ -13,28 +13,30 @@ pub(crate) use _thread::{ #[pymodule] pub(crate) mod _thread { + use parking_lot::{ + RawMutex, RawThreadId, + lock_api::{RawMutex as RawMutexT, RawMutexTimed, RawReentrantMutex}, + }; + use crate::{ AsObject, Py, PyPayload, PyRef, PyResult, VirtualMachine, builtins::{PyDictRef, PyIntRef, PyStr, PyTupleRef, PyType, PyTypeRef, PyUtf8StrRef}, common::wtf8::Wtf8Buf, frame::FrameRef, function::{ArgCallable, FuncArgs, KwArgs, OptionalArg, PySetterValue, TimeoutSeconds}, - signal::SignalNum, types::{Constructor, GetAttr, Representable, SetAttr}, }; - use alloc::{ - fmt, - sync::{Arc, Weak}, - }; - use core::{cell::RefCell, time::Duration}; - use parking_lot::{ - RawMutex, RawThreadId, - lock_api::{RawMutex as RawMutexT, RawMutexTimed, RawReentrantMutex}, - }; + + use alloc::sync::{Arc, Weak}; + use core::{cell::RefCell, fmt, time::Duration}; + use std::thread; + use rustpython_common::str::levenshtein::{MOVE_COST, levenshtein_distance}; #[cfg(any(unix, windows))] use rustpython_host_env::thread as host_thread; - use std::thread; + + #[cfg(all(not(target_arch = "wasm32"), feature = "host_env"))] + use crate::signal::SignalNum; // PYTHREAD_NAME: show current thread name pub(crate) const PYTHREAD_NAME: Option<&str> = cfg_select! { @@ -44,12 +46,11 @@ pub(crate) mod _thread { _ => None, }; - // TIMEOUT_MAX_IN_MICROSECONDS is a value in microseconds - #[cfg(not(target_os = "windows"))] - const TIMEOUT_MAX_IN_MICROSECONDS: i64 = i64::MAX / 1_000; - - #[cfg(target_os = "windows")] - const TIMEOUT_MAX_IN_MICROSECONDS: i64 = 0xffffffff * 1_000; + const TIMEOUT_MAX_IN_MICROSECONDS: i64 = if cfg!(target_os = "windows") { + 0xffffffff * 1_000 + } else { + i64::MAX / 1_000 + }; /// [CPython `SYSTEM_PAGE_SIZE`](https://github.com/python/cpython/blob/v3.14.5/Include/internal/pycore_obmalloc.h#L170) const SYSTEM_PAGE_SIZE: usize = 4 * 1024;