mirror of
https://github.com/RustPython/RustPython.git
synced 2026-10-28 18:59:04 +08:00
Remove static cache for builtins iter/reversed reduce
- Replace static_cell! cached builtins_iter/builtins_reversed with per-call vm.builtins.get_attr lookups to support runtime builtins mutation (CPython issue #101765) - Refactor PositionIterInternal::reduce to take func, active, and empty callbacks; resolve builtins outside the lock to prevent deadlock from reentrant iterator access - Add __reduce__ and fix reentrant next() in PyCallableIterator - Remove expectedFailure from test_reverse_iterator_picking and test_reduce_mutating_builtins_iter
This commit is contained in:
1 parent
b52b049982
commit
b6e7d71e2e
14 files changed
+145
-88
No files matched your search
Vendored
-2
@@ -355,8 +355,6 @@ class BaseTest:
|
||||
self.assertEqual(list(a), list(self.example))
|
||||
self.assertEqual(list(reversed(a)), list(iter(a))[::-1])
|
||||
|
||||
# TODO: RUSTPYTHON
|
||||
@unittest.expectedFailure
|
||||
def test_reverse_iterator_picking(self):
|
||||
orig = array.array(self.typecode, self.example)
|
||||
data = list(orig)
|
||||
|
||||
Vendored
-1
@@ -249,7 +249,6 @@ class TestCase(unittest.TestCase):
|
||||
self.assertEqual(list(empit), [5, 6])
|
||||
self.assertEqual(list(a), [0, 1, 2, 3, 4, 5, 6])
|
||||
|
||||
@unittest.expectedFailure # TODO: RUSTPYTHON; + (<built-in function iter>, ('',))
|
||||
def test_reduce_mutating_builtins_iter(self):
|
||||
# This is a reproducer of issue #101765
|
||||
# where iter `__reduce__` calls could lead to a segfault or SystemError
|
||||
|
||||
@@ -19,7 +19,7 @@ mod array {
|
||||
builtins::{
|
||||
PositionIterInternal, PyByteArray, PyBytes, PyBytesRef, PyDictRef, PyFloat,
|
||||
PyGenericAlias, PyInt, PyList, PyListRef, PyStr, PyStrRef, PyTupleRef, PyType,
|
||||
PyTypeRef,
|
||||
PyTypeRef, builtins_iter,
|
||||
},
|
||||
class_or_notimplemented,
|
||||
convert::{ToPyObject, ToPyResult, TryFromBorrowedObject, TryFromObject},
|
||||
@@ -1376,9 +1376,13 @@ mod array {
|
||||
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
self.internal
|
||||
.lock()
|
||||
.builtins_iter_reduce(|x| x.clone().into(), vm)
|
||||
let func = builtins_iter(vm);
|
||||
self.internal.lock().reduce(
|
||||
func,
|
||||
|x| x.clone().into(),
|
||||
|vm| vm.ctx.empty_tuple.clone().into(),
|
||||
vm,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
//! Implementation of the python bytearray object.
|
||||
use super::{
|
||||
PositionIterInternal, PyBytes, PyBytesRef, PyDictRef, PyGenericAlias, PyIntRef, PyStrRef,
|
||||
PyTuple, PyTupleRef, PyType, PyTypeRef,
|
||||
PyTuple, PyTupleRef, PyType, PyTypeRef, iter::builtins_iter,
|
||||
};
|
||||
use crate::{
|
||||
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject,
|
||||
@@ -857,9 +857,13 @@ impl PyByteArrayIterator {
|
||||
}
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
self.internal
|
||||
.lock()
|
||||
.builtins_iter_reduce(|x| x.clone().into(), vm)
|
||||
let func = builtins_iter(vm);
|
||||
self.internal.lock().reduce(
|
||||
func,
|
||||
|x| x.clone().into(),
|
||||
|vm| vm.ctx.empty_tuple.clone().into(),
|
||||
vm,
|
||||
)
|
||||
}
|
||||
|
||||
#[pymethod]
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use super::{
|
||||
PositionIterInternal, PyDictRef, PyGenericAlias, PyIntRef, PyStrRef, PyTuple, PyTupleRef,
|
||||
PyType, PyTypeRef,
|
||||
PyType, PyTypeRef, iter::builtins_iter,
|
||||
};
|
||||
use crate::{
|
||||
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult,
|
||||
@@ -747,9 +747,13 @@ impl PyBytesIterator {
|
||||
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
self.internal
|
||||
.lock()
|
||||
.builtins_iter_reduce(|x| x.clone().into(), vm)
|
||||
let func = builtins_iter(vm);
|
||||
self.internal.lock().reduce(
|
||||
func,
|
||||
|x| x.clone().into(),
|
||||
|vm| vm.ctx.empty_tuple.clone().into(),
|
||||
vm,
|
||||
)
|
||||
}
|
||||
|
||||
#[pymethod]
|
||||
|
||||
@@ -898,7 +898,7 @@ macro_rules! dict_view {
|
||||
#[allow(clippy::redundant_closure_call)]
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
let iter = builtins_iter(vm).to_owned();
|
||||
let iter = builtins_iter(vm);
|
||||
let internal = self.internal.lock();
|
||||
let entries = match &internal.status {
|
||||
IterStatus::Active(dict) => dict
|
||||
@@ -968,7 +968,7 @@ macro_rules! dict_view {
|
||||
#[allow(clippy::redundant_closure_call)]
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
let iter = builtins_reversed(vm).to_owned();
|
||||
let iter = builtins_reversed(vm);
|
||||
let internal = self.internal.lock();
|
||||
// TODO: entries must be reversed too
|
||||
let entries = match &internal.status {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::{
|
||||
IterStatus, PositionIterInternal, PyGenericAlias, PyIntRef, PyTupleRef, PyType, PyTypeRef,
|
||||
iter::builtins_reversed,
|
||||
};
|
||||
use crate::common::lock::{PyMutex, PyRwLock};
|
||||
use crate::{
|
||||
@@ -124,9 +125,13 @@ impl PyReverseSequenceIterator {
|
||||
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
self.internal
|
||||
.lock()
|
||||
.builtins_reversed_reduce(|x| x.clone(), vm)
|
||||
let func = builtins_reversed(vm);
|
||||
self.internal.lock().reduce(
|
||||
func,
|
||||
|x| x.clone(),
|
||||
|vm| vm.ctx.empty_tuple.clone().into(),
|
||||
vm,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -4,17 +4,14 @@
|
||||
|
||||
use super::{PyInt, PyTupleRef, PyType};
|
||||
use crate::{
|
||||
Context, Py, PyObject, PyObjectRef, PyPayload, PyResult, VirtualMachine,
|
||||
Context, Py, PyObjectRef, PyPayload, PyResult, VirtualMachine,
|
||||
class::PyClassImpl,
|
||||
function::ArgCallable,
|
||||
object::{Traverse, TraverseFn},
|
||||
protocol::PyIterReturn,
|
||||
types::{IterNext, Iterable, SelfIter},
|
||||
};
|
||||
use rustpython_common::{
|
||||
lock::{PyMutex, PyRwLock, PyRwLockUpgradableReadGuard},
|
||||
static_cell,
|
||||
};
|
||||
use rustpython_common::lock::{PyMutex, PyRwLock, PyRwLockUpgradableReadGuard};
|
||||
|
||||
/// Marks status of iterator.
|
||||
#[derive(Debug, Clone)]
|
||||
@@ -71,33 +68,29 @@ impl<T> PositionIterInternal<T> {
|
||||
}
|
||||
}
|
||||
|
||||
fn _reduce<F>(&self, func: PyObjectRef, f: F, vm: &VirtualMachine) -> PyTupleRef
|
||||
/// Build a pickle-compatible reduce tuple.
|
||||
///
|
||||
/// `func` must be resolved **before** acquiring any lock that guards this
|
||||
/// `PositionIterInternal`, so that the builtins lookup cannot trigger
|
||||
/// reentrant iterator access and deadlock.
|
||||
pub fn reduce<F, E>(
|
||||
&self,
|
||||
func: PyObjectRef,
|
||||
active: F,
|
||||
empty: E,
|
||||
vm: &VirtualMachine,
|
||||
) -> PyTupleRef
|
||||
where
|
||||
F: FnOnce(&T) -> PyObjectRef,
|
||||
E: FnOnce(&VirtualMachine) -> PyObjectRef,
|
||||
{
|
||||
if let IterStatus::Active(obj) = &self.status {
|
||||
vm.new_tuple((func, (f(obj),), self.position))
|
||||
vm.new_tuple((func, (active(obj),), self.position))
|
||||
} else {
|
||||
vm.new_tuple((func, (vm.ctx.new_list(Vec::new()),)))
|
||||
vm.new_tuple((func, (empty(vm),)))
|
||||
}
|
||||
}
|
||||
|
||||
pub fn builtins_iter_reduce<F>(&self, f: F, vm: &VirtualMachine) -> PyTupleRef
|
||||
where
|
||||
F: FnOnce(&T) -> PyObjectRef,
|
||||
{
|
||||
let iter = builtins_iter(vm).to_owned();
|
||||
self._reduce(iter, f, vm)
|
||||
}
|
||||
|
||||
pub fn builtins_reversed_reduce<F>(&self, f: F, vm: &VirtualMachine) -> PyTupleRef
|
||||
where
|
||||
F: FnOnce(&T) -> PyObjectRef,
|
||||
{
|
||||
let reversed = builtins_reversed(vm).to_owned();
|
||||
self._reduce(reversed, f, vm)
|
||||
}
|
||||
|
||||
fn _next<F, OP>(&mut self, f: F, op: OP) -> PyResult<PyIterReturn>
|
||||
where
|
||||
F: FnOnce(&T, usize) -> PyResult<PyIterReturn>,
|
||||
@@ -160,18 +153,12 @@ impl<T> PositionIterInternal<T> {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn builtins_iter(vm: &VirtualMachine) -> &PyObject {
|
||||
static_cell! {
|
||||
static INSTANCE: PyObjectRef;
|
||||
}
|
||||
INSTANCE.get_or_init(|| vm.builtins.get_attr("iter", vm).unwrap())
|
||||
pub fn builtins_iter(vm: &VirtualMachine) -> PyObjectRef {
|
||||
vm.builtins.get_attr("iter", vm).unwrap()
|
||||
}
|
||||
|
||||
pub fn builtins_reversed(vm: &VirtualMachine) -> &PyObject {
|
||||
static_cell! {
|
||||
static INSTANCE: PyObjectRef;
|
||||
}
|
||||
INSTANCE.get_or_init(|| vm.builtins.get_attr("reversed", vm).unwrap())
|
||||
pub fn builtins_reversed(vm: &VirtualMachine) -> PyObjectRef {
|
||||
vm.builtins.get_attr("reversed", vm).unwrap()
|
||||
}
|
||||
|
||||
#[pyclass(module = false, name = "iterator", traverse)]
|
||||
@@ -211,7 +198,13 @@ impl PySequenceIterator {
|
||||
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
self.internal.lock().builtins_iter_reduce(|x| x.clone(), vm)
|
||||
let func = builtins_iter(vm);
|
||||
self.internal.lock().reduce(
|
||||
func,
|
||||
|x| x.clone(),
|
||||
|vm| vm.ctx.empty_tuple.clone().into(),
|
||||
vm,
|
||||
)
|
||||
}
|
||||
|
||||
#[pymethod]
|
||||
@@ -252,24 +245,47 @@ impl PyCallableIterator {
|
||||
status: PyRwLock::new(IterStatus::Active(callable)),
|
||||
}
|
||||
}
|
||||
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
let func = builtins_iter(vm);
|
||||
let status = self.status.read();
|
||||
if let IterStatus::Active(callable) = &*status {
|
||||
let callable_obj: PyObjectRef = callable.clone().into();
|
||||
vm.new_tuple((func, (callable_obj, self.sentinel.clone())))
|
||||
} else {
|
||||
vm.new_tuple((func, (vm.ctx.empty_tuple.clone(),)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl SelfIter for PyCallableIterator {}
|
||||
impl IterNext for PyCallableIterator {
|
||||
fn next(zelf: &Py<Self>, vm: &VirtualMachine) -> PyResult<PyIterReturn> {
|
||||
let status = zelf.status.upgradable_read();
|
||||
let next = if let IterStatus::Active(callable) = &*status {
|
||||
let ret = callable.invoke((), vm)?;
|
||||
if vm.bool_eq(&ret, &zelf.sentinel)? {
|
||||
*PyRwLockUpgradableReadGuard::upgrade(status) = IterStatus::Exhausted;
|
||||
PyIterReturn::StopIteration(None)
|
||||
} else {
|
||||
PyIterReturn::Return(ret)
|
||||
// Clone the callable and release the lock before invoking,
|
||||
// so that reentrant next() calls don't deadlock.
|
||||
let callable = {
|
||||
let status = zelf.status.read();
|
||||
match &*status {
|
||||
IterStatus::Active(callable) => callable.clone(),
|
||||
IterStatus::Exhausted => return Ok(PyIterReturn::StopIteration(None)),
|
||||
}
|
||||
} else {
|
||||
PyIterReturn::StopIteration(None)
|
||||
};
|
||||
Ok(next)
|
||||
|
||||
let ret = callable.invoke((), vm)?;
|
||||
|
||||
// Re-check: a reentrant call may have exhausted the iterator.
|
||||
let status = zelf.status.upgradable_read();
|
||||
if !matches!(&*status, IterStatus::Active(_)) {
|
||||
return Ok(PyIterReturn::StopIteration(None));
|
||||
}
|
||||
|
||||
if vm.bool_eq(&ret, &zelf.sentinel)? {
|
||||
*PyRwLockUpgradableReadGuard::upgrade(status) = IterStatus::Exhausted;
|
||||
Ok(PyIterReturn::StopIteration(None))
|
||||
} else {
|
||||
Ok(PyIterReturn::Return(ret))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,7 @@
|
||||
use super::{PositionIterInternal, PyGenericAlias, PyTupleRef, PyType, PyTypeRef};
|
||||
use super::{
|
||||
PositionIterInternal, PyGenericAlias, PyTupleRef, PyType, PyTypeRef,
|
||||
iter::{builtins_iter, builtins_reversed},
|
||||
};
|
||||
use crate::atomic_func;
|
||||
use crate::common::lock::{
|
||||
PyMappedRwLockReadGuard, PyMutex, PyRwLock, PyRwLockReadGuard, PyRwLockWriteGuard,
|
||||
@@ -577,9 +580,13 @@ impl PyListIterator {
|
||||
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
self.internal
|
||||
.lock()
|
||||
.builtins_iter_reduce(|x| x.clone().into(), vm)
|
||||
let func = builtins_iter(vm);
|
||||
self.internal.lock().reduce(
|
||||
func,
|
||||
|x| x.clone().into(),
|
||||
|vm| vm.ctx.new_list(Vec::new()).into(),
|
||||
vm,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -622,9 +629,13 @@ impl PyListReverseIterator {
|
||||
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
self.internal
|
||||
.lock()
|
||||
.builtins_reversed_reduce(|x| x.clone().into(), vm)
|
||||
let func = builtins_reversed(vm);
|
||||
self.internal.lock().reduce(
|
||||
func,
|
||||
|x| x.clone().into(),
|
||||
|vm| vm.ctx.new_list(Vec::new()).into(),
|
||||
vm,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use super::{
|
||||
PositionIterInternal, PyBytes, PyBytesRef, PyGenericAlias, PyInt, PyListRef, PySlice, PyStr,
|
||||
PyStrRef, PyTuple, PyTupleRef, PyType, PyTypeRef,
|
||||
PyStrRef, PyTuple, PyTupleRef, PyType, PyTypeRef, iter::builtins_iter,
|
||||
};
|
||||
use crate::{
|
||||
AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult,
|
||||
@@ -1132,9 +1132,13 @@ impl PyPayload for PyMemoryViewIterator {
|
||||
impl PyMemoryViewIterator {
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
self.internal
|
||||
.lock()
|
||||
.builtins_iter_reduce(|x| x.clone().into(), vm)
|
||||
let func = builtins_iter(vm);
|
||||
self.internal.lock().reduce(
|
||||
func,
|
||||
|x| x.clone().into(),
|
||||
|vm| vm.ctx.empty_tuple.clone().into(),
|
||||
vm,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -670,7 +670,7 @@ fn range_iter_reduce(
|
||||
index: usize,
|
||||
vm: &VirtualMachine,
|
||||
) -> PyResult<PyTupleRef> {
|
||||
let iter = builtins_iter(vm).to_owned();
|
||||
let iter = builtins_iter(vm);
|
||||
let stop = start.clone() + length * step.clone();
|
||||
let range = PyRange {
|
||||
start: PyInt::from(start).into_ref(&vm.ctx),
|
||||
|
||||
@@ -1390,7 +1390,7 @@ impl PySetIterator {
|
||||
) -> PyResult<(PyObjectRef, (PyObjectRef,))> {
|
||||
let internal = zelf.internal.lock();
|
||||
Ok((
|
||||
builtins_iter(vm).to_owned(),
|
||||
builtins_iter(vm),
|
||||
(vm.ctx
|
||||
.new_list(match &internal.status {
|
||||
IterStatus::Exhausted => vec![],
|
||||
|
||||
@@ -1,7 +1,10 @@
|
||||
use super::{
|
||||
PositionIterInternal, PyBytesRef, PyDict, PyTupleRef, PyType, PyTypeRef,
|
||||
int::{PyInt, PyIntRef},
|
||||
iter::IterStatus::{self, Exhausted},
|
||||
iter::{
|
||||
IterStatus::{self, Exhausted},
|
||||
builtins_iter,
|
||||
},
|
||||
};
|
||||
use crate::{
|
||||
AsObject, Context, Py, PyExact, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult,
|
||||
@@ -304,10 +307,13 @@ impl PyStrIterator {
|
||||
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
self.internal
|
||||
.lock()
|
||||
.0
|
||||
.builtins_iter_reduce(|x| x.clone().into(), vm)
|
||||
let func = builtins_iter(vm);
|
||||
self.internal.lock().0.reduce(
|
||||
func,
|
||||
|x| x.clone().into(),
|
||||
|vm| vm.ctx.empty_str.to_owned().into(),
|
||||
vm,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
use super::{PositionIterInternal, PyGenericAlias, PyStrRef, PyType, PyTypeRef};
|
||||
use super::{
|
||||
PositionIterInternal, PyGenericAlias, PyStrRef, PyType, PyTypeRef, iter::builtins_iter,
|
||||
};
|
||||
use crate::common::{
|
||||
hash::{PyHash, PyUHash},
|
||||
lock::PyMutex,
|
||||
@@ -559,9 +561,13 @@ impl PyTupleIterator {
|
||||
|
||||
#[pymethod]
|
||||
fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef {
|
||||
self.internal
|
||||
.lock()
|
||||
.builtins_iter_reduce(|x| x.clone().into(), vm)
|
||||
let func = builtins_iter(vm);
|
||||
self.internal.lock().reduce(
|
||||
func,
|
||||
|x| x.clone().into(),
|
||||
|vm| vm.ctx.empty_tuple.clone().into(),
|
||||
vm,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user