E5F0 Clear frame locals and stack on generator close + Add dir_fd support for rmdir, remove/unlink, scandir by youknowone · Pull Request #7222 · RustPython/RustPython · GitHub
[go: up one dir, main page]
Skip to content

Clear frame locals and stack on generator close + Add dir_fd support for rmdir, remove/unlink, scandir #7222

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 3 commits into from
Feb 26, 2026
Merged

Clear frame locals and stack on generator close + Add dir_fd support for rmdir, remove/unlink, scandir #7222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
055584c
Clear frame locals and stack on generator close
youknowone Feb 25, 2026
7e7832d
Add dir_fd support for rmdir, remove/unlink, scandir
youknowone Feb 24, 2026
a25d565
cells_free
youknowone Feb 26, 2026
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
2 changes: 1 addition & 1 deletion Lib/test/test_generators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,6 @@ def f():
with self.assertRaises(RuntimeError):
gen.close()

@unittest.expectedFailure # TODO: RUSTPYTHON; no deterministic GC finalization
def test_close_releases_frame_locals(self):
# See gh-118272

Expand All @@ -684,6 +683,7 @@ def genfn():

# See https://github.com/python/cpython/issues/125723
class GeneratorDeallocTest(unittest.TestCase):
@unittest.expectedFailure # TODO: RUSTPYTHON; frame uses shared Arc, no ownership transfer
def test_frame_outlives_generator(self):
def g1():
a = 42
10BC0 Expand Down
1 change: 0 additions & 1 deletion Lib/test/test_os.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5312,7 +5312,6 @@ def test_bytes_like(self):
with self.assertRaises(TypeError):
os.scandir(path_bytes)

@unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: <builtin_function_or_method object at 0xba3106920> not found in {<builtin_function_or_method object at 0xba31078e0>, <builtin_function_or_method object at 0xba31079c0>, <builtin_function_or_method object at 0xba3107b10>, <builtin_function_or_method object at 0xba3159500>, <builtin_function_or_method object at 0xba3159570>, <builtin_function_or_method object at 0xba3107800>, <builtin_function_or_method object at 0xba3106760>, <builtin_function_or_method object at 0xba3106a00>, <builtin_function_or_method object at 0xba3106990>, <builtin_function_or_method object at 0xba3107330>, <builtin_function_or_method object at 0xba31072c0>, <builtin_function_or_method object at 0xba31064c0>}
@unittest.skipUnless(os.listdir in os.supports_fd,
'fd support for listdir required for this test.')
def test_fd(self):
Expand Down
2 changes: 1 addition & 1 deletion crates/common/src/os.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ pub mod ffi {

fn from_bytes(slice: &[u8]) -> &OsStr {
// WASI strings are guaranteed to be UTF-8
OsStr::new(std::str::from_utf8(slice).expect("wasip2 strings are UTF-8"))
OsStr::new(core::str::from_utf8(slice).expect("wasip2 strings are UTF-8"))
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/vm/src/builtins/frame.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ impl Py<Frame> {
}
}

// Clear the evaluation stack
self.clear_value_stack();
// Clear the evaluation stack and cell references
self.clear_stack_and_cells();

// Clear temporary refs
self.temporary_refs.lock().clear();
Expand Down
2 changes: 1 addition & 1 deletion crates/vm/src/builtins/function.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ impl PyFunction {
if let Some(cell2arg) = code.cell2arg.as_deref() {
for (cell_idx, arg_idx) in cell2arg.iter().enumerate().filter(|(_, i)| **i != -1) {
let x = fastlocals[*arg_idx as usize].take();
frame.cells_frees[cell_idx].set(x);
frame.set_cell_contents(cell_idx, x);
}
}

Expand Down
12 changes: 7 additions & 5 deletions crates/vm/src/builtins/super.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,17 @@ impl Initializer for PySuper {
if frame.code.arg_count == 0 {
return Err(vm.new_runtime_error("super(): no arguments"));
}
let obj = frame.fastlocals.lock()[0]
.clone()
// Lock fastlocals briefly to get arg[0], then drop the guard
// before calling get_cell_contents (which also locks fastlocals).
let first_arg = frame.fastlocals.lock()[0].clone();
let obj = first_arg
.or_else(|| {
if let Some(cell2arg) = frame.code.cell2arg.as_deref() {
cell2arg[..frame.code.cellvars.len()]
.iter()
.enumerate()
.find(|(_, arg_idx)| **arg_idx == 0)
.and_then(|(cell_idx, _)| frame.cells_frees[cell_idx].get())
.and_then(|(cell_idx, _)| frame.get_cell_contents(cell_idx))
} else {
None
}
Expand All @@ -104,8 +106,8 @@ impl Initializer for PySuper {
for (i, var) in frame.code.freevars.iter().enumerate() {
if var.as_bytes() == b"__class__" {
let i = frame.code.cellvars.len() + i;
let class = frame.cells_frees[i]
.get()
let class = frame
.get_cell_contents(i)
.ok_or_else(|| vm.new_runtime_error("super(): empty __class__ cell"))?;
typ = Some(class.downcast().map_err(|o| {
vm.new_type_error(format!(
Expand Down
3 changes: 3 additions & 0 deletions crates/vm/src/coroutine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,9 @@ impl Coro {
)
});
self.closed.store(true);
// Release frame locals and stack to free references held by the
// closed generator, matching gen_send_ex2 with close_on_completion.
self.frame.clear_locals_and_stack();
match result {
Ok(ExecutionResult::Yield(_)) => {
Err(vm.new_runtime_error(format!("{} ignored GeneratorExit", gen_name(jen, vm))))
Expand Down
85 changes: 57 additions & 28 deletions crates/vm/src/frame.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ struct FrameState {
// We need 1 stack per frame
/// The main data frame of the stack machine
stack: BoxVec<Option<PyObjectRef>>,
/// Cell and free variable references (cellvars + freevars).
cells_frees: Box<[PyCellRef]>,
/// index of last instruction ran
#[cfg(feature = "threading")]
lasti: u32,
Expand Down Expand Up @@ -93,7 +95,6 @@ pub struct Frame {
pub func_obj: Option<PyObjectRef>,

pub fastlocals: PyMutex<Box<[Option<PyObjectRef>]>>,
pub(crate) cells_frees: Box<[PyCellRef]>,
pub locals: ArgMapping,
pub globals: PyDictRef,
pub builtins: PyObjectRef,
Expand Down Expand Up @@ -135,6 +136,7 @@ impl PyPayload for Frame {
unsafe impl Traverse for FrameState {
fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) {
self.stack.traverse(tracer_fn);
self.cells_frees.traverse(tracer_fn);
}
}

Expand All @@ -143,7 +145,6 @@ unsafe impl Traverse for Frame {
self.code.traverse(tracer_fn);
self.func_obj.traverse(tracer_fn);
self.fastlocals.traverse(tracer_fn);
self.cells_frees.traverse(tracer_fn);
self.locals.traverse(tracer_fn);
self.globals.traverse(tracer_fn);
self.builtins.traverse(tracer_fn);
Expand Down Expand Up @@ -193,13 +194,13 @@ impl Frame {

let state = FrameState {
stack: BoxVec::new(code.max_stackdepth as usize),
cells_frees,
#[cfg(feature = "threading")]
lasti: 0,
};

Self {
fastlocals: PyMutex::new(fastlocals_vec.into_boxed_slice()),
cells_frees,
locals: scope.locals,
globals: scope.globals,
builtins,
Expand All @@ -218,9 +219,38 @@ impl Frame {
}
}

/// Clear the evaluation stack. Used by frame.clear() to break reference cycles.
pub(crate) fn clear_value_stack(&self) {
self.state.lock().stack.clear();
/// Clear evaluation stack and state-owned cell/free references.
/// For full local/cell cleanup, call `clear_locals_and_stack()`.
pub(crate) fn clear_stack_and_cells(&self) {
let mut state = self.state.lock();
state.stack.clear();
let _old = core::mem::take(&mut state.cells_frees);
}

/// Clear locals and stack after generator/coroutine close.
/// Releases references held by the frame, matching _PyFrame_ClearLocals.
pub(crate) fn clear_locals_and_stack(&self) {
self.clear_stack_and_cells();
let mut fastlocals = self.fastlocals.lock();
for slot in fastlocals.iter_mut() {
*slot = None;
}
}

/// Get cell contents by cell index. Reads through fastlocals (no state lock needed).
pub(crate) fn get_cell_contents(&self, cell_idx: usize) -> Option<PyObjectRef> {
let nlocals = self.code.varnames.len();
let fastlocals = self.fastlocals.lock();
fastlocals
.get(nlocals + cell_idx)
.and_then(|slot| slot.as_ref())
.and_then(|obj| obj.downcast_ref::<PyCell>())
.and_then(|cell| cell.get())
}

/// Set cell contents by cell index. Only safe to call before frame execution starts.
pub(crate) fn set_cell_contents(&self, cell_idx: usize, value: Option<PyObjectRef>) {
self.state.lock().cells_frees[cell_idx].set(value);
}

/// Store a borrowed back-reference to the owning generator/coroutine.
Expand Down Expand Up @@ -296,23 +326,25 @@ impl Frame {
}
}
if !code.cellvars.is_empty() || !code.freevars.is_empty() {
let map_to_dict = |keys: &[&PyStrInterned], values: &[PyCellRef]| {
for (&k, v) in zip(keys, values) {
if let Some(value) = v.get() {
locals.mapping().ass_subscript(k, Some(value), vm)?;
} else {
match locals.mapping().ass_subscript(k, None, vm) {
Ok(()) => {}
Err(e) if e.fast_isinstance(vm.ctx.exceptions.key_error) => {}
Err(e) => return Err(e),
}
}
// Access cells through fastlocals to avoid locking state
// (state may be held by with_exec during frame execution).
for (i, &k) in code.cellvars.iter().enumerate() {
let cell_value = self.get_cell_contents(i);
match locals.mapping().ass_subscript(k, cell_value, vm) {
Ok(()) => {}
Err(e) if e.fast_isinstance(vm.ctx.exceptions.key_error) => {}
Err(e) => return Err(e),
}
Ok(())
};
map_to_dict(&code.cellvars, &self.cells_frees)?;
}
if code.flags.contains(bytecode::CodeFlags::OPTIMIZED) {
map_to_dict(&code.freevars, &self.cells_frees[code.cellvars.len()..])?;
for (i, &k) in code.freevars.iter().enumerate() {
let cell_value = self.get_cell_contents(code.cellvars.len() + i);
match locals.mapping().ass_subscript(k, cell_value, vm) {
Ok(()) => {}
Err(e) if e.fast_isinstance(vm.ctx.exceptions.key_error) => {}
Err(e) => return Err(e),
}
}
}
}
Ok(locals.clone())
Expand All @@ -326,7 +358,6 @@ impl Py<Frame> {
let exec = ExecutingFrame {
code: &self.code,
fastlocals: &self.fastlocals,
cells_frees: &self.cells_frees,
locals: &self.locals,
globals: &self.globals,
builtins: &self.builtins,
Expand Down Expand Up @@ -372,7 +403,6 @@ impl Py<Frame> {
let exec = ExecutingFrame {
code: &self.code,
fastlocals: &self.fastlocals,
cells_frees: &self.cells_frees,
locals: &self.locals,
globals: &self.globals,
builtins: &self.builtins,
Expand Down Expand Up @@ -407,7 +437,6 @@ impl Py<Frame> {
struct ExecutingFrame<'a> {
code: &'a PyRef<PyCode>,
fastlocals: &'a PyMutex<Box<[Option<PyObjectRef>]>>,
cells_frees: &'a [PyCellRef],
locals: &'a ArgMapping,
globals: &'a PyDictRef,
builtins: &'a PyObjectRef,
Expand Down Expand Up @@ -1011,7 +1040,7 @@ impl ExecutingFrame<'_> {
}
Instruction::DeleteAttr { idx } => self.delete_attr(vm, idx.get(arg)),
Instruction::DeleteDeref(i) => {
self.cells_frees[i.get(arg) as usize].set(None);
self.state.cells_frees[i.get(arg) as usize].set(None);
Ok(None)
}
Instruction::DeleteFast(idx) => {
Expand Down Expand Up @@ -1436,7 +1465,7 @@ impl ExecutingFrame<'_> {
};
self.push_value(match value {
Some(v) => v,
None => self.cells_frees[i]
None => self.state.cells_frees[i]
.get()
.ok_or_else(|| self.unbound_cell_exception(i, vm))?,
});
Expand Down Expand Up @@ -1493,7 +1522,7 @@ impl ExecutingFrame<'_> {
}
Instruction::LoadDeref(i) => {
let idx = i.get(arg) as usize;
let x = self.cells_frees[idx]
let x = self.state.cells_frees[idx]
.get()
.ok_or_else(|| self.unbound_cell_exception(idx, vm))?;
self.push_value(x);
Expand Down Expand Up @@ -2083,7 +2112,7 @@ impl ExecutingFrame<'_> {
Instruction::StoreAttr { idx } => self.store_attr(vm, idx.get(arg)),
Instruction::StoreDeref(i) => {
let value = self.pop_value();
self.cells_frees[i.get(arg) as usize].set(Some(value));
self.state.cells_frees[i.get(arg) as usize].set(Some(value));
Ok(None)
}
Instruction::StoreFast(idx) => {
Expand Down
Loading
Loading
0