Skip to content

Commit 7ee743e

Browse files
committed
fix more windows fix
1 parent ccf40fd commit 7ee743e

5 files changed

Lines changed: 169 additions & 93 deletions

File tree

‎Lib/test/test_faulthandler.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -533,7 +533,6 @@ def funcA():
533533
def test_dump_traceback(self):
534534
self.check_dump_traceback()
535535

536-
@unittest.expectedFailure # TODO: RUSTPYTHON; - binary file write needs different handling
537536
def test_dump_traceback_file(self):
538537
with temporary_filename() as filename:
539538
self.check_dump_traceback(filename=filename)
@@ -629,11 +628,9 @@ def run(self):
629628
self.assertRegex(output, regex)
630629
self.assertEqual(exitcode, 0)
631630

632-
@unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: Regex didn't match: '^Thread 0x[0-9a-f]+( \\[.*\\])? \\(most recent call first\\):\n(?: File ".*threading.py", line [0-9]+ in [_a-z]+\n){1,3} File "<string>", line (?:22|23) in run\n File ".*threading.py", line [0-9]+ in _bootstrap_inner\n File ".*threading.py", line [0-9]+ in _bootstrap\n\nCurrent thread 0x[0-9a-f]+( \\[.*\\])? \\(most recent call first\\):\n File "<string>", line 10 in dump\n File "<string>", line 28 in <module>$' not found in 'Stack (most recent call first):\n File "<string>", line 10 in dump\n File "<string>", line 28 in <module>'
633631
def test_dump_traceback_threads(self):
634632
self.check_dump_traceback_threads(None)
635633

636-
@unittest.expectedFailure # TODO: RUSTPYTHON; - TypeError: a bytes-like object is required, not 'str'
637634
def test_dump_traceback_threads_file(self):
638635
with temporary_filename() as filename:
639636
self.check_dump_traceback_threads(filename)

‎crates/stdlib/src/faulthandler.rs‎

