From b376dea5cddf0767d9d2fe8c6ab61923fc269779 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 11 Dec 2025 20:12:41 +0900 Subject: [PATCH 1/7] impl faulthandler --- Lib/test/test_faulthandler.py | 4 ---- crates/stdlib/src/faulthandler.rs | 17 +++++++++++++++-- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_faulthandler.py b/Lib/test/test_faulthandler.py index 6316f937e2..2e36db99b9 100644 --- a/Lib/test/test_faulthandler.py +++ b/Lib/test/test_faulthandler.py @@ -411,8 +411,6 @@ def test_dump_ext_modules(self): for name in ('sys', 'faulthandler'): self.assertIn(name, modules) - # TODO: RUSTPYTHON, AttributeError: module 'faulthandler' has no attribute 'is_enabled' - @unittest.expectedFailure def test_is_enabled(self): orig_stderr = sys.stderr try: @@ -435,8 +433,6 @@ def test_is_enabled(self): finally: sys.stderr = orig_stderr - # TODO: RUSTPYTHON, subprocess.CalledProcessError: Command ... returned non-zero exit status 1. - @unittest.expectedFailure @support.requires_subprocess() def test_disabled_by_default(self): # By default, the module should be disabled diff --git a/crates/stdlib/src/faulthandler.rs b/crates/stdlib/src/faulthandler.rs index 5c9196ad33..b98ba723d0 100644 --- a/crates/stdlib/src/faulthandler.rs +++ b/crates/stdlib/src/faulthandler.rs @@ -3,6 +3,9 @@ pub(crate) use decl::make_module; #[pymodule(name = "faulthandler")] mod decl { use crate::vm::{VirtualMachine, frame::Frame, function::OptionalArg, stdlib::sys::PyStderr}; + use std::sync::atomic::{AtomicBool, Ordering}; + + static ENABLED: AtomicBool = AtomicBool::new(false); fn dump_frame(frame: &Frame, vm: &VirtualMachine) { let stderr = PyStderr(vm); @@ -39,8 +42,18 @@ mod decl { } #[pyfunction] - const fn enable(_args: EnableArgs) { - // TODO + fn enable(_args: EnableArgs) { + ENABLED.store(true, Ordering::Relaxed); + } + + #[pyfunction] + fn disable() { + ENABLED.store(false, Ordering::Relaxed); + } + + #[pyfunction] + fn is_enabled() -> bool { + ENABLED.load(Ordering::Relaxed) } #[derive(FromArgs)] From f9f8427d9ec0be07235b997bedb3f1a7dfb7dd93 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 11 Dec 2025 20:31:45 +0900 Subject: [PATCH 2/7] faulthandler --- Lib/test/test_faulthandler.py | 17 +- crates/stdlib/src/faulthandler.rs | 321 +++++++++++++++++++++++++++--- 2 files changed, 298 insertions(+), 40 deletions(-) diff --git a/Lib/test/test_faulthandler.py b/Lib/test/test_faulthandler.py index 2e36db99b9..ffdc82bf05 100644 --- a/Lib/test/test_faulthandler.py +++ b/Lib/test/test_faulthandler.py @@ -524,12 +524,10 @@ def funcA(): self.assertEqual(trace, expected) self.assertEqual(exitcode, 0) - # TODO: RUSTPYTHON, AssertionError: Lists differ - @unittest.expectedFailure def test_dump_traceback(self): self.check_dump_traceback() - # TODO: RUSTPYTHON + # TODO: RUSTPYTHON - binary file write needs different handling @unittest.expectedFailure def test_dump_traceback_file(self): with temporary_filename() as filename: @@ -543,8 +541,6 @@ def test_dump_traceback_fd(self): with tempfile.TemporaryFile('wb+') as fp: self.check_dump_traceback(fd=fp.fileno()) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_truncate(self): maxlen = 500 func_name = 'x' * (maxlen + 50) @@ -866,8 +862,6 @@ def check_stderr_none(self): finally: sys.stderr = stderr - # TODO: RUSTPYTHON, AssertionError: RuntimeError not raised - @unittest.expectedFailure def test_stderr_None(self): # Issue #21497: provide a helpful error if sys.stderr is None, # instead of just an attribute error: "None has no attribute fileno". @@ -898,8 +892,6 @@ def test_raise_exception(self): 3, name) - # TODO: RUSTPYTHON, AttributeError: module 'msvcrt' has no attribute 'GetErrorMode' - @unittest.expectedFailure @unittest.skipUnless(MS_WINDOWS, 'specific to Windows') def test_ignore_exception(self): for exc_code in ( @@ -916,8 +908,6 @@ def test_ignore_exception(self): self.assertEqual(output, []) self.assertEqual(exitcode, exc_code) - # TODO: RUSTPYTHON, AttributeError: module 'msvcrt' has no attribute 'GetErrorMode' - @unittest.expectedFailure @unittest.skipUnless(MS_WINDOWS, 'specific to Windows') def test_raise_nonfatal_exception(self): # These exceptions are not strictly errors. Letting @@ -946,8 +936,6 @@ def test_raise_nonfatal_exception(self): self.assertIn(exitcode, (exc, exc & ~0x10000000)) - # TODO: RUSTPYTHON, AttributeError: module 'msvcrt' has no attribute 'GetErrorMode' - @unittest.expectedFailure @unittest.skipUnless(MS_WINDOWS, 'specific to Windows') def test_disable_windows_exc_handler(self): code = dedent(""" @@ -961,8 +949,6 @@ def test_disable_windows_exc_handler(self): self.assertEqual(output, []) self.assertEqual(exitcode, 0xC0000005) - # TODO: RUSTPYTHON, AssertionError: Lists differ - @unittest.expectedFailure def test_cancel_later_without_dump_traceback_later(self): # bpo-37933: Calling cancel_dump_traceback_later() # without dump_traceback_later() must not segfault. @@ -974,7 +960,6 @@ def test_cancel_later_without_dump_traceback_later(self): self.assertEqual(output, []) self.assertEqual(exitcode, 0) - @unittest.expectedFailureIfWindows("TODO: RUSTPYTHON; AttributeError: module 'msvcrt' has no attribute 'GetErrorMode'") @threading_helper.requires_working_threading() @unittest.skipUnless(support.Py_GIL_DISABLED, "only meaningful if the GIL is disabled") def test_free_threaded_dump_traceback(self): diff --git a/crates/stdlib/src/faulthandler.rs b/crates/stdlib/src/faulthandler.rs index b98ba723d0..cfe5640ff4 100644 --- a/crates/stdlib/src/faulthandler.rs +++ b/crates/stdlib/src/faulthandler.rs @@ -2,53 +2,130 @@ pub(crate) use decl::make_module; #[pymodule(name = "faulthandler")] mod decl { - use crate::vm::{VirtualMachine, frame::Frame, function::OptionalArg, stdlib::sys::PyStderr}; - use std::sync::atomic::{AtomicBool, Ordering}; + use crate::vm::{ + PyObjectRef, PyResult, VirtualMachine, frame::Frame, function::OptionalArg, py_io::Write, + }; + use std::sync::atomic::{AtomicBool, AtomicI32, Ordering}; static ENABLED: AtomicBool = AtomicBool::new(false); + #[allow(dead_code)] + static FATAL_ERROR_FD: AtomicI32 = AtomicI32::new(2); // stderr by default - fn dump_frame(frame: &Frame, vm: &VirtualMachine) { - let stderr = PyStderr(vm); - writeln!( - stderr, + const MAX_FUNCTION_NAME_LEN: usize = 500; + + fn truncate_name(name: &str) -> String { + if name.len() > MAX_FUNCTION_NAME_LEN { + format!("{}...", &name[..MAX_FUNCTION_NAME_LEN]) + } else { + name.to_string() + } + } + + fn get_file_for_output( + file: OptionalArg, + vm: &VirtualMachine, + ) -> PyResult { + match file { + OptionalArg::Present(f) => { + // If it's an integer, we can't use it directly as a file object + // For now, just return it and let the caller handle it + Ok(f) + } + OptionalArg::Missing => { + // Get sys.stderr + let stderr = vm.sys_module.get_attr("stderr", vm)?; + if vm.is_none(&stderr) { + return Err(vm.new_runtime_error("sys.stderr is None".to_owned())); + } + Ok(stderr) + } + } + } + + fn collect_frame_info(frame: &crate::vm::PyRef) -> String { + let func_name = truncate_name(frame.code.obj_name.as_str()); + format!( " File \"{}\", line {} in {}", frame.code.source_path, frame.current_location().line, - frame.code.obj_name + func_name ) } + #[derive(FromArgs)] + struct DumpTracebackArgs { + #[pyarg(any, default)] + file: OptionalArg, + #[pyarg(any, default = true)] + all_threads: bool, + } + #[pyfunction] - fn dump_traceback( - _file: OptionalArg, - _all_threads: OptionalArg, - vm: &VirtualMachine, - ) { - let stderr = PyStderr(vm); - writeln!(stderr, "Stack (most recent call first):"); + fn dump_traceback(args: DumpTracebackArgs, vm: &VirtualMachine) -> PyResult<()> { + let _ = args.all_threads; // TODO: implement all_threads support - for frame in vm.frames.borrow().iter() { - dump_frame(frame, vm); + let file = get_file_for_output(args.file, vm)?; + + // Collect frame info first to avoid RefCell borrow conflict + let frame_lines: Vec = vm.frames.borrow().iter().map(collect_frame_info).collect(); + + // Now write to file (in reverse order - most recent call first) + let mut writer = crate::vm::py_io::PyWriter(file, vm); + writeln!(writer, "Stack (most recent call first):")?; + for line in frame_lines.iter().rev() { + writeln!(writer, "{}", line)?; } + Ok(()) } #[derive(FromArgs)] #[allow(unused)] struct EnableArgs { #[pyarg(any, default)] - file: Option, + file: OptionalArg, #[pyarg(any, default = true)] all_threads: bool, } #[pyfunction] - fn enable(_args: EnableArgs) { + fn enable(args: EnableArgs, vm: &VirtualMachine) -> PyResult<()> { + // Check that file is valid (if provided) or sys.stderr is not None + let _file = get_file_for_output(args.file, vm)?; + ENABLED.store(true, Ordering::Relaxed); + + // Install signal handlers for fatal errors + #[cfg(not(target_arch = "wasm32"))] + { + install_fatal_handlers(vm); + } + + Ok(()) + } + + #[cfg(not(target_arch = "wasm32"))] + fn install_fatal_handlers(_vm: &VirtualMachine) { + // TODO: Install actual signal handlers for SIGSEGV, SIGFPE, etc. + // This requires careful handling because signal handlers have limited capabilities. + // For now, this is a placeholder that marks the module as enabled. } #[pyfunction] - fn disable() { - ENABLED.store(false, Ordering::Relaxed); + fn disable() -> bool { + let was_enabled = ENABLED.swap(false, Ordering::Relaxed); + + // Restore default signal handlers + #[cfg(not(target_arch = "wasm32"))] + { + uninstall_fatal_handlers(); + } + + was_enabled + } + + #[cfg(not(target_arch = "wasm32"))] + fn uninstall_fatal_handlers() { + // TODO: Restore original signal handlers } #[pyfunction] @@ -56,21 +133,217 @@ mod decl { ENABLED.load(Ordering::Relaxed) } + #[derive(FromArgs)] + #[allow(unused)] + struct DumpTracebackLaterArgs { + #[pyarg(positional)] + timeout: f64, + #[pyarg(any, default = false)] + repeat: bool, + #[pyarg(any, default)] + file: OptionalArg, + #[pyarg(any, default = false)] + exit: bool, + } + + #[pyfunction] + fn dump_traceback_later(args: DumpTracebackLaterArgs, vm: &VirtualMachine) -> PyResult<()> { + // Check that file is valid + let _file = get_file_for_output(args.file, vm)?; + + // TODO: Implement watchdog thread that dumps traceback after timeout + // For now, this is a stub + Err(vm.new_not_implemented_error("dump_traceback_later is not yet implemented")) + } + + #[pyfunction] + fn cancel_dump_traceback_later() { + // TODO: Cancel the watchdog thread + // For now, this is a no-op since dump_traceback_later is not implemented + } + + #[cfg(unix)] #[derive(FromArgs)] #[allow(unused)] struct RegisterArgs { #[pyarg(positional)] - signum: i64, + signum: i32, #[pyarg(any, default)] - file: Option, + file: OptionalArg, #[pyarg(any, default = true)] all_threads: bool, #[pyarg(any, default = false)] chain: bool, } + #[cfg(unix)] + #[pyfunction] + fn register(args: RegisterArgs, vm: &VirtualMachine) -> PyResult<()> { + // Check that file is valid + let _file = get_file_for_output(args.file, vm)?; + + // TODO: Register a handler for the given signal + Err(vm.new_not_implemented_error("register is not yet fully implemented")) + } + + #[cfg(unix)] + #[pyfunction] + fn unregister(signum: i32, vm: &VirtualMachine) -> PyResult { + let _ = signum; + // TODO: Unregister the handler for the given signal + Err(vm.new_not_implemented_error("unregister is not yet fully implemented")) + } + + // Test functions for faulthandler testing + + #[pyfunction] + fn _read_null() { + // This function intentionally causes a segmentation fault by reading from NULL + // Used for testing faulthandler + #[cfg(not(target_arch = "wasm32"))] + unsafe { + suppress_crash_report(); + let ptr: *const i32 = std::ptr::null(); + std::ptr::read_volatile(ptr); + } + } + + #[derive(FromArgs)] + #[allow(dead_code)] + struct SigsegvArgs { + #[pyarg(any, default = false)] + release_gil: bool, + } + + #[pyfunction] + fn _sigsegv(_args: SigsegvArgs) { + // Raise SIGSEGV signal + #[cfg(not(target_arch = "wasm32"))] + { + suppress_crash_report(); + + #[cfg(windows)] + { + // On Windows, we need to raise SIGSEGV multiple times + loop { + unsafe { + libc::raise(libc::SIGSEGV); + } + } + } + #[cfg(not(windows))] + unsafe { + libc::raise(libc::SIGSEGV); + } + } + } + + #[pyfunction] + fn _sigabrt() { + #[cfg(not(target_arch = "wasm32"))] + { + suppress_crash_report(); + unsafe { + libc::abort(); + } + } + } + + #[pyfunction] + fn _sigfpe() { + #[cfg(not(target_arch = "wasm32"))] + { + suppress_crash_report(); + // Raise SIGFPE + unsafe { + libc::raise(libc::SIGFPE); + } + } + } + #[pyfunction] - const fn register(_args: RegisterArgs) { - // TODO + fn _fatal_error_c_thread() { + // This would call Py_FatalError in a new C thread + // For RustPython, we just panic in a new thread + #[cfg(not(target_arch = "wasm32"))] + { + suppress_crash_report(); + std::thread::spawn(|| { + panic!("Fatal Python error: in new thread"); + }); + // Wait a bit for the thread to panic + std::thread::sleep(std::time::Duration::from_secs(1)); + } + } + + #[cfg(not(target_arch = "wasm32"))] + fn suppress_crash_report() { + #[cfg(windows)] + { + use windows_sys::Win32::System::Diagnostics::Debug::{ + SEM_NOGPFAULTERRORBOX, SetErrorMode, + }; + unsafe { + let mode = SetErrorMode(SEM_NOGPFAULTERRORBOX); + SetErrorMode(mode | SEM_NOGPFAULTERRORBOX); + } + } + + #[cfg(unix)] + { + // Disable core dumps + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + { + use libc::{RLIMIT_CORE, rlimit, setrlimit}; + let rl = rlimit { + rlim_cur: 0, + rlim_max: 0, + }; + unsafe { + let _ = setrlimit(RLIMIT_CORE, &rl); + } + } + } + } + + // Windows-specific constants + #[cfg(windows)] + #[pyattr] + const _EXCEPTION_ACCESS_VIOLATION: u32 = 0xC0000005; + + #[cfg(windows)] + #[pyattr] + const _EXCEPTION_INT_DIVIDE_BY_ZERO: u32 = 0xC0000094; + + #[cfg(windows)] + #[pyattr] + const _EXCEPTION_STACK_OVERFLOW: u32 = 0xC00000FD; + + #[cfg(windows)] + #[pyattr] + const _EXCEPTION_NONCONTINUABLE: u32 = 0x00000001; + + #[cfg(windows)] + #[pyattr] + const _EXCEPTION_NONCONTINUABLE_EXCEPTION: u32 = 0xC0000025; + + #[cfg(windows)] + #[derive(FromArgs)] + struct RaiseExceptionArgs { + #[pyarg(positional)] + code: u32, + #[pyarg(positional, default = 0)] + flags: u32, + } + + #[cfg(windows)] + #[pyfunction] + fn _raise_exception(args: RaiseExceptionArgs) { + use windows_sys::Win32::System::Diagnostics::Debug::RaiseException; + + suppress_crash_report(); + unsafe { + RaiseException(args.code, args.flags, 0, std::ptr::null()); + } } } From 7ee01187f3a08d1276602a71ca1dad2af1963e49 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 11 Dec 2025 20:48:41 +0900 Subject: [PATCH 3/7] string --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 976d0bc0a3..0737ebdddc 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -207,7 +207,7 @@ fn run_rustpython(vm: &VirtualMachine, run_mode: RunMode) -> PyResult<()> { let res = match run_mode { RunMode::Command(command) => { debug!("Running command {command}"); - vm.run_code_string(scope.clone(), &command, "".to_owned()) + vm.run_code_string(scope.clone(), &command, "".to_owned()) .map(drop) } RunMode::Module(module) => { From 0a2b2baa8ede6f8417a8c7eb850d0809890bfbcc Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 11 Dec 2025 21:12:09 +0900 Subject: [PATCH 4/7] fix faulthandler on macos --- Lib/test/test_subprocess.py | 2 - crates/stdlib/src/faulthandler.rs | 237 +++++++++++++++++++++++++++++- 2 files changed, 230 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 29ed4639c4..8756223026 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1833,8 +1833,6 @@ def test_run_with_shell_timeout_and_capture_output(self): msg="TimeoutExpired was delayed! Bad traceback:\n```\n" f"{stacks}```") - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_encoding_warning(self): code = textwrap.dedent("""\ from subprocess import * diff --git a/crates/stdlib/src/faulthandler.rs b/crates/stdlib/src/faulthandler.rs index cfe5640ff4..421add0968 100644 --- a/crates/stdlib/src/faulthandler.rs +++ b/crates/stdlib/src/faulthandler.rs @@ -11,6 +11,16 @@ mod decl { #[allow(dead_code)] static FATAL_ERROR_FD: AtomicI32 = AtomicI32::new(2); // stderr by default + // Fatal signal numbers that should use enable() instead + #[cfg(unix)] + const FATAL_SIGNALS: &[i32] = &[ + libc::SIGBUS, + libc::SIGILL, + libc::SIGFPE, + libc::SIGABRT, + libc::SIGSEGV, + ]; + const MAX_FUNCTION_NAME_LEN: usize = 500; fn truncate_name(name: &str) -> String { @@ -162,6 +172,166 @@ mod decl { // For now, this is a no-op since dump_traceback_later is not implemented } + #[cfg(unix)] + mod user_signals { + use std::sync::Mutex; + + const NSIG: usize = 64; + + #[derive(Clone)] + pub struct UserSignal { + pub enabled: bool, + pub fd: i32, + #[allow(dead_code)] + pub all_threads: bool, + pub chain: bool, + pub previous: libc::sighandler_t, + } + + impl Default for UserSignal { + fn default() -> Self { + Self { + enabled: false, + fd: 2, // stderr + all_threads: true, + chain: false, + previous: libc::SIG_DFL, + } + } + } + + static USER_SIGNALS: Mutex>> = Mutex::new(None); + + pub fn get_user_signal(signum: usize) -> Option { + let guard = USER_SIGNALS.lock().unwrap(); + guard.as_ref().and_then(|v| v.get(signum).cloned()) + } + + pub fn set_user_signal(signum: usize, signal: UserSignal) { + let mut guard = USER_SIGNALS.lock().unwrap(); + if guard.is_none() { + *guard = Some(vec![UserSignal::default(); NSIG]); + } + if let Some(ref mut v) = *guard + && signum < v.len() + { + v[signum] = signal; + } + } + + pub fn clear_user_signal(signum: usize) -> Option { + let mut guard = USER_SIGNALS.lock().unwrap(); + if let Some(ref mut v) = *guard + && signum < v.len() + && v[signum].enabled + { + let old = v[signum].clone(); + v[signum] = UserSignal::default(); + return Some(old); + } + None + } + + pub fn is_enabled(signum: usize) -> bool { + let guard = USER_SIGNALS.lock().unwrap(); + guard + .as_ref() + .and_then(|v| v.get(signum)) + .is_some_and(|s| s.enabled) + } + } + + #[cfg(unix)] + extern "C" fn faulthandler_user_signal(signum: libc::c_int) { + let user = match user_signals::get_user_signal(signum as usize) { + Some(u) if u.enabled => u, + _ => return, + }; + + // Write traceback header + let header = b"Current thread 0x0000 (most recent call first):\n"; + let _ = unsafe { + libc::write( + user.fd, + header.as_ptr() as *const libc::c_void, + header.len(), + ) + }; + + // Note: We cannot easily access RustPython's frame stack from a signal handler + // because signal handlers run asynchronously. We just output a placeholder. + let msg = b" \n"; + let _ = unsafe { libc::write(user.fd, msg.as_ptr() as *const libc::c_void, msg.len()) }; + + // If chain is enabled, call the previous handler + if user.chain && user.previous != libc::SIG_DFL && user.previous != libc::SIG_IGN { + // Re-register the old handler and raise the signal + unsafe { + libc::signal(signum, user.previous); + libc::raise(signum); + // Re-register our handler + libc::signal(signum, faulthandler_user_signal as libc::sighandler_t); + } + } + } + + #[cfg(unix)] + fn check_signum(signum: i32, vm: &VirtualMachine) -> PyResult<()> { + // Check if it's a fatal signal + if FATAL_SIGNALS.contains(&signum) { + return Err(vm.new_runtime_error(format!( + "signal {} cannot be registered, use enable() instead", + signum + ))); + } + + // Check if signal is in valid range + if !(1..64).contains(&signum) { + return Err(vm.new_value_error("signal number out of range".to_owned())); + } + + Ok(()) + } + + #[cfg(unix)] + fn get_fd_from_file(file: OptionalArg, vm: &VirtualMachine) -> PyResult { + match file { + OptionalArg::Present(f) => { + // Check if it's an integer (file descriptor) + if let Ok(fd) = f.try_to_value::(vm) { + if fd < 0 { + return Err( + vm.new_value_error("file is not a valid file descriptor".to_owned()) + ); + } + return Ok(fd); + } + // Try to get fileno() from file object + let fileno = vm.call_method(&f, "fileno", ())?; + let fd: i32 = fileno.try_to_value(vm)?; + if fd < 0 { + return Err( + vm.new_value_error("file is not a valid file descriptor".to_owned()) + ); + } + // Try to flush the file + let _ = vm.call_method(&f, "flush", ()); + Ok(fd) + } + OptionalArg::Missing => { + // Get sys.stderr + let stderr = vm.sys_module.get_attr("stderr", vm)?; + if vm.is_none(&stderr) { + return Err(vm.new_runtime_error("sys.stderr is None".to_owned())); + } + let fileno = vm.call_method(&stderr, "fileno", ())?; + let fd: i32 = fileno.try_to_value(vm)?; + let _ = vm.call_method(&stderr, "flush", ()); + Ok(fd) + } + } + } + #[cfg(unix)] #[derive(FromArgs)] #[allow(unused)] @@ -179,19 +349,60 @@ mod decl { #[cfg(unix)] #[pyfunction] fn register(args: RegisterArgs, vm: &VirtualMachine) -> PyResult<()> { - // Check that file is valid - let _file = get_file_for_output(args.file, vm)?; + check_signum(args.signum, vm)?; - // TODO: Register a handler for the given signal - Err(vm.new_not_implemented_error("register is not yet fully implemented")) + let fd = get_fd_from_file(args.file, vm)?; + + let signum = args.signum as usize; + + // Get current handler to save as previous + let previous = if !user_signals::is_enabled(signum) { + // Install signal handler + let prev = unsafe { + libc::signal(args.signum, faulthandler_user_signal as libc::sighandler_t) + }; + if prev == libc::SIG_ERR { + return Err(vm.new_os_error(format!( + "Failed to register signal handler for signal {}", + args.signum + ))); + } + prev + } else { + // Already registered, keep previous handler + user_signals::get_user_signal(signum) + .map(|u| u.previous) + .unwrap_or(libc::SIG_DFL) + }; + + user_signals::set_user_signal( + signum, + user_signals::UserSignal { + enabled: true, + fd, + all_threads: args.all_threads, + chain: args.chain, + previous, + }, + ); + + Ok(()) } #[cfg(unix)] #[pyfunction] fn unregister(signum: i32, vm: &VirtualMachine) -> PyResult { - let _ = signum; - // TODO: Unregister the handler for the given signal - Err(vm.new_not_implemented_error("unregister is not yet fully implemented")) + check_signum(signum, vm)?; + + if let Some(old) = user_signals::clear_user_signal(signum as usize) { + // Restore previous handler + unsafe { + libc::signal(signum, old.previous); + } + Ok(true) + } else { + Ok(false) + } } // Test functions for faulthandler testing @@ -222,6 +433,12 @@ mod decl { { suppress_crash_report(); + // Reset SIGSEGV to default behavior before raising + // This ensures the process will actually crash + unsafe { + libc::signal(libc::SIGSEGV, libc::SIG_DFL); + } + #[cfg(windows)] { // On Windows, we need to raise SIGSEGV multiple times @@ -254,6 +471,12 @@ mod decl { #[cfg(not(target_arch = "wasm32"))] { suppress_crash_report(); + + // Reset SIGFPE to default behavior before raising + unsafe { + libc::signal(libc::SIGFPE, libc::SIG_DFL); + } + // Raise SIGFPE unsafe { libc::raise(libc::SIGFPE); From 97943dce3e13b4af1c0c398fe93f7cc6f1a6e5f2 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 11 Dec 2025 21:48:51 +0900 Subject: [PATCH 5/7] fix watchdog --- crates/stdlib/src/faulthandler.rs | 213 ++++++++++++++++++++++++++++-- 1 file changed, 204 insertions(+), 9 deletions(-) diff --git a/crates/stdlib/src/faulthandler.rs b/crates/stdlib/src/faulthandler.rs index 421add0968..687b3a1a26 100644 --- a/crates/stdlib/src/faulthandler.rs +++ b/crates/stdlib/src/faulthandler.rs @@ -3,14 +3,31 @@ pub(crate) use decl::make_module; #[pymodule(name = "faulthandler")] mod decl { use crate::vm::{ - PyObjectRef, PyResult, VirtualMachine, frame::Frame, function::OptionalArg, py_io::Write, + PyObjectRef, PyResult, VirtualMachine, builtins::PyFloat, frame::Frame, + function::OptionalArg, py_io::Write, }; use std::sync::atomic::{AtomicBool, AtomicI32, Ordering}; + use std::sync::{Arc, Condvar, Mutex}; + use std::thread; + use std::time::Duration; static ENABLED: AtomicBool = AtomicBool::new(false); #[allow(dead_code)] static FATAL_ERROR_FD: AtomicI32 = AtomicI32::new(2); // stderr by default + // Watchdog thread state for dump_traceback_later + struct WatchdogState { + cancel: bool, + fd: i32, + timeout_us: u64, + repeat: bool, + exit: bool, + header: String, + } + + type WatchdogHandle = Arc<(Mutex, Condvar)>; + static WATCHDOG: Mutex> = Mutex::new(None); + // Fatal signal numbers that should use enable() instead #[cfg(unix)] const FATAL_SIGNALS: &[i32] = &[ @@ -143,11 +160,129 @@ mod decl { ENABLED.load(Ordering::Relaxed) } + fn format_timeout(timeout_us: u64) -> String { + let sec = timeout_us / 1_000_000; + let us = timeout_us % 1_000_000; + let min = sec / 60; + let sec = sec % 60; + let hour = min / 60; + let min = min % 60; + + if us != 0 { + format!("Timeout ({:02}:{:02}:{:02}.{:06})!\n", hour, min, sec, us) + } else { + format!("Timeout ({:02}:{:02}:{:02})!\n", hour, min, sec) + } + } + + fn get_fd_from_file_opt(file: OptionalArg, vm: &VirtualMachine) -> PyResult { + match file { + OptionalArg::Present(f) => { + // Check if it's an integer (file descriptor) + if let Ok(fd) = f.try_to_value::(vm) { + if fd < 0 { + return Err( + vm.new_value_error("file is not a valid file descriptor".to_owned()) + ); + } + return Ok(fd); + } + // Try to get fileno() from file object + let fileno = vm.call_method(&f, "fileno", ())?; + let fd: i32 = fileno.try_to_value(vm)?; + if fd < 0 { + return Err( + vm.new_value_error("file is not a valid file descriptor".to_owned()) + ); + } + // Try to flush the file + let _ = vm.call_method(&f, "flush", ()); + Ok(fd) + } + OptionalArg::Missing => { + // Get sys.stderr + let stderr = vm.sys_module.get_attr("stderr", vm)?; + if vm.is_none(&stderr) { + return Err(vm.new_runtime_error("sys.stderr is None".to_owned())); + } + let fileno = vm.call_method(&stderr, "fileno", ())?; + let fd: i32 = fileno.try_to_value(vm)?; + let _ = vm.call_method(&stderr, "flush", ()); + Ok(fd) + } + } + } + + fn watchdog_thread(state: WatchdogHandle) { + let (lock, cvar) = &*state; + + loop { + let (timeout, repeat, exit, fd, header, cancelled) = { + let guard = lock.lock().unwrap(); + if guard.cancel { + return; + } + ( + Duration::from_micros(guard.timeout_us), + guard.repeat, + guard.exit, + guard.fd, + guard.header.clone(), + guard.cancel, + ) + }; + + if cancelled { + return; + } + + // Wait for timeout or cancellation + let result = { + let guard = lock.lock().unwrap(); + cvar.wait_timeout(guard, timeout).unwrap() + }; + + // Check if cancelled + if result.0.cancel { + return; + } + + // Timeout occurred, dump traceback + #[cfg(not(target_arch = "wasm32"))] + { + let header_bytes = header.as_bytes(); + unsafe { + libc::write( + fd, + header_bytes.as_ptr() as *const libc::c_void, + header_bytes.len(), + ); + } + + // Note: We cannot dump actual Python traceback from a separate thread + // because we don't have access to the VM's frame stack. + // Just output a message indicating timeout occurred. + let msg = b"\n"; + unsafe { + libc::write(fd, msg.as_ptr() as *const libc::c_void, msg.len()); + } + + if exit { + std::process::exit(1); + } + } + + if !repeat { + return; + } + } + } + #[derive(FromArgs)] #[allow(unused)] struct DumpTracebackLaterArgs { #[pyarg(positional)] - timeout: f64, + timeout: PyObjectRef, #[pyarg(any, default = false)] repeat: bool, #[pyarg(any, default)] @@ -158,18 +293,78 @@ mod decl { #[pyfunction] fn dump_traceback_later(args: DumpTracebackLaterArgs, vm: &VirtualMachine) -> PyResult<()> { - // Check that file is valid - let _file = get_file_for_output(args.file, vm)?; + use num_traits::ToPrimitive; + // Convert timeout to f64 (accepting int or float) + let timeout: f64 = if let Some(float) = args.timeout.downcast_ref::() { + float.to_f64() + } else if let Some(int) = args.timeout.try_index_opt(vm).transpose()? { + int.as_bigint() + .to_i64() + .ok_or_else(|| vm.new_overflow_error("timeout value is too large".to_owned()))? + as f64 + } else { + return Err(vm.new_type_error("timeout must be a number (int or float)".to_owned())); + }; + + if timeout <= 0.0 { + return Err(vm.new_value_error("timeout must be greater than 0".to_owned())); + } + + let fd = get_fd_from_file_opt(args.file, vm)?; + + // Convert timeout to microseconds + let timeout_us = (timeout * 1_000_000.0) as u64; + if timeout_us == 0 { + return Err(vm.new_value_error("timeout must be greater than 0".to_owned())); + } + + let header = format_timeout(timeout_us); + + // Cancel any previous watchdog + cancel_dump_traceback_later(); - // TODO: Implement watchdog thread that dumps traceback after timeout - // For now, this is a stub - Err(vm.new_not_implemented_error("dump_traceback_later is not yet implemented")) + // Create new watchdog state + let state = Arc::new(( + Mutex::new(WatchdogState { + cancel: false, + fd, + timeout_us, + repeat: args.repeat, + exit: args.exit, + header, + }), + Condvar::new(), + )); + + // Store the state + { + let mut watchdog = WATCHDOG.lock().unwrap(); + *watchdog = Some(Arc::clone(&state)); + } + + // Start watchdog thread + thread::spawn(move || { + watchdog_thread(state); + }); + + Ok(()) } #[pyfunction] fn cancel_dump_traceback_later() { - // TODO: Cancel the watchdog thread - // For now, this is a no-op since dump_traceback_later is not implemented + let state = { + let mut watchdog = WATCHDOG.lock().unwrap(); + watchdog.take() + }; + + if let Some(state) = state { + let (lock, cvar) = &*state; + { + let mut guard = lock.lock().unwrap(); + guard.cancel = true; + } + cvar.notify_all(); + } } #[cfg(unix)] From 8a9096ad957a75b037ae4b8a0190a68082424197 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 11 Dec 2025 21:51:28 +0900 Subject: [PATCH 6/7] install handler --- crates/stdlib/src/faulthandler.rs | 198 +++++++++++++++++++++++++++--- 1 file changed, 181 insertions(+), 17 deletions(-) diff --git a/crates/stdlib/src/faulthandler.rs b/crates/stdlib/src/faulthandler.rs index 687b3a1a26..00f64f8f6c 100644 --- a/crates/stdlib/src/faulthandler.rs +++ b/crates/stdlib/src/faulthandler.rs @@ -12,7 +12,6 @@ mod decl { use std::time::Duration; static ENABLED: AtomicBool = AtomicBool::new(false); - #[allow(dead_code)] static FATAL_ERROR_FD: AtomicI32 = AtomicI32::new(2); // stderr by default // Watchdog thread state for dump_traceback_later @@ -28,16 +27,57 @@ mod decl { type WatchdogHandle = Arc<(Mutex, Condvar)>; static WATCHDOG: Mutex> = Mutex::new(None); - // Fatal signal numbers that should use enable() instead + // Number of fatal signals we handle + #[cfg(unix)] + const NUM_FATAL_SIGNALS: usize = 5; + #[cfg(windows)] + const NUM_FATAL_SIGNALS: usize = 3; + + // Fatal signals to handle (with names for error messages) #[cfg(unix)] - const FATAL_SIGNALS: &[i32] = &[ - libc::SIGBUS, - libc::SIGILL, - libc::SIGFPE, - libc::SIGABRT, - libc::SIGSEGV, + const FATAL_SIGNALS: [(libc::c_int, &str); NUM_FATAL_SIGNALS] = [ + (libc::SIGBUS, "Bus error"), + (libc::SIGILL, "Illegal instruction"), + (libc::SIGFPE, "Floating-point exception"), + (libc::SIGABRT, "Aborted"), + (libc::SIGSEGV, "Segmentation fault"), + ]; + + #[cfg(windows)] + const FATAL_SIGNALS: [(libc::c_int, &str); NUM_FATAL_SIGNALS] = [ + (libc::SIGFPE, "Floating-point exception"), + (libc::SIGABRT, "Aborted"), + (libc::SIGSEGV, "Segmentation fault"), ]; + // Storage for previous signal handlers (Unix) + #[cfg(unix)] + static mut PREVIOUS_HANDLERS: [libc::sigaction; NUM_FATAL_SIGNALS] = + unsafe { std::mem::zeroed() }; + + /// Signal-safe write function - no memory allocation + #[cfg(all(not(target_arch = "wasm32"), not(windows)))] + fn write_str_noraise(fd: i32, s: &str) { + unsafe { + libc::write( + fd as libc::c_int, + s.as_ptr() as *const libc::c_void, + s.len(), + ); + } + } + + #[cfg(windows)] + fn write_str_noraise(fd: i32, s: &str) { + unsafe { + libc::write( + fd as libc::c_int, + s.as_ptr() as *const libc::c_void, + s.len() as u32, + ); + } + } + const MAX_FUNCTION_NAME_LEN: usize = 500; fn truncate_name(name: &str) -> String { @@ -122,7 +162,7 @@ mod decl { ENABLED.store(true, Ordering::Relaxed); // Install signal handlers for fatal errors - #[cfg(not(target_arch = "wasm32"))] + #[cfg(any(unix, windows))] { install_fatal_handlers(vm); } @@ -130,11 +170,108 @@ mod decl { Ok(()) } - #[cfg(not(target_arch = "wasm32"))] + /// Unix signal handler for fatal errors + #[cfg(unix)] + extern "C" fn faulthandler_fatal_error(signum: libc::c_int) { + // Reentrant protection + static REENTRANT: AtomicBool = AtomicBool::new(false); + if REENTRANT.swap(true, Ordering::SeqCst) { + return; + } + + let fd = FATAL_ERROR_FD.load(Ordering::Relaxed); + + // Find and print signal name + let signal_name = FATAL_SIGNALS + .iter() + .find(|(sig, _)| *sig == signum) + .map(|(_, name)| *name) + .unwrap_or("Unknown signal"); + + write_str_noraise(fd, "\nFatal Python error: "); + write_str_noraise(fd, signal_name); + write_str_noraise(fd, "\n\n"); + + // Restore previous handler + if let Some(idx) = FATAL_SIGNALS.iter().position(|(sig, _)| *sig == signum) { + unsafe { + libc::sigaction(signum, &PREVIOUS_HANDLERS[idx], std::ptr::null_mut()); + } + } + + REENTRANT.store(false, Ordering::SeqCst); + + // Re-raise signal to trigger default behavior (core dump, etc.) + unsafe { + libc::raise(signum); + } + } + + #[cfg(unix)] fn install_fatal_handlers(_vm: &VirtualMachine) { - // TODO: Install actual signal handlers for SIGSEGV, SIGFPE, etc. - // This requires careful handling because signal handlers have limited capabilities. - // For now, this is a placeholder that marks the module as enabled. + for (idx, (signum, _)) in FATAL_SIGNALS.iter().enumerate() { + let mut action: libc::sigaction = unsafe { std::mem::zeroed() }; + action.sa_sigaction = faulthandler_fatal_error as usize; + action.sa_flags = libc::SA_NODEFER; + + unsafe { + libc::sigaction(*signum, &action, &mut PREVIOUS_HANDLERS[idx]); + } + } + } + + /// Windows signal handler for fatal errors (simpler than VEH) + #[cfg(windows)] + extern "C" fn faulthandler_fatal_error_windows(signum: libc::c_int) { + // Reentrant protection + static REENTRANT: AtomicBool = AtomicBool::new(false); + if REENTRANT.swap(true, Ordering::SeqCst) { + return; + } + + let fd = FATAL_ERROR_FD.load(Ordering::Relaxed); + + // Find and print signal name + let signal_name = FATAL_SIGNALS + .iter() + .find(|(sig, _)| *sig == signum) + .map(|(_, name)| *name) + .unwrap_or("Unknown signal"); + + write_str_noraise(fd, "\nFatal Python error: "); + write_str_noraise(fd, signal_name); + write_str_noraise(fd, "\n\n"); + + REENTRANT.store(false, Ordering::SeqCst); + + // For SIGSEGV on Windows, need to loop raise + if signum == libc::SIGSEGV { + loop { + unsafe { + libc::raise(signum); + } + } + } else { + unsafe { + libc::raise(signum); + } + } + } + + #[cfg(windows)] + static mut PREVIOUS_HANDLERS_WIN: [libc::sighandler_t; NUM_FATAL_SIGNALS] = + [0; NUM_FATAL_SIGNALS]; + + #[cfg(windows)] + fn install_fatal_handlers(_vm: &VirtualMachine) { + for (idx, (signum, _)) in FATAL_SIGNALS.iter().enumerate() { + unsafe { + PREVIOUS_HANDLERS_WIN[idx] = libc::signal( + *signum, + faulthandler_fatal_error_windows as libc::sighandler_t, + ); + } + } } #[pyfunction] @@ -142,7 +279,7 @@ mod decl { let was_enabled = ENABLED.swap(false, Ordering::Relaxed); // Restore default signal handlers - #[cfg(not(target_arch = "wasm32"))] + #[cfg(any(unix, windows))] { uninstall_fatal_handlers(); } @@ -150,9 +287,22 @@ mod decl { was_enabled } - #[cfg(not(target_arch = "wasm32"))] + #[cfg(unix)] + fn uninstall_fatal_handlers() { + for (idx, (signum, _)) in FATAL_SIGNALS.iter().enumerate() { + unsafe { + libc::sigaction(*signum, &PREVIOUS_HANDLERS[idx], std::ptr::null_mut()); + } + } + } + + #[cfg(windows)] fn uninstall_fatal_handlers() { - // TODO: Restore original signal handlers + for (idx, (signum, _)) in FATAL_SIGNALS.iter().enumerate() { + unsafe { + libc::signal(*signum, PREVIOUS_HANDLERS_WIN[idx]); + } + } } #[pyfunction] @@ -251,6 +401,15 @@ mod decl { #[cfg(not(target_arch = "wasm32"))] { let header_bytes = header.as_bytes(); + #[cfg(windows)] + unsafe { + libc::write( + fd, + header_bytes.as_ptr() as *const libc::c_void, + header_bytes.len() as u32, + ); + } + #[cfg(not(windows))] unsafe { libc::write( fd, @@ -263,6 +422,11 @@ mod decl { // because we don't have access to the VM's frame stack. // Just output a message indicating timeout occurred. let msg = b"\n"; + #[cfg(windows)] + unsafe { + libc::write(fd, msg.as_ptr() as *const libc::c_void, msg.len() as u32); + } + #[cfg(not(windows))] unsafe { libc::write(fd, msg.as_ptr() as *const libc::c_void, msg.len()); } @@ -473,7 +637,7 @@ mod decl { #[cfg(unix)] fn check_signum(signum: i32, vm: &VirtualMachine) -> PyResult<()> { // Check if it's a fatal signal - if FATAL_SIGNALS.contains(&signum) { + if FATAL_SIGNALS.iter().any(|(sig, _)| *sig == signum) { return Err(vm.new_runtime_error(format!( "signal {} cannot be registered, use enable() instead", signum From 96f44ac87286db757b4e687f9a5b4092736cad4d Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Thu, 11 Dec 2025 22:43:21 +0900 Subject: [PATCH 7/7] Apply review based on cpython --- crates/stdlib/Cargo.toml | 1 + crates/stdlib/src/faulthandler.rs | 173 +++++++++++------------------- 2 files changed, 64 insertions(+), 110 deletions(-) diff --git a/crates/stdlib/Cargo.toml b/crates/stdlib/Cargo.toml index 593b5f196f..a5328697ca 100644 --- a/crates/stdlib/Cargo.toml +++ b/crates/stdlib/Cargo.toml @@ -150,6 +150,7 @@ features = [ "Win32_NetworkManagement_Ndis", "Win32_Security_Cryptography", "Win32_Storage_FileSystem", + "Win32_System_Diagnostics_Debug", "Win32_System_Environment", "Win32_System_IO", "Win32_System_Threading" diff --git a/crates/stdlib/src/faulthandler.rs b/crates/stdlib/src/faulthandler.rs index 00f64f8f6c..e9006b982a 100644 --- a/crates/stdlib/src/faulthandler.rs +++ b/crates/stdlib/src/faulthandler.rs @@ -111,11 +111,15 @@ mod decl { fn collect_frame_info(frame: &crate::vm::PyRef) -> String { let func_name = truncate_name(frame.code.obj_name.as_str()); + // If lasti is 0, execution hasn't started yet - use first line number or 1 + let line = if frame.lasti() == 0 { + frame.code.first_line_number.map(|n| n.get()).unwrap_or(1) + } else { + frame.current_location().line.get() + }; format!( " File \"{}\", line {} in {}", - frame.code.source_path, - frame.current_location().line, - func_name + frame.code.source_path, line, func_name ) } @@ -171,37 +175,34 @@ mod decl { } /// Unix signal handler for fatal errors + // faulthandler_fatal_error() in Modules/faulthandler.c #[cfg(unix)] extern "C" fn faulthandler_fatal_error(signum: libc::c_int) { - // Reentrant protection - static REENTRANT: AtomicBool = AtomicBool::new(false); - if REENTRANT.swap(true, Ordering::SeqCst) { + if !ENABLED.load(Ordering::Relaxed) { return; } let fd = FATAL_ERROR_FD.load(Ordering::Relaxed); - // Find and print signal name - let signal_name = FATAL_SIGNALS - .iter() - .find(|(sig, _)| *sig == signum) - .map(|(_, name)| *name) - .unwrap_or("Unknown signal"); + // Find handler and restore previous handler BEFORE printing + let signal_name = + if let Some(idx) = FATAL_SIGNALS.iter().position(|(sig, _)| *sig == signum) { + // Restore previous handler first + unsafe { + libc::sigaction(signum, &PREVIOUS_HANDLERS[idx], std::ptr::null_mut()); + } + FATAL_SIGNALS[idx].1 + } else { + "Unknown signal" + }; - write_str_noraise(fd, "\nFatal Python error: "); + // Print error message + write_str_noraise(fd, "Fatal Python error: "); write_str_noraise(fd, signal_name); write_str_noraise(fd, "\n\n"); - // Restore previous handler - if let Some(idx) = FATAL_SIGNALS.iter().position(|(sig, _)| *sig == signum) { - unsafe { - libc::sigaction(signum, &PREVIOUS_HANDLERS[idx], std::ptr::null_mut()); - } - } - - REENTRANT.store(false, Ordering::SeqCst); - // Re-raise signal to trigger default behavior (core dump, etc.) + // Called immediately thanks to SA_NODEFER flag unsafe { libc::raise(signum); } @@ -211,7 +212,7 @@ mod decl { fn install_fatal_handlers(_vm: &VirtualMachine) { for (idx, (signum, _)) in FATAL_SIGNALS.iter().enumerate() { let mut action: libc::sigaction = unsafe { std::mem::zeroed() }; - action.sa_sigaction = faulthandler_fatal_error as usize; + action.sa_sigaction = faulthandler_fatal_error as libc::sighandler_t; action.sa_flags = libc::SA_NODEFER; unsafe { @@ -220,41 +221,43 @@ mod decl { } } - /// Windows signal handler for fatal errors (simpler than VEH) + /// Windows signal handler for fatal errors + // faulthandler_fatal_error() in Modules/faulthandler.c #[cfg(windows)] extern "C" fn faulthandler_fatal_error_windows(signum: libc::c_int) { - // Reentrant protection - static REENTRANT: AtomicBool = AtomicBool::new(false); - if REENTRANT.swap(true, Ordering::SeqCst) { + if !ENABLED.load(Ordering::Relaxed) { return; } let fd = FATAL_ERROR_FD.load(Ordering::Relaxed); - // Find and print signal name - let signal_name = FATAL_SIGNALS - .iter() - .find(|(sig, _)| *sig == signum) - .map(|(_, name)| *name) - .unwrap_or("Unknown signal"); + // Find handler and restore previous handler BEFORE printing + let signal_name = + if let Some(idx) = FATAL_SIGNALS.iter().position(|(sig, _)| *sig == signum) { + // Restore previous handler first + unsafe { + libc::signal(signum, PREVIOUS_HANDLERS_WIN[idx]); + } + FATAL_SIGNALS[idx].1 + } else { + "Unknown signal" + }; - write_str_noraise(fd, "\nFatal Python error: "); + // Print error message + write_str_noraise(fd, "Fatal Python error: "); write_str_noraise(fd, signal_name); write_str_noraise(fd, "\n\n"); - REENTRANT.store(false, Ordering::SeqCst); - - // For SIGSEGV on Windows, need to loop raise + // On Windows, don't explicitly call the previous handler for SIGSEGV + // The execution continues and the same instruction raises the same fault, + // calling the now-restored previous handler if signum == libc::SIGSEGV { - loop { - unsafe { - libc::raise(signum); - } - } - } else { - unsafe { - libc::raise(signum); - } + return; + } + + // For other signals, re-raise to call the previous handler + unsafe { + libc::raise(signum); } } @@ -367,36 +370,25 @@ mod decl { let (lock, cvar) = &*state; loop { - let (timeout, repeat, exit, fd, header, cancelled) = { - let guard = lock.lock().unwrap(); - if guard.cancel { - return; - } - ( - Duration::from_micros(guard.timeout_us), - guard.repeat, - guard.exit, - guard.fd, - guard.header.clone(), - guard.cancel, - ) - }; - - if cancelled { + // Hold lock across wait_timeout to avoid race condition + let mut guard = lock.lock().unwrap(); + if guard.cancel { return; } + let timeout = Duration::from_micros(guard.timeout_us); + let result = cvar.wait_timeout(guard, timeout).unwrap(); + guard = result.0; - // Wait for timeout or cancellation - let result = { - let guard = lock.lock().unwrap(); - cvar.wait_timeout(guard, timeout).unwrap() - }; - - // Check if cancelled - if result.0.cancel { + // Check if cancelled after wait + if guard.cancel { return; } + // Extract values before releasing lock for I/O + let (repeat, exit, fd, header) = + (guard.repeat, guard.exit, guard.fd, guard.header.clone()); + drop(guard); // Release lock before I/O + // Timeout occurred, dump traceback #[cfg(not(target_arch = "wasm32"))] { @@ -652,45 +644,6 @@ mod decl { Ok(()) } - #[cfg(unix)] - fn get_fd_from_file(file: OptionalArg, vm: &VirtualMachine) -> PyResult { - match file { - OptionalArg::Present(f) => { - // Check if it's an integer (file descriptor) - if let Ok(fd) = f.try_to_value::(vm) { - if fd < 0 { - return Err( - vm.new_value_error("file is not a valid file descriptor".to_owned()) - ); - } - return Ok(fd); - } - // Try to get fileno() from file object - let fileno = vm.call_method(&f, "fileno", ())?; - let fd: i32 = fileno.try_to_value(vm)?; - if fd < 0 { - return Err( - vm.new_value_error("file is not a valid file descriptor".to_owned()) - ); - } - // Try to flush the file - let _ = vm.call_method(&f, "flush", ()); - Ok(fd) - } - OptionalArg::Missing => { - // Get sys.stderr - let stderr = vm.sys_module.get_attr("stderr", vm)?; - if vm.is_none(&stderr) { - return Err(vm.new_runtime_error("sys.stderr is None".to_owned())); - } - let fileno = vm.call_method(&stderr, "fileno", ())?; - let fd: i32 = fileno.try_to_value(vm)?; - let _ = vm.call_method(&stderr, "flush", ()); - Ok(fd) - } - } - } - #[cfg(unix)] #[derive(FromArgs)] #[allow(unused)] @@ -710,7 +663,7 @@ mod decl { fn register(args: RegisterArgs, vm: &VirtualMachine) -> PyResult<()> { check_signum(args.signum, vm)?; - let fd = get_fd_from_file(args.file, vm)?; + let fd = get_fd_from_file_opt(args.file, vm)?; let signum = args.signum as usize;