Skip to content

Commit bc4425b

Browse files
youknowoneyouknowone
committed
Unify frame chain: replace dual-chain with single FrameChainPtr
Replace the dual-chain frame management (CURRENT_FRAME AtomicPtr<Frame> + CURRENT_LIGHT_FRAME Cell<*const LightFrame>) with a single unified chain using FrameChainPtr, a tagged pointer where bit 0 distinguishes heavy (*const Frame) from light (*const LightFrame) entries. Changes: - Add FrameChainPtr type with tagged pointer encoding - Change LightFrame: replace previous_light + saved_current_frame with single previous: FrameChainPtr field - Change InterpreterFrame.previous from AtomicPtr<Frame> to PyAtomic<usize> - Change CURRENT_FRAME TLS from AtomicPtr<Frame> to AtomicUsize - Remove CURRENT_LIGHT_FRAME TLS entirely - Simplify all chain walkers (frame_at_offset, frame_at_offset_vm, find_owned_chain_frame, for_each_current_frame, current_thread_frame) - Remove light-frame materialization block from with_frame - Update f_back to dispatch on FrameChainPtr tags - Update faulthandler and gc_state to walk unified chain - Adaptive C stack check: every call in debug, every 16th in release Assisted-by: Claude
1 parent ece8197 commit bc4425b

8 files changed

Lines changed: 344 additions & 302 deletions

File tree