Lines changed: 141 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ mod decl {
66
use crate::vm::{
77
PyObjectRef, PyResult, VirtualMachine,
88
frame::Frame,
9-
frame_snapshot::{self, FRAME_SNAPSHOTS, SNAPSHOT_COUNT, SNAPSHOT_ENABLED},
9+
frame_snapshot::SNAPSHOT_ENABLED,
1010
function::{ArgIntoFloat, OptionalArg},
11-
py_io::Write,
1211
};
1312
use alloc::sync::Arc;
1413
use core::sync::atomic::{AtomicBool, AtomicI32, Ordering};
@@ -98,6 +97,10 @@ mod decl {
9897
all_threads: AtomicBool::new(true),
9998
};
10099

100+
/// Arc<Mutex<Vec<FrameRef>>> - shared frame slot for a thread
101+
#[cfg(feature = "threading")]
102+
type ThreadFrameSlot = Arc<parking_lot::Mutex<Vec<crate::vm::frame::FrameRef>>>;
103+
101104
// Watchdog thread state for dump_traceback_later
102105
struct WatchdogState {
103106
cancel: bool,
@@ -106,6 +109,8 @@ mod decl {
106109
repeat: bool,
107110
exit: bool,
108111
header: String,
112+
#[cfg(feature = "threading")]
113+
thread_frame_slots: Vec<(u64, ThreadFrameSlot)>,
109114
}
110115

111116
type WatchdogHandle = Arc<(Mutex<WatchdogState>, Condvar)>;
@@ -238,6 +243,7 @@ mod decl {
238243
/// Dump frame snapshots to fd (signal-safe)
239244
#[cfg(any(unix, windows))]
240245
fn dump_snapshot_frames(fd: i32) {
246+
use crate::vm::frame_snapshot::{FRAME_SNAPSHOTS, SNAPSHOT_COUNT};
241247
let count = SNAPSHOT_COUNT.load(Ordering::Acquire);
242248
if count > 0 {
243249
#[allow(clippy::needless_range_loop)]
@@ -282,35 +288,60 @@ mod decl {
282288

283289
/// Snapshot current Python frames into signal-safe global storage.
284290
fn snapshot_current_frames(vm: &VirtualMachine) {
285-
frame_snapshot::do_update_frame_snapshots(&vm.frames.borrow());
291+
crate::vm::frame_snapshot::do_update_frame_snapshots(&vm.frames.borrow());
286292
}
287293

288294
const MAX_FUNCTION_NAME_LEN: usize = 500;
289295

290-
fn truncate_name(name: &str) -> String {
291-
if name.len() > MAX_FUNCTION_NAME_LEN {
292-
format!("{}...", &name[..MAX_FUNCTION_NAME_LEN])
296+
/// Write a frame's info to an fd using signal-safe I/O.
297+
/// Function name is truncated to MAX_FUNCTION_NAME_LEN.
298+
#[cfg(any(unix, windows))]
299+
fn dump_frame_from_ref(fd: i32, frame: &crate::vm::PyRef<Frame>) {
300+
let funcname = frame.code.obj_name.as_str();
301+
let filename = frame.code.source_path.as_str();
302+
let lineno = if frame.lasti() == 0 {
303+
frame.code.first_line_number.map(|n| n.get()).unwrap_or(1) as u32
304+
} else {
305+
frame.current_location().line.get() as u32
306+
};
307+
308+
puts(fd, " File \"");
309+
puts(fd, filename);
310+
puts(fd, "\", line ");
311+
dump_decimal(fd, lineno as usize);
312+
puts(fd, " in ");
313+
if funcname.len() > MAX_FUNCTION_NAME_LEN {
314+
puts(fd, &funcname[..MAX_FUNCTION_NAME_LEN]);
315+
puts(fd, "...");
293316
} else {
294-
name.to_string()
317+
puts(fd, funcname);
295318
}
319+
puts(fd, "\n");
296320
}
297321

298-
fn collect_frame_info(frame: &crate::vm::PyRef<Frame>) -> String {
299-
let func_name = truncate_name(frame.code.obj_name.as_str());
300-
let line = if frame.lasti() == 0 {
301-
// Frame hasn't started executing yet. Use first instruction's line.
302-
if !frame.code.locations.is_empty() {
303-
frame.code.locations[0].0.line.get()
304-
} else {
305-
frame.code.first_line_number.map(|n| n.get()).unwrap_or(1)
306-
}
322+
/// Dump traceback for a thread given its frame stack (for cross-thread dumping).
323+
#[cfg(all(any(unix, windows), feature = "threading"))]
324+
fn dump_traceback_thread_frames(
325+
fd: i32,
326+
thread_id: u64,
327+
is_current: bool,
328+
frames: &[crate::vm::frame::FrameRef],
329+
) {
330+
if is_current {
331+
puts(fd, "Current thread ");
307332
} else {
308-
frame.current_location().line.get()
309-
};
310-
format!(
311-
" File \"{}\", line {} in {}",
312-
frame.code.source_path, line, func_name
313-
)
333+
puts(fd, "Thread ");
334+
}
335+
dump_hexadecimal(fd, thread_id, core::mem::size_of::<usize>() * 2);
336+
puts(fd, " (most recent call first):\n");
337+
338+
if frames.is_empty() {
339+
puts(fd, " <no Python frame>\n");
340+
} else {
341+
for frame in frames.iter().rev() {
342+
dump_frame_from_ref(fd, frame);
343+
}
344+
}
314345
}
315346

316347
#[derive(FromArgs)]
@@ -323,66 +354,73 @@ mod decl {
323354

324355
#[pyfunction]
325356
fn dump_traceback(args: DumpTracebackArgs, vm: &VirtualMachine) -> PyResult<()> {
326-
let _ = args.all_threads; // TODO: implement all_threads support
357+
let fd = get_fd_from_file_opt(args.file, vm)?;
327358

328-
// Collect frame info first to avoid RefCell borrow conflict
329-
let frame_lines: Vec<String> = vm.frames.borrow().iter().map(collect_frame_info).collect();
359+
#[cfg(any(unix, windows))]
360+
{
361+
if args.all_threads {
362+
dump_all_threads(fd, vm);
363+
} else {
364+
puts(fd, "Stack (most recent call first):\n");
365+
let frames = vm.frames.borrow();
366+
for frame in frames.iter().rev() {
367+
dump_frame_from_ref(fd, frame);
368+
}
369+
}
370+
}
330371

331-
// Check if file argument is an integer (fd)
332-
let file_obj = match args.file {
333-
OptionalArg::Present(ref f) if !vm.is_none(f) => Some(f.clone()),
334-
_ => None,
335-
};
372+
#[cfg(not(any(unix, windows)))]
373+
{
374+
let _ = (fd, args.all_threads);
375+
}
376+
377+
Ok(())
378+
}
336379

337-
if let Some(ref f) = file_obj
338-
&& let Ok(fd) = f.try_to_value::<i32>(vm)
380+
/// Dump tracebacks of all threads.
381+
#[cfg(any(unix, windows))]
382+
fn dump_all_threads(fd: i32, vm: &VirtualMachine) {
383+
// Get all threads' frame stacks from the shared registry
384+
#[cfg(feature = "threading")]
339385
{
340-
// Write directly to file descriptor using libc::write
341-
#[cfg(not(target_arch = "wasm32"))]
342-
{
343-
let mut output = String::new();
344-
output.push_str("Stack (most recent call first):\n");
345-
for line in frame_lines.iter().rev() {
346-
output.push_str(line);
347-
output.push('\n');
348-
}
349-
let bytes = output.as_bytes();
350-
unsafe {
351-
#[cfg(windows)]
352-
{
353-
libc::write(
354-
fd,
355-
bytes.as_ptr() as *const libc::c_void,
356-
bytes.len() as u32,
357-
);
358-
}
359-
#[cfg(not(windows))]
360-
{
361-
libc::write(fd, bytes.as_ptr() as *const libc::c_void, bytes.len());
362-
}
386+
let current_tid = rustpython_vm::stdlib::thread::get_ident();
387+
let registry = vm.state.thread_frames.lock();
388+
389+
// First dump non-current threads, then current thread last
390+
for (&tid, slot) in registry.iter() {
391+
if tid == current_tid {
392+
continue;
363393
}
394+
let frames_guard = slot.lock();
395+
dump_traceback_thread_frames(fd, tid, false, &frames_guard);
396+
puts(fd, "\n");
364397
}
365-
return Ok(());
366-
}
367398

368-
// Use PyWriter for file objects (or stderr)
369-
let file = match file_obj {
370-
Some(f) => f,
371-
None => {
372-
let stderr = vm.sys_module.get_attr("stderr", vm)?;
373-
if vm.is_none(&stderr) {
374-
return Err(vm.new_runtime_error("sys.stderr is None".to_owned()));
399+
// Now dump current thread (use vm.frames for most up-to-date data)
400+
puts(fd, "Current thread ");
401+
dump_hexadecimal(fd, current_tid, core::mem::size_of::<usize>() * 2);
402+
puts(fd, " (most recent call first):\n");
403+
let frames = vm.frames.borrow();
404+
if frames.is_empty() {
405+
puts(fd, " <no Python frame>\n");
406+
} else {
407+
for frame in frames.iter().rev() {
408+
dump_frame_from_ref(fd, frame);
375409
}
376-
stderr
377410
}
378-
};
411+
}
379412

380-
let mut writer = crate::vm::py_io::PyWriter(file, vm);
381-
writeln!(writer, "Stack (most recent call first):")?;
382-
for line in frame_lines.iter().rev() {
383-
writeln!(writer, "{}", line)?;
413+
#[cfg(not(feature = "threading"))]
414+
{
415+
let current_tid = current_thread_id();
416+
puts(fd, "Current thread ");
417+
dump_hexadecimal(fd, current_tid, core::mem::size_of::<usize>() * 2);
418+
puts(fd, " (most recent call first):\n");
419+
let frames = vm.frames.borrow();
420+
for frame in frames.iter().rev() {
421+
dump_frame_from_ref(fd, frame);
422+
}
384423
}
385-
Ok(())
386424
}
387425

388426
#[derive(FromArgs)]
@@ -819,8 +857,12 @@ mod decl {
819857
}
820858

821859
// Extract values before releasing lock for I/O
822-
let (repeat, exit, fd, header) =
823-
(guard.repeat, guard.exit, guard.fd, guard.header.clone());
860+
let repeat = guard.repeat;
861+
let exit = guard.exit;
862+
let fd = guard.fd;
863+
let header = guard.header.clone();
864+
#[cfg(feature = "threading")]
865+
let thread_frame_slots = guard.thread_frame_slots.clone();
824866
drop(guard); // Release lock before I/O
825867

826868
// Timeout occurred, dump traceback
@@ -831,9 +873,20 @@ mod decl {
831873
{
832874
puts_bytes(fd, header.as_bytes());
833875

834-
// Dump traceback from frame snapshots (updated by main thread)
835-
write_thread_id(fd, false);
836-
dump_snapshot_frames(fd);
876+
// Use thread frame slots when threading is enabled (includes all threads).
877+
// Fall back to frame snapshots for non-threaded builds.
878+
#[cfg(feature = "threading")]
879+
{
880+
for (tid, slot) in &thread_frame_slots {
881+
let frames = slot.lock();
882+
dump_traceback_thread_frames(fd, *tid, false, &frames);
883+
}
884+
}
885+
#[cfg(not(feature = "threading"))]
886+
{
887+
write_thread_id(fd, false);
888+
dump_snapshot_frames(fd);
889+
}
837890

838891
if exit {
839892
std::process::exit(1);
@@ -881,6 +934,16 @@ mod decl {
881934
SNAPSHOT_ENABLED.store(true, Ordering::Relaxed);
882935
snapshot_current_frames(vm);
883936

937+
// Snapshot thread frame slots so watchdog can dump tracebacks
938+
#[cfg(feature = "threading")]
939+
let thread_frame_slots: Vec<(u64, ThreadFrameSlot)> = {
940+
let registry = vm.state.thread_frames.lock();
941+
registry
942+
.iter()
943+
.map(|(&id, slot)| (id, Arc::clone(slot)))
944+
.collect()
945+
};
946+
884947
// Cancel any previous watchdog
885948
cancel_dump_traceback_later();
886949

@@ -893,6 +956,8 @@ mod decl {
893956
repeat: args.repeat,
894957
exit: args.exit,
895958
header,
959+
#[cfg(feature = "threading")]
960+
thread_frame_slots,
896961
}),
897962
Condvar::new(),
898963
));

‎crates/vm/src/stdlib/thread.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
//! Implementation of the _thread module
22
#[cfg(unix)]
33
pub(crate) use _thread::after_fork_child;
4+
pub use _thread::get_ident;
45
#[cfg_attr(target_arch = "wasm32", allow(unused_imports))]
56
pub(crate) use _thread::{
6-
CurrentFrameSlot, HandleEntry, RawRMutex, ShutdownEntry, get_all_current_frames, get_ident,
7+
CurrentFrameSlot, HandleEntry, RawRMutex, ShutdownEntry, get_all_current_frames,
78
init_main_thread_ident, module_def,
89
};
910

@@ -873,12 +874,12 @@ pub(crate) mod _thread {
873874
// Re-export type from vm::thread for PyGlobalState
874875
pub use crate::vm::thread::CurrentFrameSlot;
875876

876-
/// Get all threads' current frames. Used by sys._current_frames().
877+
/// Get all threads' current (top) frames. Used by sys._current_frames().
877878
pub fn get_all_current_frames(vm: &VirtualMachine) -> Vec<(u64, FrameRef)> {
878879
let registry = vm.state.thread_frames.lock();
879880
registry
880881
.iter()
881-
.filter_map(|(id, slot)| slot.lock().clone().map(|f| (*id, f)))
882+
.filter_map(|(id, slot)| slot.lock().last().cloned().map(|f| (*id, f)))
882883
.collect()
883884
}
884885

‎crates/vm/src/vm/mod.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -939,9 +939,9 @@ impl VirtualMachine {
939939
) -> PyResult<R> {
940940
self.with_recursion("", || {
941941
self.frames.borrow_mut().push(frame.clone());
942-
// Update the current frame slot for sys._current_frames()
942+
// Update the shared frame stack for sys._current_frames() and faulthandler
943943
#[cfg(feature = "threading")]
944-
crate::vm::thread::update_current_frame(Some(frame.clone()));
944+
crate::vm::thread::push_thread_frame(frame.clone());
945945
// Update frame snapshots for signal handlers (faulthandler)
946946
crate::frame_snapshot::update_frame_snapshots(&self.frames.borrow());
947947
// Push a new exception context for frame isolation
@@ -953,9 +953,9 @@ impl VirtualMachine {
953953
self.pop_exception();
954954
// defer dec frame
955955
let _popped = self.frames.borrow_mut().pop();
956-
// Update the frame slot to the new top frame (or None if empty)
956+
// Pop from shared frame stack
957957
#[cfg(feature = "threading")]
958-
crate::vm::thread::update_current_frame(self.frames.borrow().last().cloned());
958+
crate::vm::thread::pop_thread_frame();
959959
// Update frame snapshots after pop
960960
crate::frame_snapshot::update_frame_snapshots(&self.frames.borrow());
961961
result

0 commit comments

Comments
 (0)