Skip to content

Commit f9b42f2

Browse files
youknowoneyouknowone
committed
cells_free
1 parent 7e7832d commit f9b42f2

File tree

6 files changed

+60
-37
lines changed

6 files changed

+60
-37
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,8 @@ impl Py<Frame> {
179179
}
180180
}
181181

182-
// Clear the evaluation stack
183-
self.clear_value_stack();
182+
// Clear the evaluation stack and cell references
183+
self.clear_stack_and_cells();
184184

185185
// Clear temporary refs
186186
self.temporary_refs.lock().clear();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ impl PyFunction {
431431
if let Some(cell2arg) = code.cell2arg.as_deref() {
432432
for (cell_idx, arg_idx) in cell2arg.iter().enumerate().filter(|(_, i)| **i != -1) {
433433
let x = fastlocals[*arg_idx as usize].take();
434-
frame.cells_frees[cell_idx].set(x);
434+
frame.set_cell_contents(cell_idx, x);
435435
}
436436
}
437437

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ impl Initializer for PySuper {
9393
.iter()
9494
.enumerate()
9595
.find(|(_, arg_idx)| **arg_idx == 0)
96-
.and_then(|(cell_idx, _)| frame.cells_frees[cell_idx].get())
96+
.and_then(|(cell_idx, _)| frame.get_cell_contents(cell_idx))
9797
} else {
9898
None
9999
}
@@ -104,8 +104,8 @@ impl Initializer for PySuper {
104104
for (i, var) in frame.code.freevars.iter().enumerate() {
105105
if var.as_bytes() == b"__class__" {
106106
let i = frame.code.cellvars.len() + i;
107-
let class = frame.cells_frees[i]
108-
.get()
107+
let class = frame
108+
.get_cell_contents(i)
109109
.ok_or_else(|| vm.new_runtime_error("super(): empty __class__ cell"))?;
110110
typ = Some(class.downcast().map_err(|o| {
111111
vm.new_type_error(format!(

‎crates/vm/src/frame.rs‎

Lines changed: 47 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ struct FrameState {
5353
// We need 1 stack per frame
5454
/// The main data frame of the stack machine
5555
stack: BoxVec<Option<PyObjectRef>>,
56+
/// Cell and free variable references (cellvars + freevars).
57+
cells_frees: Box<[PyCellRef]>,
5658
/// index of last instruction ran
5759
#[cfg(feature = "threading")]
5860
lasti: u32,
@@ -93,7 +95,6 @@ pub struct Frame {
9395
pub func_obj: Option<PyObjectRef>,
9496

9597
pub fastlocals: PyMutex<Box<[Option<PyObjectRef>]>>,
96-
pub(crate) cells_frees: Box<[PyCellRef]>,
9798
pub locals: ArgMapping,
9899
pub globals: PyDictRef,
99100
pub builtins: PyObjectRef,
@@ -135,6 +136,7 @@ impl PyPayload for Frame {
135136
unsafe impl Traverse for FrameState {
136137
fn traverse(&self, tracer_fn: &mut TraverseFn<'_>) {
137138
self.stack.traverse(tracer_fn);
139+
self.cells_frees.traverse(tracer_fn);
138140
}
139141
}
140142

@@ -143,7 +145,6 @@ unsafe impl Traverse for Frame {
143145
self.code.traverse(tracer_fn);
144146
self.func_obj.traverse(tracer_fn);
145147
self.fastlocals.traverse(tracer_fn);
146-
self.cells_frees.traverse(tracer_fn);
147148
self.locals.traverse(tracer_fn);
148149
self.globals.traverse(tracer_fn);
149150
self.builtins.traverse(tracer_fn);
@@ -193,13 +194,13 @@ impl Frame {
193194

194195
let state = FrameState {
195196
stack: BoxVec::new(code.max_stackdepth as usize),
197+
cells_frees,
196198
#[cfg(feature = "threading")]
197199
lasti: 0,
198200
};
199201

200202
Self {
201203
fastlocals: PyMutex::new(fastlocals_vec.into_boxed_slice()),
202-
cells_frees,
203204
locals: scope.locals,
204205
globals: scope.globals,
205206
builtins,
@@ -218,21 +219,39 @@ impl Frame {
218219
}
219220
}
220221

221-
/// Clear the evaluation stack. Used by frame.clear() to break reference cycles.
222-
pub(crate) fn clear_value_stack(&self) {
223-
self.state.lock().stack.clear();
222+
/// Clear the evaluation stack and drop all cell/free variable references.
223+
pub(crate) fn clear_stack_and_cells(&self) {
224+
let mut state = self.state.lock();
225+
state.stack.clear();
226+
let _old = core::mem::take(&mut state.cells_frees);
224227
}
225228

226229
/// Clear locals and stack after generator/coroutine close.
227230
/// Releases references held by the frame, matching _PyFrame_ClearLocals.
228231
pub(crate) fn clear_locals_and_stack(&self) {
229-
self.state.lock().stack.clear();
232+
self.clear_stack_and_cells();
230233
let mut fastlocals = self.fastlocals.lock();
231234
for slot in fastlocals.iter_mut() {
232235
*slot = None;
233236
}
234237
}
235238

239+
/// Get cell contents by cell index. Reads through fastlocals (no state lock needed).
240+
pub(crate) fn get_cell_contents(&self, cell_idx: usize) -> Option<PyObjectRef> {
241+
let nlocals = self.code.varnames.len();
242+
let fastlocals = self.fastlocals.lock();
243+
fastlocals
244+
.get(nlocals + cell_idx)
245+
.and_then(|slot| slot.as_ref())
246+
.and_then(|obj| obj.downcast_ref::<PyCell>())
247+
.and_then(|cell| cell.get())
248+
}
249+
250+
/// Set cell contents by cell index. Only safe to call before frame execution starts.
251+
pub(crate) fn set_cell_contents(&self, cell_idx: usize, value: Option<PyObjectRef>) {
252+
self.state.lock().cells_frees[cell_idx].set(value);
253+
}
254+
236255
/// Store a borrowed back-reference to the owning generator/coroutine.
237256
/// The caller must ensure the generator outlives the frame.
238257
pub fn set_generator(&self, generator: &PyObject) {
@@ -306,23 +325,26 @@ impl Frame {
306325
}
307326
}
308327
if !code.cellvars.is_empty() || !code.freevars.is_empty() {
309-
let map_to_dict = |keys: &[&PyStrInterned], values: &[PyCellRef]| {
310-
for (&k, v) in zip(keys, values) {
311-
if let Some(value) = v.get() {
312-
locals.mapping().ass_subscript(k, Some(value), vm)?;
313-
} else {
314-
match locals.mapping().ass_subscript(k, None, vm) {
315-
Ok(()) => {}
316-
Err(e) if e.fast_isinstance(vm.ctx.exceptions.key_error) => {}
317-
Err(e) => return Err(e),
328+
let state = self.state.lock();
329+
if !state.cells_frees.is_empty() {
330+
let map_to_dict = |keys: &[&PyStrInterned], values: &[PyCellRef]| {
331+
for (&k, v) in zip(keys, values) {
332+
if let Some(value) = v.get() {
333+
locals.mapping().ass_subscript(k, Some(value), vm)?;
334+
} else {
335+
match locals.mapping().ass_subscript(k, None, vm) {
336+
Ok(()) => {}
337+
Err(e) if e.fast_isinstance(vm.ctx.exceptions.key_error) => {}
338+
Err(e) => return Err(e),
339+
}
318340
}
319341
}
342+
Ok(())
343+
};
344+
map_to_dict(&code.cellvars, &state.cells_frees)?;
345+
if code.flags.contains(bytecode::CodeFlags::OPTIMIZED) {
346+
map_to_dict(&code.freevars, &state.cells_frees[code.cellvars.len()..])?;
320347
}
321-
Ok(())
322-
};
323-
map_to_dict(&code.cellvars, &self.cells_frees)?;
324-
if code.flags.contains(bytecode::CodeFlags::OPTIMIZED) {
325-
map_to_dict(&code.freevars, &self.cells_frees[code.cellvars.len()..])?;
326348
}
327349
}
328350
Ok(locals.clone())
@@ -336,7 +358,6 @@ impl Py<Frame> {
336358
let exec = ExecutingFrame {
337359
code: &self.code,
338360
fastlocals: &self.fastlocals,
339-
cells_frees: &self.cells_frees,
340361
locals: &self.locals,
341362
globals: &self.globals,
342363
builtins: &self.builtins,
@@ -382,7 +403,6 @@ impl Py<Frame> {
382403
let exec = ExecutingFrame {
383404
code: &self.code,
384405
fastlocals: &self.fastlocals,
385-
cells_frees: &self.cells_frees,
386406
locals: &self.locals,
387407
globals: &self.globals,
388408
builtins: &self.builtins,
@@ -417,7 +437,6 @@ impl Py<Frame> {
417437
struct ExecutingFrame<'a> {
418438
code: &'a PyRef<PyCode>,
419439
fastlocals: &'a PyMutex<Box<[Option<PyObjectRef>]>>,
420-
cells_frees: &'a [PyCellRef],
421440
locals: &'a ArgMapping,
422441
globals: &'a PyDictRef,
423442
builtins: &'a PyObjectRef,
@@ -1021,7 +1040,7 @@ impl ExecutingFrame<'_> {
10211040
}
10221041
Instruction::DeleteAttr { idx } => self.delete_attr(vm, idx.get(arg)),
10231042
Instruction::DeleteDeref(i) => {
1024-
self.cells_frees[i.get(arg) as usize].set(None);
1043+
self.state.cells_frees[i.get(arg) as usize].set(None);
10251044
Ok(None)
10261045
}
10271046
Instruction::DeleteFast(idx) => {
@@ -1446,7 +1465,7 @@ impl ExecutingFrame<'_> {
14461465
};
14471466
self.push_value(match value {
14481467
Some(v) => v,
1449-
None => self.cells_frees[i]
1468+
None => self.state.cells_frees[i]
14501469
.get()
14511470
.ok_or_else(|| self.unbound_cell_exception(i, vm))?,
14521471
});
@@ -1503,7 +1522,7 @@ impl ExecutingFrame<'_> {
15031522
}
15041523
Instruction::LoadDeref(i) => {
15051524
let idx = i.get(arg) as usize;
1506-
let x = self.cells_frees[idx]
1525+
let x = self.state.cells_frees[idx]
15071526
.get()
15081527
.ok_or_else(|| self.unbound_cell_exception(idx, vm))?;
15091528
self.push_value(x);
@@ -2093,7 +2112,7 @@ impl ExecutingFrame<'_> {
20932112
Instruction::StoreAttr { idx } => self.store_attr(vm, idx.get(arg)),
20942113
Instruction::StoreDeref(i) => {
20952114
let value = self.pop_value();
2096-
self.cells_frees[i.get(arg) as usize].set(Some(value));
2115+
self.state.cells_frees[i.get(arg) as usize].set(Some(value));
20972116
Ok(None)
20982117
}
20992118
Instruction::StoreFast(idx) => {

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -367,9 +367,9 @@ pub(super) mod _os {
367367
dir_fd: DirFd<'_, { RMDIR_DIR_FD as usize }>,
368368
vm: &VirtualMachine,
369369
) -> PyResult<()> {
370-
let c_path = path.clone().into_cstring(vm)?;
371370
#[cfg(not(target_os = "redox"))]
372371
if let Some(fd) = dir_fd.raw_opt() {
372+
let c_path = path.clone().into_cstring(vm)?;
373373
let res = unsafe { libc::unlinkat(fd, c_path.as_ptr(), libc::AT_REMOVEDIR) };
374374
return if res < 0 {
375375
let err = crate::common::os::errno_io_error();
@@ -736,7 +736,11 @@ pub(super) mod _os {
736736
#[cfg(unix)]
737737
return self.test_mode_via_stat(false, libc::S_IFLNK as _, vm);
738738
#[cfg(not(unix))]
739-
Err(io::Error::other("file_type unavailable").into_pyexception(vm))
739+
match &self.file_type {
740+
Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(false),
741+
Err(e) => Err(e.to_pyexception(vm)),
742+
Ok(_) => Ok(false),
743+
}
740744
}
741745

742746
#[pymethod]

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,9 +512,9 @@ pub mod module {
512512
dir_fd: DirFd<'_, { _os::UNLINK_DIR_FD as usize }>,
513513
vm: &VirtualMachine,
514514
) -> PyResult<()> {
515-
let c_path = path.clone().into_cstring(vm)?;
516515
#[cfg(not(target_os = "redox"))]
517516
if let Some(fd) = dir_fd.raw_opt() {
517+
let c_path = path.clone().into_cstring(vm)?;
518518
let res = unsafe { libc::unlinkat(fd, c_path.as_ptr(), 0) };
519519
return if res < 0 {
520520
let err = crate::common::os::errno_io_error();

0 commit comments

Comments
 (0)