Visitar URL original
Faulthandler by youknowone · Pull Request #6400 · RustPython/RustPython · GitHub
Skip to content

Faulthandler #6400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 7 commits into from
Dec 11, 2025
Merged

Faulthandler #6400

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b376dea
impl faulthandler
youknowone Dec 11, 2025
f9f8427
faulthandler
youknowone Dec 11, 2025
7ee0118
string
youknowone Dec 11, 2025
0a2b2ba
fix faulthandler on macos
youknowone Dec 11, 2025
97943dc
fix watchdog
youknowone Dec 11, 2025
8a9096a
install handler
youknowone Dec 11, 2025
96f44ac
Apply review based on cpython
youknowone Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Apply review based on cpython
  • Loading branch information
@youknowone
youknowone committed Dec 11, 2025
commit 96f44ac87286db757b4e687f9a5b4092736cad4d
1 change: 1 addition & 0 deletions crates/stdlib/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
173 changes: 63 additions & 110 deletions crates/stdlib/src/faulthandler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,15 @@ mod decl {

fn collect_frame_info(frame: &crate::vm::PyRef<Frame>) -> 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
)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Expand Down Expand Up @@ -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);
}
Expand All @@ -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 {
Expand All @@ -220,41 +221,43 @@ mod decl {
}
}
Comment on lines +211 to +222

@coderabbitai coderabbitai Bot Dec 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Incorrect signal handler setup: mixing sa_sigaction with simple handler.

Line 214 assigns to sa_sigaction but the handler signature is extern "C" fn(c_int), which is the sa_handler signature. Without SA_SIGINFO flag, you should use sa_handler instead. This is undefined behavior.

     #[cfg(unix)]
     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 {
                 libc::sigaction(*signum, &action, &mut PREVIOUS_HANDLERS[idx]);
             }
         }
     }

Note: On some platforms, sa_sigaction and sa_handler are a union. Ensure the cast is correct for your target platforms. The handler function signature extern "C" fn(c_int) is correct for a simple handler.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In crates/stdlib/src/faulthandler.rs around lines 210 to 221, the signal handler
is being assigned to sa_sigaction while the function has the simple handler
signature (extern "C" fn(c_int)); change the assignment to use sa_handler
(casting the function pointer to the correct type) and keep sa_flags as-is, or
if you truly need sa_sigaction then change the handler signature to the
SA_SIGINFO form and set SA_SIGINFO in sa_flags; ensure the cast respects
platforms where sa_handler/sa_sigaction are a union so the pointer type is
correct for the target.


/// 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);
}
}

Expand Down Expand Up @@ -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;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

// 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"))]
{
Expand Down Expand Up @@ -652,45 +644,6 @@ mod decl {
Ok(())
}

#[cfg(unix)]
fn get_fd_from_file(file: OptionalArg<PyObjectRef>, vm: &VirtualMachine) -> PyResult<i32> {
match file {
OptionalArg::Present(f) => {
// Check if it's an integer (file descriptor)
if let Ok(fd) = f.try_to_value::<i32>(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)]
Expand All @@ -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;

Expand Down
Loading