8000 cells_free · RustPython/RustPython@44048cb · GitHub
[go: up one dir, main page]
Skip to content

Commit 44048cb

Browse files
youknowoneyouknowone
committed
8000 cells_free
1 parent 7e7832d commit 44048cb

File tree

6 files changed

+70
-41
lines changed

6 files changed

+70
-41
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 & 29 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,25 @@ 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),
318-
}
319-
}
328+
// Access cells through fastlocals to avoid locking state
329+
// (state may be held by with_exec during frame execution).
330+
for (i, &k) in code.cellvars.iter().enumerate() {
331+
let cell_value = self.get_cell_contents(i);
332+
match locals.mapping().ass_subscript(k, cell_value, vm) {
333+
Ok(()) => {}
334+
Err(e) if e.fast_isinstance(vm.ctx.exceptions.key_error) => {}
335+
Err(e) => return Err(e),
320336
}
321-
Ok(())
322-
};
323-
map_to_dict(&code.cellvars, &self.cells_frees)?;
337+
}
324338
if code.flags.contains(bytecode::CodeFlags::OPTIMIZED) {
325-
map_to_dict(&code.freevars, &self.cells_frees[code.cellvars.len()..])?;
339+
for (i, &k) in code.freevars.iter().enumerate() {
340+
let cell_value = self.get_cell_contents(code.cellvars.len() + i);
341+
match locals.mapping().ass_subscript(k, cell_value, vm) {
342+
Ok(()) => {}
343+
Err(e) if e.fast_isinstance(vm.ctx.exceptions.key_error) => {}
344+
Err(e) => return Err(e),
345+
}
346+
}
326347
}
327348
}
328349
Ok(locals.clone())
@@ -336,7 +357,6 @@ impl Py<Frame> {
336357
let exec = ExecutingFrame {
337358
code: &self.code,
338359
fastlocals: &self.fastlocals,
339-
cells_frees: &self.cells_frees,
340360
locals: &self.locals,
341361
globals: &self.globals,
342362
builtins: &self.builtins,
@@ -382,7 +402,6 @@ impl Py<Frame> {
382402
let exec = ExecutingFrame {
383403
code: &self.code,
384404
fastlocals: &self.fastlocals,
385-
cells_frees: &self.cells_frees,
386405
locals: &self.locals,
387406
globals: &self.globals,
388407
builtins: &self.builtins,
@@ -417,7 +436,6 @@ impl Py<Frame> {
417436
struct ExecutingFrame<'a> {
418437
code: &'a PyRef<PyCode>,
419438
fastlocals: &'a PyMutex<Box<[Option<PyObjectRef>]>>,
420-
cells_frees: &'a [PyCellRef],
421439
locals: &'a ArgMapping,
422440
globals: &'a PyDictRef,
423441
builtins: &'a PyObjectRef,
@@ -1021,7 +1039,7 @@ impl ExecutingFrame<'_> {
10211039
}
10221040
Instruction::DeleteAttr { idx } => self.delete_attr(vm, idx.get(arg)),
10231041
Instruction::DeleteDeref(i) => {
1024-
self.cells_frees[i.get(arg) as usize].set(None);
1042+
self.state.cells_frees[i.get(arg) as usize].set(None);
10251043
Ok(None)
10261044
}
10271045
Instruction::DeleteFast(idx) => {
@@ -1446,7 +1464,7 @@ impl ExecutingFrame<'_> {
14461464
};
14471465
self.push_value(match value {
14481466
Some(v) => v,
1449-
None => self.cells_frees[i]
1467+
None => self.state.cells_frees[i]
14501468
.get()
14511469
.ok_or_else(|| self.unbound_cell_exception(i, vm))?,
14521470
});
@@ -1503,7 +1521,7 @@ impl ExecutingFrame<'_> {
15031521
}
15041522
Instruction::LoadDeref(i) => {
15051523
let idx = i.get(arg) as usize;
1506-
let x = self.cells_frees[idx]
1524+
let x = self.state.cells_frees[idx]
15071525
.get()
15081526
.ok_or_else(|| self.unbound_cell_exception(idx, vm))?;
15091527
self.push_value(x);
@@ -2093,7 +2111,7 @@ impl ExecutingFrame<'_> {
20932111
Instruction::StoreAttr { idx } => self.store_attr(vm, idx.get(arg)),
20942112
Instruction::StoreDeref(i) => {
20952113
let value = self.pop_value();
2096-
self.cells_frees[i.get(arg) as usize].set(Some(value));
2114+
self.state.cells_frees[i.get(arg) as usize].set(Some(value));
20972115
Ok(None)
20982116
}
20992117
Instruction::StoreFast(idx) => {

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
22

33
use crate::{
44
AsObject, Py, PyObjectRef, PyPayload, PyResult, TryFromObject, VirtualMachine,
5-
builtins::{PyDictRef, PyModule, PySet},
5+
builtins::{PyModule, PySet},
66
common::crt_fd,
77
convert::{IntoPyException, ToPyException, ToPyObject},
8-
function::{ArgMapping, ArgumentError, FromArgs, FuncArgs},
8+
function::{ArgumentError, FromArgs, FuncArgs},
99
};
1010
use std::{fs, io, path::Path};
1111

@@ -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,14 @@ 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) => {
742+
use crate::convert::ToPyException;
743+
Err(e.to_pyexception(vm))
744+
}
745+
Ok(_) => Ok(false),
746+
}
740747
}
741748

742749
#[pymethod]
@@ -2341,7 +2348,11 @@ pub fn module_exec(vm: &VirtualMachine, module: &Py<PyModule>) -> PyResult<()> {
23412348
/// For `os._Environ`, accesses the internal `_data` dict directly at the Rust level.
23422349
/// This avoids Python-level method calls that can deadlock after fork() when
23432350
/// parking_lot locks are held by threads that no longer exist.
2344-
pub(crate) fn envobj_to_dict(env: ArgMapping, vm: &VirtualMachine) -> PyResult<PyDictRef> {
2351+
#[cfg(any(unix, windows))]
2352+
pub(crate) fn envobj_to_dict(
2353+
env: crate::function::ArgMapping,
2354+
vm: &VirtualMachine,
2355+
) -> PyResult<crate::builtins::PyDictRef> {
23452356
let obj = env.obj();
23462357
if let Some(dict) = obj.downcast_ref_if_exact::<crate::builtins::PyDict>(vm) {
23472358
return Ok(dict.to_owned());

‎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)
0