Skip to content

Commit 1a5d08b

Browse files
youknowoneyouknowone
committed
try fix
1 parent dd272c1 commit 1a5d08b

File tree

5 files changed

+42
-33
lines changed

5 files changed

+42
-33
lines changed

‎crates/stdlib/src/faulthandler.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,20 +344,23 @@ mod decl {
344344
}
345345

346346
/// Dump traceback for a thread given its frame stack (for cross-thread dumping).
347+
/// # Safety
348+
/// Each `FramePtr` must point to a live frame (caller holds the Mutex).
347349
#[cfg(all(any(unix, windows), feature = "threading"))]
348350
fn dump_traceback_thread_frames(
349351
fd: i32,
350352
thread_id: u64,
351353
is_current: bool,
352-
frames: &[crate::vm::frame::FrameRef],
354+
frames: &[rustpython_vm::vm::FramePtr],
353355
) {
354356
write_thread_id(fd, thread_id, is_current);
355357

356358
if frames.is_empty() {
357359
puts(fd, " <no Python frame>\n");
358360
} else {
359-
for frame in frames.iter().rev() {
360-
dump_frame_from_ref(fd, frame);
361+
for fp in frames.iter().rev() {
362+
// SAFETY: caller holds the Mutex, so the owning thread can't pop.
363+
dump_frame_from_ref(fd, unsafe { fp.as_ref() });
361364
}
362365
}
363366
}

‎crates/vm/src/builtins/frame.rs‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -219,16 +219,14 @@ impl Py<Frame> {
219219
{
220220
let registry = vm.state.thread_frames.lock();
221221
for slot in registry.values() {
222-
if let Some(frame) = slot
223-
.frames
224-
.lock()
225-
.iter()
226-
.find(|f| {
227-
let ptr: *const Frame = &****f;
228-
core::ptr::eq(ptr, previous)
229-
})
230-
.cloned()
231-
{
222+
let frames = slot.frames.lock();
223+
// SAFETY: the owning thread can't pop while we hold the Mutex,
224+
// so FramePtr is valid for the duration of the lock.
225+
if let Some(frame) = frames.iter().find_map(|fp| {
226+
let f = unsafe { fp.as_ref() };
227+
let ptr: *const Frame = &**f;
228+
core::ptr::eq(ptr, previous).then(|| f.to_owned())
229+
}) {
232230
return Some(frame);
233231
}
234232
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,14 @@ pub(crate) mod _thread {
891891
let registry = vm.state.thread_frames.lock();
892892
registry
893893
.iter()
894-
.filter_map(|(id, slot)| slot.frames.lock().last().cloned().map(|f| (*id, f)))
894+
.filter_map(|(id, slot)| {
895+
let frames = slot.frames.lock();
896+
// SAFETY: the owning thread can't pop while we hold the Mutex,
897+
// so the FramePtr is valid for the duration of the lock.
898+
frames
899+
.last()
900+
.map(|fp| (*id, unsafe { fp.as_ref() }.to_owned()))
901+
})
895902
.collect()
896903
}
897904

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,12 +1029,11 @@ impl VirtualMachine {
10291029
// SAFETY: `frame` (FrameRef) stays alive for the entire closure scope,
10301030
// keeping the FramePtr valid. We pass a clone to `f` so that `f`
10311031
// consuming its FrameRef doesn't invalidate our pointer.
1032-
self.frames
1033-
.borrow_mut()
1034-
.push(FramePtr(NonNull::from(&*frame)));
1032+
let fp = FramePtr(NonNull::from(&*frame));
1033+
self.frames.borrow_mut().push(fp);
10351034
// Update the shared frame stack for sys._current_frames() and faulthandler
10361035
#[cfg(feature = "threading")]
1037-
crate::vm::thread::push_thread_frame(frame.clone());
1036+
crate::vm::thread::push_thread_frame(fp);
10381037
// Link frame into the signal-safe frame chain (previous pointer)
10391038
let old_frame = crate::vm::thread::set_current_frame((&**frame) as *const Frame);
10401039
frame.previous.store(
@@ -1101,9 +1100,10 @@ impl VirtualMachine {
11011100
self.recursion_depth.update(|d| d + 1);
11021101

11031102
// SAFETY: frame (&FrameRef) stays alive for the duration, so NonNull is valid until pop.
1104-
self.frames
1105-
.borrow_mut()
1106-
.push(FramePtr(NonNull::from(&**frame)));
1103+
let fp = FramePtr(NonNull::from(&**frame));
1104+
self.frames.borrow_mut().push(fp);
1105+
#[cfg(feature = "threading")]
1106+
crate::vm::thread::push_thread_frame(fp);
11071107
let old_frame = crate::vm::thread::set_current_frame((&***frame) as *const Frame);
11081108
frame.previous.store(
11091109
old_frame as *mut Frame,
@@ -1125,6 +1125,8 @@ impl VirtualMachine {
11251125
.expect("pop_exception() without nested exc stack");
11261126
crate::vm::thread::set_current_frame(old_frame);
11271127
self.frames.borrow_mut().pop();
1128+
#[cfg(feature = "threading")]
1129+
crate::vm::thread::pop_thread_frame();
11281130
self.recursion_depth.update(|d| d - 1);
11291131
}
11301132

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
use crate::frame::Frame;
22
use crate::{AsObject, PyObject, VirtualMachine};
33
#[cfg(feature = "threading")]
4-
use crate::{builtins::PyBaseExceptionRef, frame::FrameRef};
4+
use crate::builtins::PyBaseExceptionRef;
5+
#[cfg(feature = "threading")]
6+
use super::FramePtr;
57
#[cfg(feature = "threading")]
68
use alloc::sync::Arc;
79
use core::{
@@ -16,7 +18,9 @@ use std::thread_local;
1618
/// The exception field uses atomic operations for lock-free cross-thread reads.
1719
#[cfg(feature = "threading")]
1820
pub struct ThreadSlot {
19-
pub frames: parking_lot::Mutex<Vec<FrameRef>>,
21+
/// Raw frame pointers, valid while the owning thread's call stack is active.
22+
/// Readers must hold the Mutex and convert to FrameRef inside the lock.
23+
pub frames: parking_lot::Mutex<Vec<FramePtr>>,
2024
pub exception: crate::PyAtomicRef<Option<crate::exceptions::types::PyBaseException>>,
2125
}
2226

@@ -83,13 +87,13 @@ fn init_thread_slot_if_needed(vm: &VirtualMachine) {
8387
});
8488
}
8589

86-
/// Push a frame onto the current thread's shared frame stack.
87-
/// Called when a new frame is entered.
90+
/// Push a frame pointer onto the current thread's shared frame stack.
91+
/// The pointed-to frame must remain alive until the matching pop.
8892
#[cfg(feature = "threading")]
89-
pub fn push_thread_frame(frame: FrameRef) {
93+
pub fn push_thread_frame(fp: FramePtr) {
9094
CURRENT_THREAD_SLOT.with(|slot| {
9195
if let Some(s) = slot.borrow().as_ref() {
92-
s.frames.lock().push(frame);
96+
s.frames.lock().push(fp);
9397
}
9498
});
9599
}
@@ -157,12 +161,7 @@ pub fn cleanup_current_thread_frames(vm: &VirtualMachine) {
157161
#[cfg(feature = "threading")]
158162
pub fn reinit_frame_slot_after_fork(vm: &VirtualMachine) {
159163
let current_ident = crate::stdlib::thread::get_ident();
160-
let current_frames: Vec<FrameRef> = vm
161-
.frames
162-
.borrow()
163-
.iter()
164-
.map(|fp| unsafe { fp.as_ref() }.to_owned())
165-
.collect();
164+
let current_frames: Vec<FramePtr> = vm.frames.borrow().clone();
166165
let new_slot = Arc::new(ThreadSlot {
167166
frames: parking_lot::Mutex::new(current_frames),
168167
exception: crate::PyAtomicRef::from(vm.topmost_exception()),

0 commit comments

Comments
 (0)