‎.gitignore‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,5 +29,6 @@ Lib/site-packages/*
2929
Lib/test/data/*
3030
!Lib/test/data/README
3131
cpython/
32-
.claude/scheduled_tasks.lock
32+
.claude/
33+
rustpython-unicode-isolation-issue.md
3334
docs/superpowers/

‎crates/stdlib/src/faulthandler.rs‎

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -119,25 +119,27 @@ mod decl {
119119
}
120120

121121
/// Dump the current thread's live frame chain to fd (signal-safe).
122-
/// Walks the `Frame.previous` pointer chain starting from the
123-
/// thread-local current frame pointer.
122+
/// Walks the unified frame chain, skipping light entries (not
123+
/// signal-safe to dereference in a signal handler).
124124
#[cfg(any(unix, windows))]
125125
fn dump_live_frames(fd: i32) {
126126
const MAX_FRAME_DEPTH: usize = 100;
127127

128-
let mut frame_ptr = crate::vm::vm::thread::get_current_frame();
129-
if frame_ptr.is_null() {
128+
let mut cur = crate::vm::vm::thread::get_current_frame();
129+
if cur.is_null() {
130130
puts(fd, " <no Python frame>\n");
131131
return;
132132
}
133133
let mut depth = 0;
134-
while !frame_ptr.is_null() && depth < MAX_FRAME_DEPTH {
135-
let frame = unsafe { &*frame_ptr };
136-
dump_frame_from_raw(fd, frame);
137-
frame_ptr = frame.previous_frame();
138-
depth += 1;
134+
while !cur.is_null() && depth < MAX_FRAME_DEPTH {
135+
if let Some(heavy) = cur.as_heavy() {
136+
let frame = unsafe { &*heavy };
137+
dump_frame_from_raw(fd, frame);
138+
depth += 1;
139+
}
140+
cur = unsafe { cur.next() };
139141
}
140-
if depth >= MAX_FRAME_DEPTH && !frame_ptr.is_null() {
142+
if depth >= MAX_FRAME_DEPTH && !cur.is_null() {
141143
puts(fd, " ...\n");
142144
}
143145
}
@@ -268,23 +270,25 @@ mod decl {
268270
/// may still run).
269271
#[cfg(all(unix, feature = "threading"))]
270272
fn dump_traceback_thread_chain(fd: i32, thread_id: u64, is_current: bool, top: *const Frame) {
273+
use crate::vm::frame::FrameChainPtr;
271274
const MAX_FRAME_DEPTH: usize = 100;
272275
write_thread_id(fd, thread_id, is_current);
273276

274277
if top.is_null() {
275278
puts(fd, " <no Python frame>\n");
276279
return;
277280
}
278-
let mut frame_ptr = top;
281+
let mut cur = FrameChainPtr::from_heavy(top);
279282
let mut depth = 0;
280-
while !frame_ptr.is_null() && depth < MAX_FRAME_DEPTH {
281-
// SAFETY: the frame is alive per the caller's liveness guarantee.
282-
let frame = unsafe { &*frame_ptr };
283-
dump_frame_from_raw(fd, frame);
284-
frame_ptr = frame.previous_frame();
285-
depth += 1;
283+
while !cur.is_null() && depth < MAX_FRAME_DEPTH {
284+
if let Some(heavy) = cur.as_heavy() {
285+
let frame = unsafe { &*heavy };
286+
dump_frame_from_raw(fd, frame);
287+
depth += 1;
288+
}
289+
cur = unsafe { cur.next() };
286290
}
287-
if depth >= MAX_FRAME_DEPTH && !frame_ptr.is_null() {
291+
if depth >= MAX_FRAME_DEPTH && !cur.is_null() {
288292
puts(fd, " ...\n");
289293
}
290294
}

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

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -724,22 +724,26 @@ impl Py<Frame> {
724724

725725
#[pygetset]
726726
pub fn f_back(&self, vm: &VirtualMachine) -> Option<PyRef<Frame>> {
727-
#[cfg(not(feature = "threading"))]
728-
let _ = vm;
729-
let previous = self.previous_frame();
730-
if previous.is_null() {
727+
let chain = self.previous_frame();
728+
if chain.is_null() {
731729
return None;
732730
}
733731

734-
// Look for the caller on the current thread's signal-safe frame chain.
735-
// Finding it there proves it is still live on this thread.
736-
if let Some(frame) = crate::frame::find_owned_chain_frame(previous) {
732+
// Light frame predecessor: materialize it
733+
if let Some(light) = chain.as_light() {
734+
let frame = unsafe { crate::frame::materialize_light_frame_pub(light, vm) };
737735
frame.mark_escaped();
738736
return Some(frame);
739737
}
740738

741-
// The caller already returned and left the live chain, but this frame
742-
// escaped and retained a strong reference to it at release time.
739+
// Heavy frame predecessor: look up on the current thread's chain
740+
let target = chain.as_heavy().unwrap();
741+
if let Some(frame) = crate::frame::find_owned_chain_frame(target) {
742+
frame.mark_escaped();
743+
return Some(frame);
744+
}
745+
746+
// The caller already returned — check retained_back
743747
let retained = self.retained_back.lock().clone();
744748
if let Some(frame) = retained {
745749
frame.mark_escaped();
@@ -749,7 +753,7 @@ impl Py<Frame> {
749753
// The caller lives on another thread. unix: park every thread under
750754
// stop-the-world so their frame chains are quiescent and alive, then
751755
// walk each published top frame down its `previous` chain looking for
752-
// the caller. Request stop-the-world before the registry lock.
756+
// the caller.
753757
#[cfg(all(unix, feature = "threading"))]
754758
{
755759
use core::sync::atomic::Ordering;
@@ -763,15 +767,16 @@ impl Py<Frame> {
763767
for slot in registry.values() {
764768
let mut cur = slot.top_frame.load(Ordering::Relaxed) as *const Frame;
765769
while !cur.is_null() {
766-
if core::ptr::eq(cur, previous) {
767-
// SAFETY: world stopped -> this frame is alive on its
768-
// owning thread's parked call stack.
770+
if core::ptr::eq(cur, target) {
769771
let f = unsafe { &*Self::from_payload_ptr(cur) };
770772
f.mark_escaped();
771773
return Some(f.to_owned());
772774
}
773-
// SAFETY: chain frames on a parked thread are alive.
774-
cur = unsafe { (*cur).previous_frame() };
775+
// Walk heavy-only chain from top_frame (signal-safe pointers)
776+
cur = unsafe {
777+
let prev = (*cur).previous_frame();
778+
prev.as_heavy().unwrap_or(core::ptr::null())
779+
};
775780
}
776781
}
777782
}
@@ -790,7 +795,7 @@ impl Py<Frame> {
790795
if let Some(frame) = frames.iter().find_map(|fp| {
791796
let f = unsafe { fp.as_ref() };
792797
let ptr: *const Frame = &**f;
793-
core::ptr::eq(ptr, previous).then(|| f.to_owned())
798+
core::ptr::eq(ptr, target).then(|| f.to_owned())
794799
}) {
795800
frame.mark_escaped();
796801
return Some(frame);

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

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -825,6 +825,7 @@ impl Py<PyFunction> {
825825

826826
unsafe {
827827
// Initialize the LightFrame header with borrowed pointers (NO refcount bumps)
828+
let prev_chain = crate::vm::thread::get_current_frame();
828829
core::ptr::write(
829830
light,
830831
LightFrame {
@@ -834,8 +835,7 @@ impl Py<PyFunction> {
834835
func_obj: self.as_object() as *const _,
835836
lasti: Radium::new(0),
836837
prev_line: 0,
837-
previous_light: crate::vm::thread::get_current_light_frame(),
838-
saved_current_frame: crate::vm::thread::get_current_frame(),
838+
previous: prev_chain,
839839
materialized: core::cell::UnsafeCell::new(core::ptr::null_mut()),
840840
nlocalsplus: nlocalsplus as u32,
841841
max_stackdepth: max_stackdepth as u32,
@@ -865,15 +865,25 @@ impl Py<PyFunction> {
865865
}
866866
}
867867

868-
// Push light frame onto TLS chain
869-
let prev_light = crate::vm::thread::set_current_light_frame(light);
868+
// Push light frame onto the unified chain
869+
let old_chain = crate::vm::thread::set_current_frame(
870+
crate::frame::FrameChainPtr::from_light(light),
871+
);
870872

871873
// Recursion depth and C stack overflow check
872874
let depth = vm.current_recursion_depth();
873-
if depth >= vm.recursion_limit.get() || (depth & 63 == 0 && vm.check_c_stack_overflow())
874-
{
875-
// Clean up the light frame TLS before erroring
876-
crate::vm::thread::set_current_light_frame(prev_light);
875+
// C stack check: every call in debug builds (large frames may
876+
// overflow before the Python recursion limit triggers), every
877+
// 16th call in release.
878+
#[allow(clippy::bad_bit_mask)]
879+
let c_stack_due = if cfg!(debug_assertions) {
880+
true
881+
} else {
882+
depth & 15 == 0
883+
};
884+
if depth >= vm.recursion_limit.get() || (c_stack_due && vm.check_c_stack_overflow()) {
885+
// Clean up the frame chain before erroring
886+
let _ = crate::vm::thread::set_current_frame(old_chain);
877887
// Drop values we moved into localsplus
878888
let slots =
879889
core::slice::from_raw_parts_mut(lp_ptr as *mut Option<PyObjectRef>, capacity);
@@ -890,16 +900,21 @@ impl Py<PyFunction> {
890900
let materialized_cell: *const core::cell::UnsafeCell<*mut Py<Frame>> =
891901
&(*light).materialized;
892902

893-
// Panic guard: restore TLS and recursion depth, reclaim materialized
903+
// Panic guard: restore chain and recursion depth, reclaim materialized
894904
// frame, pop DataStack. Localsplus values are dropped by
895905
// LocalsPlus::drop (run_light_frame takes ownership).
896-
// TLS restored first so destructors can re-enter the VM safely.
906+
// Chain restored first so destructors can re-enter the VM safely.
897907
scopeguard::defer! {
898908
vm.recursion_depth_decrement();
899-
crate::vm::thread::set_current_light_frame(prev_light);
909+
let _ = crate::vm::thread::set_current_frame(old_chain);
900910
let materialized_ptr = *(*materialized_cell).get();
901911
if !materialized_ptr.is_null() {
902912
let reclaim = FrameRef::from_raw(materialized_ptr as *const _);
913+
if reclaim.as_object().strong_count() > 1 {
914+
crate::gc_state::gc_state().track_object(
915+
core::ptr::NonNull::from(reclaim.as_object()),
916+
);
917+
}
903918
drop(reclaim);
904919
}
905920
vm.datastack_pop(base);

0 commit comments

Comments
 (0)