Split InterpreterFrame from FrameObject for stack-allocated execution - #8354
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRustPython migrates frame execution and introspection from ChangesFrame runtime migration
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant PyFunction
participant VirtualMachine
participant InterpreterFrame
participant ThreadSlot
participant FrameObject
PyFunction->>VirtualMachine: invoke function
VirtualMachine->>InterpreterFrame: execute stack-backed frame
InterpreterFrame->>ThreadSlot: publish current iframe
InterpreterFrame->>FrameObject: materialize when required
FrameObject-->>VirtualMachine: provide frame state
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR introduces a “LightFrame” fast-path to reduce Python→Python call overhead by avoiding allocation/materialization of full Frame PyObjects for specialized exact-args call paths, and streamlines the remaining heavy with_frame path to reduce tracing/recursion-check overhead. It also updates sys._getframe and VM frame-walk helpers to account for (and lazily materialize) light frames when the call stack is observed.
Changes:
- Add a stack-allocated
LightFrameheader (with lazy materialization toFrame) plus unified heavy/light stack walking APIs for_getframe-style introspection. - Route specialized exact-args call sites through
PyFunction::invoke_light_slots()to run bytecode using a light frame on the DataStack. - Optimize
VirtualMachine::with_frameby inlining recursion checks, amortizing C-stack checks, and skipping traced-frame dispatch when tracing is off.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/vm/src/vm/thread.rs | Adds TLS pointer for the current light-frame chain (CURRENT_LIGHT_FRAME) plus get/set helpers. |
| crates/vm/src/vm/mod.rs | Streamlines with_frame recursion/C-stack checks and avoids traced dispatch when tracing is disabled; switches current_frame() to unified heavy/light lookup. |
| crates/vm/src/stdlib/sys.rs | Updates sys._getframe/related lookups to use VM-aware frame walking that can materialize light frames. |
| crates/vm/src/frame.rs | Introduces LightFrame, lazy materialization, and unified heavy/light stack walking helpers; extends ExecutingFrame to abstract heavy vs light sources. |
| crates/vm/src/builtins/function.rs | Adds invoke_light_slots fast path that allocates and runs a LightFrame on the DataStack for unobserved calls. |
Comments suppressed due to low confidence (1)
crates/vm/src/vm/mod.rs:1759
- After adding an early
recursion_depthguard, this later decrement will double-decrement the depth on normal return/panic. Remove it (or replace with a comment) and let the earlier guard handle balancing.
self.recursion_depth.update(|d| d - 1);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Set up the `previous` frame pointer for f_back: | ||
| // If the previous light frame exists, materialize it and point there. | ||
| // Otherwise, point to the top heavy frame. | ||
| let prev_light = (*light).previous_light; | ||
| let prev_frame_ptr = if !prev_light.is_null() { | ||
| let prev_materialized = materialize_light_frame(prev_light as *mut _, vm); | ||
| let ptr = &*prev_materialized as *const Py<Frame> as *mut Frame; | ||
| core::mem::forget(prev_materialized); | ||
| ptr | ||
| } else { | ||
| crate::vm::thread::get_current_frame() as *mut Frame | ||
| }; | ||
| iframe.previous.store(prev_frame_ptr, atomic::Ordering::Relaxed); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== LightFrame and thread-frame symbol references =='
rg -n -C 4 \
-e '\bLightFrame\b' \
-e '\bprevious_light\b' \
-e '\bsaved_current_frame\b' \
-e '\bget_current_light_frame\s*\(' \
-e '\bset_current_light_frame\s*\(' \
-e '\bget_current_frame\s*\(' \
-e '\bset_current_frame\s*\(' \
crates
printf '%s\n' '== Candidate source files =='
fd -t f -e rs . crates | sortLength of output: 54707
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '== Light-frame creation, execution, and cleanup =='
sed -n '780,955p' crates/vm/src/builtins/function.rs
printf '%s\n' '== Heavy-frame push/pop scopes =='
sed -n '1700,1825p' crates/vm/src/vm/mod.rs
printf '%s\n' '== f_back implementation and materialization call sites =='
rg -n -C 5 \
-e '\bfn f_back\b' \
-e '\.f_back\s*\(' \
-e '\bmaterialize_light_frame_pub\s*\(' \
-e '\bmaterialize_light_frame\s*\(' \
crates/vm/src -g '*.rs'Length of output: 20624
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '700,775p' crates/vm/src/builtins/frame.rsLength of output: 3354
@fanninpm, confirmed — the finding is valid.
A concrete failing stack is:
H0 → L1 → H1 → L2
where L1 is a light frame, H1 is a heavy Python frame entered through a native callback, and L2 is a subsequent light call.
L1.saved_current_frame == H0L2.previous_light == L1L2.saved_current_frame == H1
The current code materializes L2 with f_back = L1, skipping its immediate caller H1. Also, for a light frame without a previous_light, using get_current_frame() at materialization time can select a heavy frame that was pushed after that light frame began, rather than its caller.
f_back follows the stored previous pointer directly, so it cannot repair this later.
The materialization logic should use the entry-time boundary:
- link to
previous_lightonly when it has the samesaved_current_frame; - otherwise link to this light frame’s
saved_current_frame, rather than the current heavy-frame TLS value.
One caveat: this fixes the immediate incorrect predecessor. Preserving the entire H1 → L1 relationship for a full f_back walk across native callbacks would need additional chain handling, because heavy-frame push currently links H1.previous only to the prior heavy frame (H0).
🧬 Code Graph Analysis Results
Relevant snippets
crates/vm/src/exceptions.rs
- Lines 27-34 (GC traversal for exceptions)
unsafe impl Traverse for PyBaseException {
fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) {
self.traceback.traverse(tracer_fn);
self.cause.traverse(tracer_fn);
self.context.traverse(tracer_fn);
self.args.traverse(tracer_fn);
}
}- Lines 420-423 (Exception constructor enum)
pub enum ExceptionCtor {
Class(PyTypeRef),
Instance(PyBaseExceptionRef),
}- Lines 425-443 (converting an object into
ExceptionCtor)
impl TryFromObject for ExceptionCtor {
fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult<Self> {
obj.downcast::<PyType>()
.and_then(|cls| {
if cls.fast_issubclass(vm.ctx.exceptions.base_exception_type) {
Ok(Self::Class(cls))
} else {
Err(cls.into())
}
})
.or_else(|obj| obj.downcast::<PyBaseException>().map(Self::Instance))
.map_err(|obj| {
vm.new_type_error(format!(
"exceptions must be classes or instances deriving from BaseException, not {}",
obj.class().name()
))
})
}
}- Lines 445-480 (instantiating exception instances/values)
impl ExceptionCtor {
pub fn instantiate(self, vm: &VirtualMachine) -> PyResult<PyBaseExceptionRef> {
match self {
Self::Class(cls) => vm.invoke_exception(&cls, vec![]),
Self::Instance(exc) => Ok(exc),
}
}
pub fn instantiate_value(
self,
value: PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<PyBaseExceptionRef> {
let exc_inst = value.clone().downcast::<PyBaseException>().ok();
match (self, exc_inst) {
// both are instances; which would we choose?
(Self::Instance(_exc_a), Some(_exc_b)) => {
Err(vm.new_type_error("instance exception may not have a separate value"))
}
// if the "type" is an instance and the value isn't, use the "type"
(Self::Instance(exc), None) => Ok(exc),
// if the value is an instance of the type, use the instance value
(Self::Class(cls), Some(exc)) if exc.fast_isinstance(&cls) => Ok(exc),
// otherwise; construct an exception of the type using the value as args
(Self::Class(cls), _) => {
let args = match_class!(match value {
PyNone => vec![],
tup @ PyTuple => tup.to_vec(),
exc @ PyBaseException => exc.args().to_vec(),
obj => vec![obj],
});
vm.invoke_exception(&cls, args)
}
}
}
}- Lines 1645-1667 (exception payload fields)
pub struct PyBaseException {
pub(super) traceback: PyRwLock<Option<PyTracebackRef>>,
pub(super) cause: PyRwLock<Option<PyRef<Self>>>,
pub(super) context: PyRwLock<Option<PyRef<Self>>>,
pub(super) suppress_context: AtomicCell<bool>,
pub(super) args: PyRwLock<PyTupleRef>,
}
impl PyBaseException {
pub fn get_arg(&self, idx: usize) -> Option<PyObjectRef> {
self.args.read().get(idx).cloned()
}
}- Lines 3043-3106 (
ExceptionGroupmatching used by bytecodeCHECK_EG_MATCH)
pub(crate) fn exception_group_match(
exc_value: &PyObjectRef,
match_type: &PyObjectRef,
vm: &VirtualMachine,
) -> PyResult<(PyObjectRef, PyObjectRef)> {
// Implements _PyEval_ExceptionGroupMatch
// If exc_value is None, return (None, None)
if vm.is_none(exc_value) {
return Ok((vm.ctx.none(), vm.ctx.none()));
}
// Validate match_type and reject ExceptionGroup/BaseExceptionGroup
check_except_star_type_valid(match_type, vm)?;
// Check if exc_value matches match_type
if exc_value.is_instance(match_type, vm)? {
// Full match of exc itself
let is_eg = exc_value.fast_isinstance(vm.ctx.exceptions.base_exception_group);
let matched = if is_eg {
exc_value.clone()
} else {
// Naked exception - wrap it in ExceptionGroup
let excs = vm.ctx.new_tuple(vec![exc_value.clone()]);
let eg_type: PyObjectRef = crate::exception_group::exception_group().to_owned().into();
let wrapped = eg_type.call((vm.ctx.new_str(""), excs), vm)?;
// Copy traceback from original exception
if let Ok(exc) = exc_value.clone().downcast::<types::PyBaseException>()
&& let Some(tb) = exc.__traceback__()
&& let Ok(wrapped_exc) = wrapped.clone().downcast::<types::PyBaseException>()
{
let _ = wrapped_exc.set___traceback__(tb.into(), vm);
}
wrapped
};
return Ok((vm.ctx.none(), matched));
}
// Check for partial match if it's an exception group
if exc_value.fast_isinstance(vm.ctx.exceptions.base_exception_group) {
let pair = vm.call_method(exc_value, "split", (match_type.clone(),))?;
if !pair.class().is(vm.ctx.types.tuple_type) {
return Err(vm.new_type_error(format!(
"{}.split must return a tuple, not {}",
exc_value.class().name(),
pair.class().name()
)));
}
let pair_tuple: PyTupleRef = pair.try_into_value(vm)?;
if pair_tuple.len() < 2 {
return Err(vm.new_type_error(format!(
"{}.split must return a 2-tuple, got tuple of size {}",
exc_value.class().name(),
pair_tuple.len()
)));
}
let matched = pair_tuple[0].clone();
let rest = pair_tuple[1].clone();
return Ok((rest, matched));
}
// No match
Ok((exc_value.clone(), vm.ctx.none()))
}- Lines 3110-3175 (
prep_reraise_starused byPREP_RERAISE_STAR)
pub fn prep_reraise_star(orig: PyObjectRef, excs: PyObjectRef, vm: &VirtualMachine) -> PyResult {
use crate::builtins::PyList;
let excs_list = excs
.downcast::<PyList>()
.map_err(|_| vm.new_type_error("expected list for prep_reraise_star"))?;
let excs_vec: Vec<PyObjectRef> = excs_list.borrow_vec().to_vec();
// If no exceptions to process, return None
if excs_vec.is_empty() {
return Ok(vm.ctx.none());
}
// Special case: naked exception (not an ExceptionGroup)
// Only one except* clause could have executed, so there's at most one exception to raise
if !orig.fast_isinstance(vm.ctx.exceptions.base_exception_group) {
// Find first non-None exception
let first = excs_vec.into_iter().find(|e| !vm.is_none(e));
return Ok(first.unwrap_or_else(|| vm.ctx.none()));
}
// Split excs into raised (new) and reraised (from original) by comparing metadata
let mut raised: Vec<PyObjectRef> = Vec::new();
let mut reraised: Vec<PyObjectRef> = Vec::new();
for exc in excs_vec {
if vm.is_none(&exc) {
continue;
}
// Check if this exception came from the original group
if is_exception_from_orig(&exc, &orig, vm) {
reraised.push(exc);
} else {
raised.push(exc);
}
}
// If no exceptions to reraise, return None
if raised.is_empty() && reraised.is_empty() {
return Ok(vm.ctx.none());
}
// Project reraised exceptions onto original structure to preserve nesting
let reraised_eg = exception_group_projection(&orig, &reraised, vm)?;
// If no new raised exceptions, just return the reraised projection
if raised.is_empty() {
return Ok(reraised_eg);
}
// Combine raised with reraised_eg
if !vm.is_none(&reraised_eg) {
raised.push(reraised_eg);
}
// If only one exception, return it directly
if raised.len() == 1 {
return Ok(raised.into_iter().next().unwrap());
}
// Create new ExceptionGroup for multiple exceptions
let excs_tuple = vm.ctx.new_tuple(raised);
let eg_type: PyObjectRef = crate::exception_group::exception_group().to_owned().into();
eg_type.call((vm.ctx.new_str(""), excs_tuple), vm)
}crates/vm/src/vm/mod.rs
- Lines 780-939 (thread datastack allocation used by frame localsplus)
impl VirtualMachine {
/// Bump-allocate `size` bytes from the thread data stack.
///
/// # Safety
/// The returned pointer must be freed by calling `datastack_pop` in LIFO order.
#[inline(always)]
pub(crate) fn datastack_push(&self, size: usize) -> *mut u8 {
unsafe { (*self.datastack.get()).push(size) }
}
/// Check whether the thread data stack currently has room for `size` bytes.
#[inline(always)]
pub(crate) fn datastack_has_space(&self, size: usize) -> bool {
unsafe { (*self.datastack.get()).has_space(size) }
}
/// Pop a previous data stack allocation.
///
/// # Safety
/// `base` must be a pointer returned by `datastack_push` on this VM,
/// and all allocations made after it must already have been popped.
#[inline(always)]
pub(crate) unsafe fn datastack_pop(&self, base: *mut u8) {
unsafe { (*self.datastack.get()).pop(base) }
}
/// Temporarily detach the current thread (ATTACHED → DETACHED) while
/// running `f`, then re-attach afterwards. Allows `stop_the_world` to
/// park this thread during blocking syscalls.
#[inline]
pub fn allow_threads<R>(&self, f: impl FnOnce() -> R) -> R {
thread::allow_threads(self, f)
}
}| if !materialized_ptr.is_null() { | ||
| // Frame was materialized (observed by traceback/sys._getframe). | ||
| // Drop the leaked ref from materialization. | ||
| let materialized_ref: FrameRef = (&*materialized_ptr).to_owned(); | ||
| drop(materialized_ref); | ||
| } |
| // Run the bytecode | ||
| let lasti_ref: &rustpython_common::atomic::PyAtomic<u32> = | ||
| &*(&(*light).lasti as *const core::sync::atomic::AtomicU32 | ||
| as *const rustpython_common::atomic::PyAtomic<u32>); | ||
| let code_ref = (*(*light).code).to_owned(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- PyAtomic definitions and cfg gates ---'
rg -n -C 8 'PyAtomic|type .*Atomic|struct .*Atomic' crates --glob '*.rs' | head -n 280
printf '%s\n' '--- LightFrame declaration and lasti uses ---'
rg -n -C 12 '\bstruct\s+LightFrame\b|\blasti\b' crates/vm/src --glob '*.rs' | head -n 360
printf '%s\n' '--- workspace threading feature declarations ---'
rg -n -C 4 '^\s*threading\s*=|feature.*threading|\bthreading\b' Cargo.toml crates --glob 'Cargo.toml' | head -n 240Length of output: 46644
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate atomic and frame files ---'
fd -t f -e rs 'atomic|frame' crates/common crates/vm | sort
printf '%s\n' '--- exact PyAtomic definition ---'
rg -n -C 24 'pub\s+(type|use)\s+PyAtomic\b|PyAtomic\s*[=<]' crates/common --glob '*.rs'
printf '%s\n' '--- exact LightFrame definition and runner signature ---'
rg -n -C 28 '\bstruct\s+LightFrame\b|fn\s+run_light_frame\b|impl\s+LightFrame\b' crates/vm/src/frame.rs
printf '%s\n' '--- package feature wiring relevant to wasm and common ---'
sed -n '1,80p' crates/wasm/Cargo.toml
sed -n '1,55p' crates/common/Cargo.toml
sed -n '1,45p' crates/vm/Cargo.tomlLength of output: 20133
@fanninpm, confirmed.
PyAtomic<u32> is feature-dependent in crates/common/src/atomic.rs:
- with
threading:AtomicU32 - without
threading:core::cell::Cell<u32>
But LightFrame::lasti is unconditionally declared as core::sync::atomic::AtomicU32 (in crates/vm/src/frame.rs, Line 72), then cast to &PyAtomic<u32> in the new call path. The wasm package explicitly builds the VM without the threading feature, so that cast becomes effectively:
&AtomicU32 -> &Cell<u32>That is unsound. A matching size/alignment today would not make it valid: the types have no guaranteed layout equivalence, and Cell operations perform interior mutation through a reference that does not actually point to a Cell.
The appropriate fix is to make LightFrame::lasti a PyAtomic<u32>, initialize it with Radium::new(0), and pass &(*light).lasti directly—removing the cast entirely.
One wording refinement: the unsafe raw-pointer cast likely bypasses a normal compile-time type mismatch, so I can confirm the non-threading unsoundness rather than specifically a guaranteed build failure.
| // Recursion depth check | ||
| if vm.current_recursion_depth() >= vm.recursion_limit.get() { | ||
| // Clean up the light frame TLS before erroring | ||
| crate::vm::thread::set_current_light_frame(prev_light); | ||
| // Drop values we moved into localsplus |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '=== Native-stack guard call sites and recursion helpers ==='
rg -n -C 5 '\bcheck_c_stack_overflow\b|\bwith_recursion\b|\bwith_frame\b' crates/vm --glob '*.rs'
printf '%s\n' '=== Light-frame runner definitions/call sites ==='
rg -n -C 5 '\brun_light_frame\s*\(' crates/vm --glob '*.rs'
printf '%s\n' '=== Light-frame invocation and recursion-depth operations ==='
rg -n -C 4 '\binvoke_light_slots\s*\(|\brecursion_depth_(increment|decrement)\s*\(' crates/vm --glob '*.rs'Length of output: 34152
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '=== All tracked definitions/references related to light frames ==='
git grep -n -E 'run_light_frame|LightFrame|check_c_stack_overflow' -- '*.rs' || true
printf '%s\n' '=== Relevant VirtualMachine frame-helper implementation ==='
sed -n '1690,1775p' crates/vm/src/vm/mod.rs
printf '%s\n' '=== Complete light-frame recursion-check and execution region ==='
sed -n '850,930p' crates/vm/src/builtins/function.rs
printf '%s\n' '=== Source-file locations that may supply included/generated frame code ==='
rg -n -C 2 'include!|mod .*frame|pub.*frame' crates/vm/src --glob '*.rs'Length of output: 32072
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '=== run_light_frame implementation ==='
sed -n '2325,2415p' crates/vm/src/frame.rs
printf '%s\n' '=== Calls made by run_light_frame (for recursion/frame-helper delegation) ==='
sed -n '2334,2415p' crates/vm/src/frame.rs | rg -n -C 2 '\b(with_frame|with_recursion|check_recursive_call|run_frame|run|execute)\b' || trueLength of output: 3377
@fanninpm, confirmed.
PyFunction::invoke_light_slots checks only recursion_depth >= recursion_limit before incrementing the depth. It then calls run_light_frame, which directly executes ExecutingFrame::run and does not delegate to with_frame, with_recursion, or any other C-stack guard.
In contrast, VirtualMachine::with_frame performs the amortized native-stack probe:
let depth = self.recursion_depth.get();
if depth & 63 == 0 && self.check_c_stack_overflow() {
return Err(self.new_recursion_error(String::new()));
}Therefore the light-frame fast path bypasses the native-stack protection. With an increased sys.setrecursionlimit(), sufficiently deep recursive light-frame calls may exhaust the native stack rather than reliably raising RecursionError.
The appropriate fix is to add the same amortized check to invoke_light_slots before recursion_depth_increment() (exposing a crate-visible VM helper if needed).
| self.recursion_depth.update(|d| d + 1); | ||
|
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
crates/vm/src/vm/mod.rs:1732
self.recursion_depth.update(|d| d + 1)happens before anyscopeguard::defer!is installed. On some cfgs, the intervening operations (notablypush_thread_frame(...).lock().push(...)on non-unix threading builds) can panic (e.g. allocation failure), which would leak the incremented recursion depth and potentially break subsequent recursion checks. To keep invariants panic-safe, consider either (a) installing a recursion-depth-only defer immediately after increment, or (b) moving the increment down so it occurs after the fallible setup and immediately before the existingscopeguard::defer!block.
self.recursion_depth.update(|d| d + 1);
// Publish the frame for sys._current_frames() and faulthandler.
#[cfg(all(not(unix), feature = "threading"))]
crate::vm::thread::push_thread_frame(FramePtr(NonNull::from(&*frame)));
// Link frame into the signal-safe frame chain.
| // Generator/coroutine code and tracing must use the heavy path | ||
| if code.flags.intersects( | ||
| bytecode::CodeFlags::GENERATOR | ||
| | bytecode::CodeFlags::COROUTINE | ||
| | bytecode::CodeFlags::ASYNC_GENERATOR, | ||
| ) || vm.use_tracing.get() | ||
| { | ||
| return self.invoke_exact_args_slots(args, vm); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '=== Relevant symbols ==='
rg -n -C 5 \
-e 'struct FrameLocals\b' \
-e 'impl FrameLocals\b' \
-e 'fn lazy\b' \
-e 'pub.*fn new\(' \
-e '\binvoke_light_slots\s*\(' \
crates/vm/src
printf '%s\n' '=== Frame module outline ==='
ast-grep outline crates/vm/src/frame.rs --items all --type class,function,method --match 'Frame|FrameLocals|LightFrame' || true
printf '%s\n' '=== Function module call contexts ==='
rg -n -C 12 '\binvoke_light_slots\s*\(' crates/vm/src/builtins/function.rs crates/vm/srcLength of output: 70868
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '=== Frame locals and Frame::new ==='
sed -n '897,970p' crates/vm/src/frame.rs
sed -n '1220,1305p' crates/vm/src/frame.rs
printf '%s\n' '=== Exact-call specialization dispatch and guard setup ==='
sed -n '5340,5490p' crates/vm/src/frame.rs
rg -n -C 8 \
-e 'CallPythonExactArgs' \
-e 'CallBoundMethodExactArgs' \
-e 'can_specialize_call\s*\(' \
-e 'is_optimized_for_call_specialization\s*\(' \
crates/vm/src/frame.rs
printf '%s\n' '=== Code flag mutation/validation surfaces ==='
rg -n -C 8 \
-e 'CodeFlags' \
-e 'co_flags' \
-e 'replace\(' \
-e 'PyCodeNew' \
crates/vm/src/builtins/code.rs crates/vm/src
printf '%s\n' '=== Tests using code.replace or FunctionType ==='
rg -n -C 5 \
-e '\.replace\s*\(' \
-e 'FunctionType\s*\(' \
-e 'types\.CodeType' \
-e 'co_flags' \
Lib test crates \
-g '*.py' -g '*.rs' || trueLength of output: 192102
@fanninpm, confirmed — the finding is valid.
FrameLocals::lazy() is explicitly the CO_NEWLOCALS representation. In contrast, the normal Frame::new path uses the function’s globals as the locals mapping when CO_NEWLOCALS is absent:
None if code.flags.contains(CodeFlags::NEWLOCALS) => FrameLocals::lazy(),
None => FrameLocals::with_locals(ArgMapping::from_dict_exact(scope.globals.clone())),invoke_light_slots always selects FrameLocals::lazy(), while its current fallback only covers generator/coroutine code and tracing. The call-specialization eligibility also requires CO_OPTIMIZED but not CO_NEWLOCALS.
This state is reachable because RustPython’s code.replace(co_flags=...) and code-object constructor accept supplied flags without enforcing CO_OPTIMIZED => CO_NEWLOCALS. Consequently, a function with CO_OPTIMIZED but without CO_NEWLOCALS can be specialized and then observe an empty lazy locals dictionary instead of its globals mapping.
The fix should require CO_NEWLOCALS for the light-frame path and fall back to invoke_exact_args_slots otherwise. Adding that condition to can_specialize_call avoids creating the specialization; keeping a defensive check in invoke_light_slots would also protect current and future callers.
| // Link frame into the signal-safe frame chain. | ||
| let old_frame = crate::vm::thread::set_current_frame((&**frame) as *const Frame); | ||
| frame.previous.store( | ||
| old_frame as *mut Frame, | ||
| core::sync::atomic::Ordering::Relaxed, | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f 'frame\.rs|mod\.rs|thread\.rs' crates/vm/src | sort
ast-grep outline crates/vm/src/builtins/frame.rs --items all --view expanded
ast-grep outline crates/vm/src/frame.rs --items all --view expanded
ast-grep outline crates/vm/src/vm/thread.rs --items all --view expanded
rg -n -C 5 -P '\b(frame_at_offset_vm|find_owned_chain_frame|retained_back|f_back|with_frame|resume_gen_frame|Light)\b' \
crates/vm/src/builtins/frame.rs crates/vm/src/frame.rs crates/vm/src/vm crates/vm/src \
|| trueLength of output: 79142
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- crates/vm/src/frame.rs: materialization and unified walk ---'
sed -n '145,405p' crates/vm/src/frame.rs
echo '--- crates/vm/src/builtins/frame.rs: f_back ---'
sed -n '720,755p' crates/vm/src/builtins/frame.rs
echo '--- crates/vm/src/builtins/function.rs: light-frame invocation ---'
sed -n '840,965p' crates/vm/src/builtins/function.rs
echo '--- all light-frame state access and heavy-frame entry call sites ---'
rg -n -C 4 -P '\b(get_current_light_frame|set_current_light_frame|saved_current_frame|run_frame\s*\(|with_frame\s*\(|ensure_heavy\s*\()' \
crates/vm/src --glob '*.rs'Length of output: 34501
@fanninpm, confirmed — the concern is valid.
frame_at_offset_vm explicitly models this case: it walks “heavy frames pushed by native callbacks from within a light frame” before returning the active light frame. So the unified stack can be:
heavy callback frame → light caller → earlier heavy frame
But VirtualMachine::with_frame sets the callback frame’s previous pointer from set_current_frame, which is only the prior heavy frame. Py<Frame>::f_back follows only that pointer through find_owned_chain_frame / retained_back; neither path consults or materializes CURRENT_LIGHT_FRAME.
Therefore, when a heavy frame is entered while a light frame is active, sys._getframe() can report the light frame as its logical caller, while heavy_frame.f_back skips it. That breaks the expected adjacent-frame invariant.
The fix needs to materialize/retain the active light frame as the heavy callback frame’s back-link at the heavy-frame entry boundary, or the affected CPython test needs an explicit temporary expected-failure marker if this remains an acknowledged limitation.
|
@fanninpm Thank you so much, it is unexpectedly very helpful 😂 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
crates/vm/src/vm/mod.rs:1727
with_frame()incrementsrecursion_depthbefore installing thescopeguard::defer!that decrements it. A panic between the increment and the defer installation (e.g., duringmaterialize_light_frame_pub/Frame::newallocations) would leakrecursion_depth, potentially causing spuriousRecursionErrorlater in the thread.
self.recursion_depth.update(|d| d + 1);
| /// The current thread's topmost frame object, if any. | ||
| /// If light frames are active, they are on top of the heavy chain. |
| /// Public wrapper for `materialize_light_frame`. | ||
| /// | ||
| /// # Safety | ||
| /// `light` must point to a valid LightFrame whose borrowed pointers are still alive. | ||
| pub unsafe fn materialize_light_frame_pub(light: *mut LightFrame, vm: &VirtualMachine) -> FrameRef { |
|
the implementation direction is wrong. reworking |
2ed7ca5 to
e73a11c
Compare
| *frame.retained_back.lock() = frame | ||
| .previous_frame() | ||
| .as_heavy() | ||
| .and_then(|h| unsafe { owned_chain_frame(h) }); |
| # Complete `rustpython-unicode` isolation: case mapping, casing predicates, and sre case/space parity | ||
|
|
||
| Follow-up to #7560, continuing from #8211 (merged). |
| Forwards Claude Code hook/statusline events to the gateway (ANTHROPIC_BASE_URL) | ||
| and acts on the JSON it returns: injects the gateway's `context` as non-blocking | ||
| additional context (no yellow "blocked by hook" UI); on a closed task uploads the | ||
| named transcripts and bundles them; on an auth failure (re)creates | ||
| settings.local.json and blocks with fix-it steps instead of a raw 401. All wording |
| "mode": "merge", | ||
| "target": "/Users/youknowone/Projects/RustPython-11/.claude", | ||
| "timestamp": "2026-07-22T12:47:24Z", |
e73a11c to
f6bae97
Compare
| while !cur.is_null() && depth < MAX_FRAME_DEPTH { | ||
| if let Some(heavy) = cur.as_heavy() { | ||
| let frame = unsafe { &*heavy }; | ||
| dump_frame_from_raw(fd, frame); | ||
| depth += 1; | ||
| } | ||
| cur = unsafe { cur.next() }; | ||
| } |
| // Light frame predecessor: materialize it | ||
| if let Some(light) = chain.as_light() { | ||
| let frame = unsafe { crate::frame::materialize_light_frame_pub(light, vm) }; | ||
| frame.mark_escaped(); | ||
| return Some(frame); | ||
| } |
f6bae97 to
f1395e4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
crates/vm/src/frame.rs:407
current_thread_frame()now explicitly skips light frames. This function is used byVirtualMachine::current_frame()(and many call sites such as builtins that rely on the actual current Python frame). With light frames enabled, those APIs will observe the wrong caller frame whenever the top of stack is aLightFrame.
To preserve existing semantics, current_thread_frame() should materialize the light frame when the current VM is available (via TLS), and only fall back to the heavy-only behavior when no VM is attached.
/// The current thread's topmost heavy frame object, if any.
/// Skips light frames — use `current_thread_frame_vm` to materialize them.
#[must_use]
pub fn current_thread_frame() -> Option<FrameRef> {
let mut cur = crate::vm::thread::get_current_frame();
| pub fn set_current_frame(chain: FrameChainPtr) -> FrameChainPtr { | ||
| // Publish the top heavy frame for cross-thread readers (signal safety). | ||
| // Only heavy frames are published — light frames cannot be safely | ||
| // dereferenced from a signal handler on another thread. | ||
| #[cfg(all(unix, feature = "threading"))] | ||
| { | ||
| if let Some(heavy) = chain.as_heavy() { |
| let payload: *const Frame = &***frame; | ||
| let old_chain = | ||
| crate::vm::thread::set_current_frame(crate::frame::FrameChainPtr::from_heavy(payload)); | ||
| { | ||
| #[allow(unused_imports)] | ||
| use rustpython_common::atomic::Radium; | ||
| frame | ||
| .previous | ||
| .store(old_chain.raw(), core::sync::atomic::Ordering::Relaxed); | ||
| } |
f1395e4 to
bc4425b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
crates/vm/src/vm/thread.rs:684
set_current_frameupdatesThreadSlot::top_frameonly whenchainis heavy. When a heavy frame returns to a light frame (or to null),top_framecan remain pointing at the just-popped heavy frame, sosys._current_frames()/ GC assertions may read a dangling pointer even under stop-the-world. Publish the nearest heavy frame reachable fromchain(or null) instead of leavingtop_frameunchanged whenchainis light.
// Publish the top heavy frame for cross-thread readers (signal safety).
// Only heavy frames are published — light frames cannot be safely
// dereferenced from a signal handler on another thread.
#[cfg(all(unix, feature = "threading"))]
if let Some(heavy) = chain.as_heavy() {
crates/vm/src/builtins/frame.rs:736
f_backmaterializes a light predecessor immediately. If theFrameobject is accessed from another thread (e.g. viasys._current_frames()), this can race with the owning thread mutating the light frame’s DataStack-backed locals/state, causing a data race/UB. Only materialize directly when the light entry is on the current thread’s chain; otherwise, on unix+threading, materialize under stop-the-world (or return None on platforms without stop-the-world).
// Light frame predecessor: materialize it
if let Some(light) = chain.as_light() {
let frame = unsafe { crate::frame::materialize_light_frame_pub(light, vm) };
frame.mark_escaped();
return Some(frame);
| #[cfg(unix)] | ||
| top_frame: AtomicPtr::new(get_current_frame() as *mut Frame), | ||
| top_frame: AtomicPtr::new( | ||
| get_current_frame().as_heavy().unwrap_or(core::ptr::null()) as *mut Frame | ||
| ), |
bc4425b to
9c7476d
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/vm/src/builtins/frame.rs (2)
500-513: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the live instruction position for the initial
f_linenocheck.Line 501 reads
self.lasti()beforefind_live_source_iframe(). A materializedFrameObjectcan retainlasti == 0while its liveInterpreterFramehas advanced. The getter then returns the first line and skips the liveprev_linepath.Resolve the live position before the zero check and reuse it.
Proposed fix
- // If lasti is 0, execution hasn't started yet - use first line number - if self.lasti() == 0 { + let live = self.find_live_source_iframe(); + let current_lasti = if !live.is_null() { + unsafe { (*live).lasti.load(Relaxed) } + } else { + self.lasti() + }; + // If lasti is 0, execution hasn't started yet - use first line number + if current_lasti == 0 { return self .iframe() .code() @@ - let live = self.find_live_source_iframe(); if !live.is_null() {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/vm/src/builtins/frame.rs` around lines 500 - 513, Update the f_lineno logic in the frame getter to call find_live_source_iframe() before checking whether the instruction position is zero, and use that live frame position for the initial check. Reuse the resolved live frame for the existing prev_line path so a materialized FrameObject cannot incorrectly return first_line_number when execution has advanced.
448-467: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd lifetime synchronization guards to
find_live_source_iframe().
find_live_source_iframe()returns a rawInterpreterFramefrom the current thread’s chain, and callers accessprev_lineandlocalsplusthrough that pointer without stop-the-world or synchronized-field protection. Keep this API scoped to callers that already guarantee frame lifetime plus execution barriers, or add explicit synchronization for each read/write site.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/vm/src/builtins/frame.rs` around lines 448 - 467, Restrict find_live_source_iframe to callers that already guarantee the InterpreterFrame remains live and execution is synchronized, or add the required stop-the-world and synchronized-field guards at every site that dereferences its returned pointer, including prev_line and localsplus accesses. Ensure no raw frame pointer is read or written without these lifetime and execution barriers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/vm/src/builtins/frame.rs`:
- Around line 500-513: Update the f_lineno logic in the frame getter to call
find_live_source_iframe() before checking whether the instruction position is
zero, and use that live frame position for the initial check. Reuse the resolved
live frame for the existing prev_line path so a materialized FrameObject cannot
incorrectly return first_line_number when execution has advanced.
- Around line 448-467: Restrict find_live_source_iframe to callers that already
guarantee the InterpreterFrame remains live and execution is synchronized, or
add the required stop-the-world and synchronized-field guards at every site that
dereferences its returned pointer, including prev_line and localsplus accesses.
Ensure no raw frame pointer is read or written without these lifetime and
execution barriers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: b7c36c19-18b8-431c-a737-d1cf8b609678
📒 Files selected for processing (3)
crates/vm/src/builtins/frame.rscrates/vm/src/frame.rscrates/vm/src/vm/mod.rs
💤 Files with no reviewable changes (1)
- crates/vm/src/frame.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/vm/src/vm/mod.rs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/vm/src/builtins/frame.rs (1)
448-527: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the live
lastifor the initialf_linenocheck.
f_linenoreturns the first line whenself.lasti() == 0at Lines [500]-[502]. For a materialized live frame, execution updates the live iframe, while the materializedlastiis synchronized after execution. Therefore, the live read added at Lines [578]-[583] can be skipped while the cached value remains zero.Compute
current_lastifromfind_live_source_iframe()before the early return and use it for both checks. Add a regression test that readsframe.f_linenoduring execution.Also applies to: 571-583
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/vm/src/builtins/frame.rs` around lines 448 - 527, Update FrameObject::f_lineno to resolve the live source iframe and derive current_lasti before the initial zero check, using the live InterpreterFrame::lasti when available and the materialized value otherwise. Use current_lasti for the early first-line decision and preserve the existing live prev_line and returned-frame location behavior. Add a regression test that reads frame.f_lineno while the frame is executing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/vm/src/builtins/frame.rs`:
- Around line 448-527: Update FrameObject::f_lineno to resolve the live source
iframe and derive current_lasti before the initial zero check, using the live
InterpreterFrame::lasti when available and the materialized value otherwise. Use
current_lasti for the early first-line decision and preserve the existing live
prev_line and returned-frame location behavior. Add a regression test that reads
frame.f_lineno while the frame is executing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: ab0b5b55-ac35-477b-b7c1-6e8e5e34c15c
📒 Files selected for processing (3)
crates/vm/src/builtins/frame.rscrates/vm/src/frame.rscrates/vm/src/vm/mod.rs
💤 Files with no reviewable changes (1)
- crates/vm/src/frame.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/vm/src/vm/mod.rs
Assisted-by: Claude
ShaharNaveh
left a comment
There was a problem hiding this comment.
Wow, that cut the cpython tests time from 22m± -> 18m±
amazing
Assisted-by: Claude
fire_stop_iteration was passing the raw iterator return value to callbacks, but the STOP_ITERATION event callback signature expects a StopIteration exception instance. Wrap non-StopIteration values in a new StopIteration(value), matching PyMonitoring_FireStopIterationEvent. This fixes test_pdb_await_support where bdb's exception_callback received None instead of a StopIteration instance. Assisted-by: Claude
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/vm/src/vm/mod.rs`:
- Around line 1764-1776: When an escaped frame (strong > 1) is detected, ensure
the predecessor InterpreterFrame referenced by old_chain is materialized before
attempting to set retained_back. The current logic only sets retained_back if
prev_iframe.frame_obj() returns Some, but for unmaterialized light
InterpreterFrames this method returns None, causing f_back to be lost after the
caller returns. Materialize prev_iframe when strong > 1 and the predecessor is
not yet materialized, so that retained_back captures the predecessor correctly
and preserves f_back beyond the caller's execution lifetime, consistent with how
with_iframe handles this scenario.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ebf7152-5c9a-457a-8849-c7c42c6bfd97
📒 Files selected for processing (5)
.gitignorecrates/vm/src/builtins/frame.rscrates/vm/src/frame.rscrates/vm/src/stdlib/sys/monitoring.rscrates/vm/src/vm/mod.rs
💤 Files with no reviewable changes (2)
- .gitignore
- crates/vm/src/frame.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/vm/src/builtins/frame.rs
| // Only set retained_back if someone else holds a reference (escaped) | ||
| // AND the caller already has a FrameObject. Materializing the caller | ||
| // here would add refcounts on its local variables, preventing timely | ||
| // __del__ / ResourceWarning on dealloc. If the caller hasn't been | ||
| // materialized, f_back will resolve via the TLS chain while the | ||
| // caller is still executing, or return None after it returns. | ||
| if strong > 1 { | ||
| let mut guard = frame.iframe().retained_back.lock(); | ||
| if guard.is_none() { | ||
| let prev_iframe = unsafe { &*old_chain }; | ||
| if let Some(fo) = prev_iframe.frame_obj() { | ||
| *guard = Some(fo.to_owned()); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve f_back after the caller returns.
Line 1774 skips retention when old_chain is an unmaterialized light InterpreterFrame. If an exec() or eval() frame escapes, its f_back works only while its caller executes and becomes None after the caller returns. Materialize and retain the predecessor for escaped frames, as with_iframe already does.
Proposed fix
- if let Some(fo) = prev_iframe.frame_obj() {
- *guard = Some(fo.to_owned());
- }
+ *guard = Some(prev_iframe.materialize_chain(self));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Only set retained_back if someone else holds a reference (escaped) | |
| // AND the caller already has a FrameObject. Materializing the caller | |
| // here would add refcounts on its local variables, preventing timely | |
| // __del__ / ResourceWarning on dealloc. If the caller hasn't been | |
| // materialized, f_back will resolve via the TLS chain while the | |
| // caller is still executing, or return None after it returns. | |
| if strong > 1 { | |
| let mut guard = frame.iframe().retained_back.lock(); | |
| if guard.is_none() { | |
| let prev_iframe = unsafe { &*old_chain }; | |
| if let Some(fo) = prev_iframe.frame_obj() { | |
| *guard = Some(fo.to_owned()); | |
| } | |
| // Only set retained_back if someone else holds a reference (escaped) | |
| // AND the caller already has a FrameObject. Materializing the caller | |
| // here would add refcounts on its local variables, preventing timely | |
| // __del__ / ResourceWarning on dealloc. If the caller hasn't been | |
| // materialized, f_back will resolve via the TLS chain while the | |
| // caller is still executing, or return None after it returns. | |
| if strong > 1 { | |
| let mut guard = frame.iframe().retained_back.lock(); | |
| if guard.is_none() { | |
| let prev_iframe = unsafe { &*old_chain }; | |
| *guard = Some(prev_iframe.materialize_chain(self)); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/vm/src/vm/mod.rs` around lines 1764 - 1776, When an escaped frame
(strong > 1) is detected, ensure the predecessor InterpreterFrame referenced by
old_chain is materialized before attempting to set retained_back. The current
logic only sets retained_back if prev_iframe.frame_obj() returns Some, but for
unmaterialized light InterpreterFrames this method returns None, causing f_back
to be lost after the caller returns. Materialize prev_iframe when strong > 1 and
the predecessor is not yet materialized, so that retained_back captures the
predecessor correctly and preserves f_back beyond the caller's execution
lifetime, consistent with how with_iframe handles this scenario.
|
@ShaharNaveh would it make sense to use the allow-by-default |
I would more than like to enable it! the only reason why I haven't enabled it already is because we have too many places that violates it, but using an AI tool to document it all/parts of it would be great |
Summary
Split
InterpreterFrame(execution state) fromFrameObject(Python-visibleframeobject) so that normal function calls stack-allocate only anInterpreterFrameon the Rust stack. A fullFrameObjectis created lazily viamaterialize()only when Python code observes the frame (e.g.sys._getframe(), traceback creation,sys.settrace()).This mirrors CPython 3.11+ where
_PyInterpreterFramelives on the C stack andPyFrameObjectis allocated on demand.Architecture
InterpreterFrame—#[repr(C)]struct on the Rust stack insidewith_iframe. Contains code, globals, builtins, localsplus, lasti, trace state, and apreviouspointer forming the TLS frame chain.FrameObject—PyObjectwrapper created bymaterialize()when needed. Holds an owned copy ofInterpreterFrameand is linked viamaterialized/find_live_source_iframe()for bidirectional access.with_iframe— new fast path for regular function calls. No heap allocation, no refcount, no freelist.with_frame— existing path for generators, coroutines,exec(),eval()that require a durableFrameObject.Key design decisions
current_code()and free-threading safety:current_code()reads the topmostInterpreterFramefrom thread-localCURRENT_FRAMEand borrows itscodepointer. This is safe because: (1)CURRENT_FRAMEis per-thread TLS — no cross-thread access; (2) thecodepointer borrows from thePyFunctionon the caller's stack, which is alive while the frame executes; (3).to_owned()increments the refcount before returning, producing an independentPyRef<PyCode>.Cross-thread frame access:
f_back,sys._current_frames(), andsys._current_exceptions()use stop-the-world (STW) to safely materialize cross-thread iframe chains. STW is entered before dereferencing any cross-thread pointer to prevent use-after-free races. The non-unix path now uses STW identically to unix, replacing the previousframesMutex fallback.set_f_lineno(debugger jump): Writeslasti,pending_stack_pops, andpending_unwind_from_stackto the live source iframe viafind_live_source_iframe(), not the materialized copy, so pdbjumpcommands take effect on stack-allocated frames.GC tracking timing: Materialized
FrameObjects are tracked in GC only atwith_iframecleanup afterset_current_framerestores the old chain. This prevents premature collection while the frame is still executing.retained_back: Only captures already-materialized callers to avoid adding refcounts on local variables (which would delay__del__/ResourceWarning). For non-materialized callers,f_backresolves via the TLS chain while executing, or returnsNoneafter return.Performance improvements
Zero heap allocation for normal function calls (no
FrameObject, no freelist, no refcount)Amortized C stack overflow check (every 8th recursion depth)
Panic-safe recursion depth via
scopeguardinwith_frameBenchmark results (Apple M-series, release build)
Test fixes included
test_sys(Windows): full frame chain materialization withretained_backin non-unixget_all_current_framestest_current_exceptions(Windows): STW-based cross-threadf_backon all platformstest_frame:frame.clear()rejects live frames,f_localsproxy writes to live iframetest_traceback: deferred GC tracking for materialized framestest_generators: removed@expectedFailurefor frame/GC cycle teststest_pdb:f_tracepropagation to live source iframetest_faulthandler: usetop_iframefor all frame chain walkingAddresses youknowone#40
Summary by CodeRabbit
Performance
Bug Fixes
sys._getframebehavior.