diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index c7663980939..f828b778659 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -601,7 +601,6 @@ def test_trailing_counter(self): 'spam and eggs') self.assertRaises(struct.error, struct.unpack_from, '14s42', store, 0) - @unittest.expectedFailure # TODO: RUSTPYTHON; AssertionError: '>h' != '>hh' def test_Struct_reinitialization(self): # Issue 9422: there was a memory leak when reinitializing a # Struct instance. This test can be used to detect the leak @@ -826,7 +825,6 @@ def test_error_propagation(fmt_str): test_error_propagation('N') test_error_propagation('n') - @unittest.expectedFailure # TODO: RUSTPYTHON def test_struct_subclass_instantiation(self): # Regression test for https://github.com/python/cpython/issues/112358 class MyStruct(struct.Struct): diff --git a/Lib/test/test_support.py b/Lib/test/test_support.py index 19ea6fafcf7..42aa7e3d9bb 100644 --- a/Lib/test/test_support.py +++ b/Lib/test/test_support.py @@ -631,7 +631,6 @@ def test_has_strftime_extensions(self): else: self.assertTrue(support.has_strftime_extensions) - @unittest.expectedFailure # TODO: RUSTPYTHON; - _testinternalcapi module not available def test_get_recursion_depth(self): # test support.get_recursion_depth() code = textwrap.dedent(""" diff --git a/crates/common/src/borrow.rs b/crates/common/src/borrow.rs index 2be5f8275c8..70d755ff155 100644 --- a/crates/common/src/borrow.rs +++ b/crates/common/src/borrow.rs @@ -34,6 +34,17 @@ impl_from!('a, T, BorrowedValue<'a, T>, ); impl<'a, T: ?Sized> BorrowedValue<'a, T> { + /// Whether reaching the value holds a lock that other threads wait on. + /// + /// An immutable object hands out a plain reference and answers `false`; + /// one whose storage can change hands out a guard. A caller about to wait + /// for something unrelated -- a peer, a file, a signal -- can use this to + /// decide whether it may keep the borrow for the duration. + #[must_use] + pub const fn is_locked(&self) -> bool { + !matches!(self, Self::Ref(_)) + } + pub fn map(s: Self, f: F) -> BorrowedValue<'a, U> where F: FnOnce(&T) -> &U, diff --git a/crates/common/src/str.rs b/crates/common/src/str.rs index 326f76c43cb..39ec7da1de5 100644 --- a/crates/common/src/str.rs +++ b/crates/common/src/str.rs @@ -592,20 +592,21 @@ pub fn codepoint_range_end(s: &Wtf8, n_chars: usize) -> Option { } #[must_use] -pub fn zfill(bytes: &[u8], width: usize) -> Vec { +/// Returns `None` for a width whose result cannot be allocated. +pub fn zfill(bytes: &[u8], width: usize) -> Option> { if width <= bytes.len() { - bytes.to_vec() - } else { - let (sign, s) = match bytes.first() { - Some(_sign @ (b'+' | b'-')) => (unsafe { bytes.get_unchecked(..1) }, &bytes[1..]), - _ => (&b""[..], bytes), - }; - let mut filled = Vec::new(); - filled.extend_from_slice(sign); - filled.extend(core::iter::repeat_n(b'0', width - bytes.len())); - filled.extend_from_slice(s); - filled + return Some(bytes.to_vec()); } + let (sign, s) = match bytes.first() { + Some(_sign @ (b'+' | b'-')) => (unsafe { bytes.get_unchecked(..1) }, &bytes[1..]), + _ => (&b""[..], bytes), + }; + let mut filled = Vec::new(); + filled.try_reserve_exact(width).ok()?; + filled.extend_from_slice(sign); + filled.extend(core::iter::repeat_n(b'0', width - bytes.len())); + filled.extend_from_slice(s); + Some(filled) } /// Convert a string to ascii compatible, escaping unicode-s into escape diff --git a/crates/compiler-core/src/marshal.rs b/crates/compiler-core/src/marshal.rs index 46e0047941c..754854e7cba 100644 --- a/crates/compiler-core/src/marshal.rs +++ b/crates/compiler-core/src/marshal.rs @@ -19,6 +19,14 @@ pub enum MarshalError { InvalidLocation, /// Bad type marker BadType, + /// A type marker no reader knows + UnknownType, + /// A back reference that names nothing + InvalidRef, + /// A marker that stands for no object at all + NullObject, + /// A container length that is negative or does not fit, named by what it counts + BadSize(&'static str), } impl core::fmt::Display for MarshalError { @@ -29,6 +37,10 @@ impl core::fmt::Display for MarshalError { Self::InvalidUtf8 => f.write_str("invalid utf8"), Self::InvalidLocation => f.write_str("invalid source location"), Self::BadType => f.write_str("bad type marker"), + Self::UnknownType => f.write_str("unknown type code"), + Self::InvalidRef => f.write_str("invalid reference"), + Self::NullObject => f.write_str("NULL object in marshal data for object"), + Self::BadSize(what) => write!(f, "{what} size out of range"), } } } @@ -111,7 +123,7 @@ impl TryFrom for Type { b'A' => Self::AsciiInterned, b'z' => Self::ShortAscii, b'Z' => Self::ShortAsciiInterned, - _ => return Err(MarshalError::BadType), + _ => return Err(MarshalError::UnknownType), }) } } @@ -146,6 +158,13 @@ pub trait Read { fn read_u64(&mut self) -> Result { Ok(u64::from_le_bytes(*self.read_array()?)) } + + /// A length, read the way `r_long` reads one: it is signed, so a value + /// with the top bit set is out of range rather than four billion items. + fn read_len(&mut self, what: &'static str) -> Result { + let len = self.read_u32()? as i32; + usize::try_from(len).map_err(|_| MarshalError::BadSize(what)) + } } pub(crate) trait ReadBorrowed<'a>: Read { @@ -305,7 +324,7 @@ fn reserve_ref_slot(has_flag: bool, refs: &mut Vec>) -> Option(idx: usize, refs: &[Option]) -> Result { refs.get(idx) .and_then(|v| v.clone()) - .ok_or(MarshalError::InvalidBytecode) + .ok_or(MarshalError::InvalidRef) } /// Read a marshal bytes object (TYPE_STRING = b's'), resolving TYPE_REF @@ -408,7 +427,7 @@ fn read_marshal_str_vec( } let n = match type_byte { - b'(' => rdr.read_u32()? as usize, + b'(' => rdr.read_len("tuple")?, b')' => rdr.read_u8()? as usize, _ => return Err(MarshalError::BadType), }; @@ -471,7 +490,7 @@ fn read_marshal_const_tuple( } let n = match type_byte { - b'(' => rdr.read_u32()? as usize, + b'(' => rdr.read_len("tuple")?, b')' => rdr.read_u8()? as usize, _ => return Err(MarshalError::BadType), }; @@ -553,7 +572,7 @@ pub trait MarshalBag: Copy { fn make_code( &self, code: CodeObject<::Constant>, - ) -> Self::Value; + ) -> Result; /// Construct a runtime code object while retaining the exact values read /// from ``co_consts``. Compiler bags ignore this second channel; runtime @@ -563,7 +582,7 @@ pub trait MarshalBag: Copy { &self, code: CodeObject<::Constant>, _constants: Vec, - ) -> Self::Value { + ) -> Result { self.make_code(code) } @@ -583,8 +602,12 @@ pub trait MarshalBag: Copy { /// Install partially-built containers in the marshal reference table /// before reading their children, as CPython's `r_object()` does. /// Runtime bags can opt in; constant bags retain collect-then-construct. - fn make_tuple_placeholder(&self, _len: usize) -> Option { - None + /// + /// `len` comes straight from the input and is only bounded by what a + /// length can hold, so a bag that opts in reports the room it cannot get + /// rather than taking it for granted. + fn make_tuple_placeholder(&self, _len: usize) -> Result> { + Ok(None) } fn set_tuple_item( @@ -596,8 +619,8 @@ pub trait MarshalBag: Copy { Err(MarshalError::BadType) } - fn make_list_placeholder(&self, _len: usize) -> Option { - None + fn make_list_placeholder(&self, _len: usize) -> Result> { + Ok(None) } fn set_list_item(&self, _list: &Self::Value, _index: usize, _value: Self::Value) -> Result<()> { @@ -725,8 +748,8 @@ impl MarshalBag for Bag { fn make_code( &self, code: CodeObject<::Constant>, - ) -> Self::Value { - self.make_code(code) + ) -> Result { + Ok(self.make_code(code)) } fn make_stop_iter(&self) -> Result { @@ -830,10 +853,7 @@ fn deserialize_value_after_header( // TYPE_REF: return previously stored object if type_code == Type::Ref as u8 { let idx = rdr.read_u32()? as usize; - return refs - .get(idx) - .and_then(|v| v.clone()) - .ok_or(MarshalError::InvalidBytecode); + return resolve_ref(idx, refs); } // Reserve ref slot before reading (matches write order) @@ -986,7 +1006,7 @@ fn deserialize_code_value_inner( linetable, exceptiontable, }; - Ok(bag.make_code_with_constants(code, constant_values)) + bag.make_code_with_constants(code, constant_values) } fn deserialize_value_typed( @@ -1033,13 +1053,13 @@ fn deserialize_value_typed( bag.make_complex(value) } Type::Ascii | Type::Unicode => { - let len = rdr.read_u32()?; - let value = rdr.read_wtf8(len)?; + let len = rdr.read_len("string")?; + let value = rdr.read_wtf8(len as u32)?; bag.make_str(value) } Type::AsciiInterned | Type::Interned => { - let len = rdr.read_u32()?; - let value = rdr.read_wtf8(len)?; + let len = rdr.read_len("string")?; + let value = rdr.read_wtf8(len as u32)?; bag.make_interned_str(value) } Type::ShortAscii => { @@ -1056,7 +1076,7 @@ fn deserialize_value_typed( let len = rdr.read_u8()? as usize; let d = depth - 1; if let Some(index) = slot - && let Some(tuple) = bag.make_tuple_placeholder(len) + && let Some(tuple) = bag.make_tuple_placeholder(len)? { refs[index] = Some(tuple.clone()); for item_index in 0..len { @@ -1070,17 +1090,17 @@ fn deserialize_value_typed( } } Type::Null => { - return Err(MarshalError::BadType); + return Err(MarshalError::NullObject); } Type::Ref => { // Handled in deserialize_value_depth before calling this function return Err(MarshalError::BadType); } Type::Tuple => { - let len = rdr.read_u32()? as usize; + let len = rdr.read_len("tuple")?; let d = depth - 1; if let Some(index) = slot - && let Some(tuple) = bag.make_tuple_placeholder(len) + && let Some(tuple) = bag.make_tuple_placeholder(len)? { refs[index] = Some(tuple.clone()); for item_index in 0..len { @@ -1094,10 +1114,10 @@ fn deserialize_value_typed( } } Type::List => { - let len = rdr.read_u32()? as usize; + let len = rdr.read_len("list")?; let d = depth - 1; if let Some(index) = slot - && let Some(list) = bag.make_list_placeholder(len) + && let Some(list) = bag.make_list_placeholder(len)? { refs[index] = Some(list.clone()); for item_index in 0..len { @@ -1111,7 +1131,7 @@ fn deserialize_value_typed( } } Type::Set => { - let len = rdr.read_u32()? as usize; + let len = rdr.read_len("set")?; let d = depth - 1; if let Some(index) = slot && let Some(set) = bag.make_set_placeholder() @@ -1128,7 +1148,7 @@ fn deserialize_value_typed( } } Type::FrozenSet => { - let len = rdr.read_u32()?; + let len = rdr.read_len("set")?; let d = depth - 1; let it = (0..len).map(|_| deserialize_value_depth(rdr, bag, d, refs)); itertools::process_results(it, |it| bag.make_frozenset(it))?? @@ -1165,8 +1185,8 @@ fn deserialize_value_typed( } Type::Bytes => { // After marshaling, byte arrays are converted into bytes. - let len = rdr.read_u32()?; - let value = rdr.read_slice(len)?; + let len = rdr.read_len("bytes object")?; + let value = rdr.read_slice(len as u32)?; bag.make_bytes(value) } Type::Code => return Err(MarshalError::BadType), diff --git a/crates/host_env/src/io.rs b/crates/host_env/src/io.rs index 6df29bcd6bc..f32ef2f6944 100644 --- a/crates/host_env/src/io.rs +++ b/crates/host_env/src/io.rs @@ -199,6 +199,29 @@ pub fn is_seekable(fd: crt_fd::Borrowed<'_>) -> bool { os::seek_fd(fd, 0, libc::SEEK_CUR).is_ok() } +/// Whether a read from `fd` answers from data the file already holds, rather +/// than waiting for whoever writes the other end. +/// +/// Seeking answers this everywhere but Windows, where a pipe seeks too -- +/// `lseek` on one succeeds and reports a position, so a reader that took +/// seekability for an answer would wait on a peer while holding whatever it +/// holds for the length of the call. +#[cfg(not(windows))] +pub fn reads_without_waiting(fd: crt_fd::Borrowed<'_>) -> bool { + is_seekable(fd) +} + +#[cfg(windows)] +pub fn reads_without_waiting(fd: crt_fd::Borrowed<'_>) -> bool { + use std::os::windows::io::AsRawHandle; + use windows_sys::Win32::Storage::FileSystem::{FILE_TYPE_DISK, GetFileType}; + + let Ok(handle) = crt_fd::as_handle(fd) else { + return false; + }; + unsafe { GetFileType(handle.as_raw_handle() as _) == FILE_TYPE_DISK } +} + pub fn validate_whence(whence: i32) -> bool { let standard = (0..=2).contains(&whence); #[cfg(any(target_os = "dragonfly", target_os = "freebsd", target_os = "linux"))] diff --git a/crates/host_env/src/io_unsupported.rs b/crates/host_env/src/io_unsupported.rs index e46f05af900..d9fdc5d0d23 100644 --- a/crates/host_env/src/io_unsupported.rs +++ b/crates/host_env/src/io_unsupported.rs @@ -176,6 +176,10 @@ pub fn is_seekable(_fd: crt_fd::Borrowed<'_>) -> bool { false } +pub fn reads_without_waiting(_fd: crt_fd::Borrowed<'_>) -> bool { + false +} + pub fn validate_whence(whence: i32) -> bool { (0..=2).contains(&whence) } diff --git a/crates/stdlib/src/_asyncio.rs b/crates/stdlib/src/_asyncio.rs index 9ad75fb8d69..c3f28590e6a 100644 --- a/crates/stdlib/src/_asyncio.rs +++ b/crates/stdlib/src/_asyncio.rs @@ -724,47 +724,56 @@ pub(crate) mod _asyncio { /// Add waiter to fut_awaited_by with single-object optimization fn awaited_by_add(&self, waiter: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - let mut awaited_by = self.fut_awaited_by.write(); - if awaited_by.is_none() { - // First waiter - store directly - *awaited_by = Some(waiter); - return Ok(()); - } + // Storing a waiter in the set runs its __hash__ and __eq__, which can + // come back to this future, so the field is locked only while it is + // read or written. + let existing = { + let mut awaited_by = self.fut_awaited_by.write(); + match awaited_by.as_ref() { + // First waiter - store directly + None => { + *awaited_by = Some(waiter); + return Ok(()); + } + Some(existing) => existing.clone(), + } + }; if self.fut_awaited_by_is_set.load(Ordering::Relaxed) { // Already a Set - add to it - let set = awaited_by.as_ref().unwrap(); - vm.call_method(set, "add", (waiter,))?; - } else { - // Single object - convert to Set - let existing = awaited_by.take().unwrap(); - let new_set = PySet::default().into_ref(&vm.ctx); - new_set.add(existing, vm)?; - new_set.add(waiter, vm)?; - *awaited_by = Some(new_set.into()); - self.fut_awaited_by_is_set.store(true, Ordering::Relaxed); + return vm.call_method(&existing, "add", (waiter,)).map(drop); } + + // Single object - convert to Set + let new_set = PySet::default().into_ref(&vm.ctx); + new_set.add(existing, vm)?; + new_set.add(waiter, vm)?; + *self.fut_awaited_by.write() = Some(new_set.into()); + self.fut_awaited_by_is_set.store(true, Ordering::Relaxed); Ok(()) } /// Discard waiter from fut_awaited_by with single-object optimization fn awaited_by_discard(&self, waiter: &PyObject, vm: &VirtualMachine) -> PyResult<()> { - let mut awaited_by = self.fut_awaited_by.write(); - if awaited_by.is_none() { - return Ok(()); - } - - let obj = awaited_by.as_ref().unwrap(); - if !self.fut_awaited_by_is_set.load(Ordering::Relaxed) { - // Single object - check if it matches - if obj.is(waiter) { - *awaited_by = None; + // As in awaited_by_add, discarding from the set runs Python. + let set = { + let mut awaited_by = self.fut_awaited_by.write(); + let Some(obj) = awaited_by.as_ref() else { + return Ok(()); + }; + if !self.fut_awaited_by_is_set.load(Ordering::Relaxed) { + // Single object - check if it matches + if obj.is(waiter) { + *awaited_by = None; + } + return Ok(()); } - } else { - // It's a Set - use discard - vm.call_method(obj, "discard", (waiter.to_owned(),))?; - } - Ok(()) + obj.clone() + }; + + // It's a Set - use discard + vm.call_method(&set, "discard", (waiter.to_owned(),)) + .map(drop) } #[pymethod] diff --git a/crates/stdlib/src/array.rs b/crates/stdlib/src/array.rs index 68e7aab2566..0f652efd35c 100644 --- a/crates/stdlib/src/array.rs +++ b/crates/stdlib/src/array.rs @@ -55,6 +55,11 @@ pub mod array { $($n(Vec<$t>),)* } + /// One item, already converted to the array's element type. + enum ArrayItem { + $($n($t),)* + } + impl ArrayContentType { fn from_char(c: char) -> Result { match c { @@ -303,17 +308,31 @@ pub mod array { } } - fn setitem_by_index( + /// Convert an object to the element type of the array with + /// this typecode. This runs the object's conversion methods, + /// which can reach the array, so it takes the typecode by + /// value and holds no lock on it. + fn item_from_object( + typecode: char, + value: PyObjectRef, + vm: &VirtualMachine + ) -> PyResult { + match typecode { + $($c => Ok(ArrayItem::$n(<$t>::try_into_from_object(vm, value)?)),)* + _ => unreachable!("array has a typecode"), + } + } + + fn setitem_by_item( &mut self, i: isize, - value: PyObjectRef, + item: ArrayItem, vm: &VirtualMachine ) -> PyResult<()> { - match self { - $(ArrayContentType::$n(v) => { - let value = <$t>::try_into_from_object(vm, value)?; - v.setitem_by_index(vm, i, value) - })* + match (self, item) { + $((ArrayContentType::$n(v), ArrayItem::$n(value)) => + v.setitem_by_index(vm, i, value),)* + _ => unreachable!("item was converted for this array"), } } @@ -906,6 +925,11 @@ pub mod array { #[pymethod] fn frombytes(&self, b: ArgBytesLike, vm: &VirtualMachine) -> PyResult<()> { + // The source is read as bytes, so items of any other width would + // be reinterpreted rather than appended. + if b.itemsize() != 1 { + return Err(vm.new_type_error("a bytes-like object is required")); + } let b = b.borrow_buf(); let itemsize = self.read().itemsize(); self._from_bytes(&b, itemsize, vm) @@ -1047,7 +1071,11 @@ pub mod array { vm: &VirtualMachine, ) -> PyResult<()> { match SequenceIndex::try_from_borrowed_object(vm, needle, "array")? { - SequenceIndex::Int(i) => zelf.write().setitem_by_index(i, value, vm), + SequenceIndex::Int(i) => { + let typecode = zelf.read().typecode(); + let item = ArrayContentType::item_from_object(typecode, value, vm)?; + zelf.write().setitem_by_item(i, item, vm) + } SequenceIndex::Slice(slice) => { let cloned; let guard; @@ -1408,7 +1436,9 @@ pub mod array { ass_item: atomic_func!(|seq, i, value, vm| { let zelf = PyArray::sequence_downcast(seq); if let Some(value) = value { - zelf.write().setitem_by_index(i, value, vm) + let typecode = zelf.read().typecode(); + let item = ArrayContentType::item_from_object(typecode, value, vm)?; + zelf.write().setitem_by_item(i, item, vm) } else { zelf.write().delitem_by_index(i, vm) } @@ -1443,8 +1473,15 @@ pub mod array { type Resizable<'a> = PyRwLockWriteGuard<'a, ArrayContentType>; fn try_resizable_opt(&self) -> Option> { - let w = self.write(); - (self.exports.load(atomic::Ordering::SeqCst) == 0).then_some(w) + // An export is a borrow someone else still holds, so it is + // answered before the lock rather than by waiting on it. + (self.exports.load(atomic::Ordering::SeqCst) == 0).then(|| self.write()) + } + + fn try_resizable(&self, vm: &VirtualMachine) -> PyResult> { + self.try_resizable_opt().ok_or_else(|| { + vm.new_buffer_error("cannot resize an array that is exporting buffers") + }) } } diff --git a/crates/stdlib/src/hashlib.rs b/crates/stdlib/src/hashlib.rs index c2153b08a59..d7f94cc2796 100644 --- a/crates/stdlib/src/hashlib.rs +++ b/crates/stdlib/src/hashlib.rs @@ -847,15 +847,15 @@ pub(crate) mod _hashlib { if len < 1 { return Err(vm.new_value_error("key length must be greater than 0.")); } - usize::try_from(len) - .map_err(|_| vm.new_overflow_error("key length is too great."))? + i32::try_from(len).map_err(|_| vm.new_overflow_error("key length is too great."))? + as usize } None => hash_digest_size(&name).ok_or_else(|| unsupported_hash(&name, vm))?, }; let password_buf = args.password.borrow_buf(); let salt_buf = args.salt.borrow_buf(); - let mut dk = vec![0u8; dklen]; + let mut dk = vm.new_zeroed_bytes(dklen)?; macro_rules! do_pbkdf2 { ($hash_ty:ty) => {{ diff --git a/crates/stdlib/src/mmap.rs b/crates/stdlib/src/mmap.rs index b5dec976594..14957ad904e 100644 --- a/crates/stdlib/src/mmap.rs +++ b/crates/stdlib/src/mmap.rs @@ -1152,24 +1152,35 @@ mod mmap { } #[pymethod] - fn write(&self, bytes: ArgBytesLike, vm: &VirtualMachine) -> PyResult { - let pos = self.pos(); - let size = self.__len__(); - - let data = bytes.borrow_buf(); + fn write(zelf: &Py, bytes: ArgBytesLike, vm: &VirtualMachine) -> PyResult { + let self_ = &**zelf; + let pos = self_.pos(); + let size = self_.__len__(); + + // Writing locks the map, and reading a source that views this same + // map locks it too, so such a source is copied out first. + let copied; + let borrowed; + let data: &[u8] = if bytes.source_object().is(zelf.as_object()) { + copied = bytes.borrow_buf().to_vec(); + &copied + } else { + borrowed = bytes.borrow_buf(); + &borrowed + }; if pos > size || size - pos < data.len() { return Err(vm.new_value_error("data out of range")); } - let len = self.try_writable(vm, |mmap| { + let len = self_.try_writable(vm, |mmap| { (&mut mmap[pos..(pos + data.len())]) - .write(&data) + .write(data) .map_err(|err| err.to_pyexception(vm))?; Ok(data.len()) })??; - self.advance_pos(len); + self_.advance_pos(len); Ok(PyInt::from(len).into_ref(&vm.ctx)) } diff --git a/crates/stdlib/src/pystruct.rs b/crates/stdlib/src/pystruct.rs index c525942e35e..496b448e5e8 100644 --- a/crates/stdlib/src/pystruct.rs +++ b/crates/stdlib/src/pystruct.rs @@ -10,13 +10,14 @@ pub(crate) use _struct::module_def; #[pymodule] pub(crate) mod _struct { use crate::vm::{ - AsObject, Py, PyObjectRef, PyPayload, PyResult, TryFromObject, VirtualMachine, + AsObject, Py, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine, buffer::{FormatSpec, new_struct_error, struct_error_type}, builtins::{PyBytes, PyStr, PyStrRef, PyTupleRef, PyType, PyTypeRef}, - function::{ArgBytesLike, ArgMemoryBuffer, PosArgs}, + common::lock::{PyMappedRwLockReadGuard, PyRwLock, PyRwLockReadGuard}, + function::{ArgBytesLike, ArgMemoryBuffer, FuncArgs, PosArgs}, match_class, protocol::PyIterReturn, - types::{Constructor, IterNext, Iterable, Representable, SelfIter}, + types::{Constructor, Initializer, IterNext, Iterable, Representable, SelfIter}, }; use crossbeam_utils::atomic::AtomicCell; use rustpython_common::wtf8::{Wtf8Buf, wtf8_concat}; @@ -251,41 +252,76 @@ pub(crate) mod _struct { Ok(fmt.format_spec(vm)?.size) } + /// What a `Struct` is once a format has been read into it. Held apart + /// from the object because `__new__` hands out a `Struct` that `__init__` + /// has not filled in yet, and `__init__` may be called again on one that + /// already holds a format. + #[derive(Debug)] + struct StructSpec { + spec: FormatSpec, + format: PyStrRef, + } + #[pyattr] #[pyclass(name = "Struct", traverse)] #[derive(Debug, PyPayload)] struct PyStruct { #[pytraverse(skip)] - spec: FormatSpec, - format: PyStrRef, + inner: PyRwLock>, } impl Constructor for PyStruct { + type Args = FuncArgs; + + fn py_new(_cls: &Py, _args: Self::Args, _vm: &VirtualMachine) -> PyResult { + Ok(Self { + inner: PyRwLock::new(None), + }) + } + } + + impl Initializer for PyStruct { type Args = IntoStructFormatBytes; - fn py_new(_cls: &Py, fmt: Self::Args, vm: &VirtualMachine) -> PyResult { + fn init(zelf: PyRef, fmt: Self::Args, vm: &VirtualMachine) -> PyResult<()> { + // The format is read before anything is replaced, so a format that + // cannot be read leaves the object as it was. let spec = fmt.format_spec(vm)?; - let format = fmt.0; - Ok(Self { spec, format }) + *zelf.inner.write() = Some(StructSpec { + spec, + format: fmt.0, + }); + Ok(()) } } - #[pyclass(with(Constructor, Representable))] + #[pyclass(with(Constructor, Initializer, Representable), flags(BASETYPE))] impl PyStruct { + /// The format this was initialized with, or an error if `__init__` + /// never ran. + fn ready(&self, vm: &VirtualMachine) -> PyResult> { + PyRwLockReadGuard::try_map(self.inner.read(), Option::as_ref) + .map_err(|_| vm.new_runtime_error("Struct object is not initialized")) + } + #[pygetset] - fn format(&self) -> PyStrRef { - self.format.clone() + fn format(&self, vm: &VirtualMachine) -> PyResult { + Ok(self.ready(vm)?.format.clone()) } + /// The size an uninitialized `Struct` reports, which no format has + /// yet given a value. #[pygetset] - #[inline] - const fn size(&self) -> usize { - self.spec.size + fn size(&self) -> isize { + self.inner + .read() + .as_ref() + .map_or(-1, |inner| inner.spec.size as isize) } #[pymethod] fn pack(&self, args: PosArgs, vm: &VirtualMachine) -> PyResult> { - self.spec.pack(args.into_vec(), vm) + self.ready(vm)?.spec.pack(args.into_vec(), vm) } #[pymethod] @@ -296,23 +332,28 @@ pub(crate) mod _struct { args: PosArgs, vm: &VirtualMachine, ) -> PyResult<()> { - let offset = get_buffer_offset(buffer.len(), offset, self.size(), true, vm)?; + let inner = self.ready(vm)?; + let offset = get_buffer_offset(buffer.len(), offset, inner.spec.size, true, vm)?; buffer.with_ref(|data| { - self.spec + inner + .spec .pack_into(&mut data[offset..], args.into_vec(), vm) }) } #[pymethod] fn unpack(&self, data: ArgBytesLike, vm: &VirtualMachine) -> PyResult { - data.with_ref(|buf| self.spec.unpack(buf, vm)) + let inner = self.ready(vm)?; + data.with_ref(|buf| inner.spec.unpack(buf, vm)) } #[pymethod] fn unpack_from(&self, args: UpdateFromArgs, vm: &VirtualMachine) -> PyResult { - let offset = get_buffer_offset(args.buffer.len(), args.offset, self.size(), false, vm)?; + let inner = self.ready(vm)?; + let size = inner.spec.size; + let offset = get_buffer_offset(args.buffer.len(), args.offset, size, false, vm)?; args.buffer - .with_ref(|buf| self.spec.unpack(&buf[offset..][..self.size()], vm)) + .with_ref(|buf| inner.spec.unpack(&buf[offset..][..size], vm)) } #[pymethod] @@ -321,14 +362,19 @@ pub(crate) mod _struct { buffer: ArgBytesLike, vm: &VirtualMachine, ) -> PyResult { - UnpackIterator::with_buffer(vm, self.spec.clone(), buffer) + let spec = self.ready(vm)?.spec.clone(); + UnpackIterator::with_buffer(vm, spec, buffer) } } impl Representable for PyStruct { #[inline] - fn repr_wtf8(zelf: &Py, _vm: &VirtualMachine) -> PyResult { - Ok(wtf8_concat!("Struct('", zelf.format.as_wtf8(), "')")) + fn repr_wtf8(zelf: &Py, vm: &VirtualMachine) -> PyResult { + Ok(wtf8_concat!( + "Struct('", + zelf.ready(vm)?.format.as_wtf8(), + "')" + )) } } diff --git a/crates/stdlib/src/select.rs b/crates/stdlib/src/select.rs index c1f10f3ecc2..84ec92927e8 100644 --- a/crates/stdlib/src/select.rs +++ b/crates/stdlib/src/select.rs @@ -79,16 +79,26 @@ mod decl { } let deadline = timeout.map(|s| time::time(vm).unwrap() + s); + let max_fds: usize = cfg_select! { + windows => FD_SETSIZE as usize, + _ => FD_SETSIZE, + }; + let seq2set = |list: &PyObject| -> PyResult<(Vec, FdSet)> { - let v: Vec = list.try_to_value(vm)?; - - let too_many_fds = cfg_select! { - windows => v.len() > FD_SETSIZE as usize, - _ => v.len() > FD_SETSIZE, - }; - if too_many_fds { - return Err(vm.new_value_error("too many file descriptors in select()")); - } + // The limit is answered while the sequence is walked rather than + // from the length of the result. fileno() runs Python and can + // append to the very list being walked, and a walk that re-reads + // the list each step -- which is what `seq2set` does -- then never + // reaches a length to check. + let seen = core::cell::Cell::new(0usize); + let v: Vec = vm.extract_elements_with(list, |obj| { + let selectable = Selectable::try_from_object(vm, obj)?; + seen.set(seen.get() + 1); + if seen.get() > max_fds { + return Err(vm.new_value_error("too many file descriptors in select()")); + } + Ok(selectable) + })?; let mut fds = FdSet::new(); for fd in &v { @@ -304,7 +314,10 @@ mod decl { timeout: OptionalArg>, vm: &VirtualMachine, ) -> PyResult> { - let mut fds = self.fds.lock(); + // Poll a copy: the wait releases the GIL-equivalent and runs + // signal handlers, which can register or unregister on the same + // object, and a held lock would deadlock them. + let mut fds = self.fds.lock().clone(); let TimeoutArg(timeout) = timeout.unwrap_or_default(); let timeout_ms = match timeout { Some(d) => i32::try_from(d.as_millis()) diff --git a/crates/stdlib/src/socket.rs b/crates/stdlib/src/socket.rs index f78bec69dc5..4e02dca451c 100644 --- a/crates/stdlib/src/socket.rs +++ b/crates/stdlib/src/socket.rs @@ -40,7 +40,6 @@ mod _socket { } use core::{ - mem::MaybeUninit, net::{Ipv4Addr, Ipv6Addr, SocketAddr}, time::Duration, }; @@ -1589,7 +1588,10 @@ mod _socket { vm: &VirtualMachine, ) -> Result, IoOrPyException> { let flags = flags.unwrap_or(0); - let mut buffer = Vec::with_capacity(bufsize); + let mut buffer = Vec::new(); + buffer + .try_reserve_exact(bufsize) + .map_err(|_| vm.new_memory_error(""))?; let sock = self.sock()?; let n = self.sock_op(vm, SockWaitKind::Read, || { sock.recv_with_flags(buffer.spare_capacity_mut(), flags) @@ -1608,8 +1610,6 @@ mod _socket { ) -> Result { let flags = flags.unwrap_or(0); let sock = self.sock()?; - let mut buf = buf.borrow_buf_mut(); - let buf = &mut *buf; // Handle nbytes parameter let read_len = if let OptionalArg::Present(nbytes) = nbytes { @@ -1621,10 +1621,13 @@ mod _socket { buf.len() }; - let buf = &mut buf[..read_len]; - self.sock_op(vm, SockWaitKind::Read, || { - sock.recv_with_flags(unsafe { slice_as_uninit(buf) }, flags) - }) + let mut scratch = alloc_recv_scratch(read_len, vm)?; + let n = self.sock_op(vm, SockWaitKind::Read, || { + sock.recv_with_flags(&mut scratch.spare_capacity_mut()[..read_len], flags) + })?; + unsafe { scratch.set_len(n) }; + buf.borrow_buf_mut()[..n].copy_from_slice(&scratch); + Ok(n) } #[pymethod] @@ -1638,7 +1641,10 @@ mod _socket { let bufsize = bufsize .to_usize() .ok_or_else(|| vm.new_value_error("negative buffersize in recvfrom"))?; - let mut buffer = Vec::with_capacity(bufsize); + let mut buffer = Vec::new(); + buffer + .try_reserve_exact(bufsize) + .map_err(|_| vm.new_memory_error(""))?; let (n, addr) = self.sock_op(vm, SockWaitKind::Read, || { self.sock()? .recv_from_with_flags(buffer.spare_capacity_mut(), flags) @@ -1655,24 +1661,28 @@ mod _socket { flags: OptionalArg, vm: &VirtualMachine, ) -> Result<(usize, PyObjectRef), IoOrPyException> { - let mut buf = buf.borrow_buf_mut(); - let buf = &mut *buf; - let buf = match nbytes { + let read_len = match nbytes { OptionalArg::Present(i) => { let i = i.to_usize().ok_or_else(|| { vm.new_value_error("negative buffersize in recvfrom_into") })?; - buf.get_mut(..i).ok_or_else(|| { - vm.new_value_error("nbytes is greater than the length of the buffer") - })? + if i > buf.len() { + return Err(vm + .new_value_error("nbytes is greater than the length of the buffer") + .into()); + } + i } - OptionalArg::Missing => buf, + OptionalArg::Missing => buf.len(), }; let flags = flags.unwrap_or(0); let sock = self.sock()?; + let mut scratch = alloc_recv_scratch(read_len, vm)?; let (n, addr) = self.sock_op(vm, SockWaitKind::Read, || { - sock.recv_from_with_flags(unsafe { slice_as_uninit(buf) }, flags) + sock.recv_from_with_flags(&mut scratch.spare_capacity_mut()[..read_len], flags) })?; + unsafe { scratch.set_len(n) }; + buf.borrow_buf_mut()[..n].copy_from_slice(&scratch); Ok((n, get_addr_tuple(&addr, vm))) } @@ -1684,7 +1694,7 @@ mod _socket { vm: &VirtualMachine, ) -> Result { let flags = flags.unwrap_or(0); - let buf = bytes.borrow_buf(); + let buf = bytes.borrow_buf_unlocked(vm)?; let buf = &*buf; self.sock_op(vm, SockWaitKind::Write, || { self.sock()?.send_with_flags(buf, flags) @@ -1704,7 +1714,7 @@ mod _socket { let deadline = timeout.map(Deadline::new); - let buf = bytes.borrow_buf(); + let buf = bytes.borrow_buf_unlocked(vm)?; let buf = &*buf; let mut buf_offset = 0; // now we have like 3 layers of interrupt loop :) @@ -1741,7 +1751,7 @@ mod _socket { OptionalArg::Missing => (0, arg2), }; let addr = self.extract_address(address, "sendto", vm)?; - let buf = bytes.borrow_buf(); + let buf = bytes.borrow_buf_unlocked(vm)?; let buf = &*buf; self.sock_op(vm, SockWaitKind::Write, || { self.sock()?.send_to_with_flags(buf, &addr, flags) @@ -1771,8 +1781,8 @@ mod _socket { let buffers = buffers .iter() - .map(|buf| buf.borrow_buf()) - .collect::>(); + .map(|buf| buf.borrow_buf_unlocked(vm)) + .collect::>>()?; let buffers = buffers .iter() .map(|buf| io::IoSlice::new(buf)) @@ -2380,8 +2390,21 @@ mod _socket { Ok(s.to_string_lossy().into_owned()) } - unsafe fn slice_as_uninit(v: &mut [T]) -> &mut [MaybeUninit] { - unsafe { &mut *(v as *mut [T] as *mut [MaybeUninit]) } + /// Room to receive into that belongs to no Python object. + /// + /// A peer may never send, so the wait for it is unbounded. The export of + /// the caller's buffer is held for the whole call, which is what keeps it + /// from being resized, but the borrow that reaches its bytes is a lock + /// every other thread touching that object waits on, and a thread waiting + /// on a lock never reaches a safepoint — holding it across the wait stops + /// the world from being stopped at all. The bytes are copied over once + /// they have arrived. + fn alloc_recv_scratch(len: usize, vm: &VirtualMachine) -> PyResult> { + let mut scratch = Vec::new(); + scratch + .try_reserve_exact(len) + .map_err(|_| vm.new_memory_error(""))?; + Ok(scratch) } enum IoOrPyException { diff --git a/crates/vm/src/anystr.rs b/crates/vm/src/anystr.rs index 0f187f6d476..4896f2789bd 100644 --- a/crates/vm/src/anystr.rs +++ b/crates/vm/src/anystr.rs @@ -27,7 +27,7 @@ pub struct SplitLinesArgs { #[derive(FromArgs)] pub struct ExpandTabsArgs { #[pyarg(any, default = 8)] - tabsize: isize, + tabsize: i32, } impl ExpandTabsArgs { @@ -132,6 +132,11 @@ where { fn new() -> Self; fn with_capacity(capacity: usize) -> Self; + /// `with_capacity`, reporting a capacity that cannot be allocated instead + /// of aborting the process on it. + fn try_with_capacity(capacity: usize) -> Option + where + Self: Sized; fn push_str(&mut self, s: &S); } @@ -285,27 +290,29 @@ pub(crate) trait AnyStr { } } - fn py_pad(&self, left: usize, right: usize, fillchar: Self::Char) -> Self::Container { - let mut u = Self::Container::with_capacity( - (left + right) * fillchar.bytes_len() + self.bytes_len(), - ); + fn py_pad(&self, left: usize, right: usize, fillchar: Self::Char) -> Option { + let capacity = left + .checked_add(right)? + .checked_mul(fillchar.bytes_len())? + .checked_add(self.bytes_len())?; + let mut u = Self::Container::try_with_capacity(capacity)?; u.extend(core::iter::repeat_n(fillchar, left)); u.push_str(self); u.extend(core::iter::repeat_n(fillchar, right)); - u + Some(u) } - fn py_center(&self, width: usize, fillchar: Self::Char, len: usize) -> Self::Container { + fn py_center(&self, width: usize, fillchar: Self::Char, len: usize) -> Option { let marg = width - len; let left = marg / 2 + (marg & width & 1); self.py_pad(left, marg - left, fillchar) } - fn py_ljust(&self, width: usize, fillchar: Self::Char, len: usize) -> Self::Container { + fn py_ljust(&self, width: usize, fillchar: Self::Char, len: usize) -> Option { self.py_pad(0, width - len, fillchar) } - fn py_rjust(&self, width: usize, fillchar: Self::Char, len: usize) -> Self::Container { + fn py_rjust(&self, width: usize, fillchar: Self::Char, len: usize) -> Option { self.py_pad(width - len, 0, fillchar) } @@ -402,7 +409,7 @@ pub(crate) trait AnyStr { elements } - fn py_zfill(&self, width: isize) -> Vec { + fn py_zfill(&self, width: isize) -> Option> { let width = width.to_usize().unwrap_or(0); let char_len = self.elements().count(); let width = self diff --git a/crates/vm/src/buffer.rs b/crates/vm/src/buffer.rs index dc3691b5421..038e7cae9f3 100644 --- a/crates/vm/src/buffer.rs +++ b/crates/vm/src/buffer.rs @@ -16,7 +16,7 @@ use malachite_bigint::BigInt; use num_traits::{PrimInt, ToPrimitive}; use std::os::raw; -type PackFunc = fn(&VirtualMachine, PyObjectRef, &mut [u8]) -> PyResult<()>; +type PackFunc = fn(&VirtualMachine, FormatType, PyObjectRef, &mut [u8]) -> PyResult<()>; type UnpackFunc = fn(&VirtualMachine, &[u8]) -> PyObjectRef; static OVERFLOW_MSG: &str = "total struct size too long"; // not a const to reduce code size @@ -490,7 +490,7 @@ impl FormatSpec { let pack = code.info.pack.unwrap(); for arg in args.by_ref().take(code.repeat) { let (item_buf, rest) = buffer.split_at_mut(code.info.size); - pack(vm, arg, item_buf)?; + pack(vm, code.code, arg, item_buf)?; buffer = rest; } } @@ -549,7 +549,12 @@ impl FormatSpec { } trait Packable { - fn pack(vm: &VirtualMachine, arg: PyObjectRef, data: &mut [u8]) -> PyResult<()>; + fn pack( + vm: &VirtualMachine, + code: FormatType, + arg: PyObjectRef, + data: &mut [u8], + ) -> PyResult<()>; fn unpack(vm: &VirtualMachine, data: &[u8]) -> PyObjectRef; } @@ -576,10 +581,11 @@ macro_rules! make_pack_prim_int { impl Packable for $T { fn pack( vm: &VirtualMachine, + code: FormatType, arg: PyObjectRef, data: &mut [u8], ) -> PyResult<()> { - let i: $T = get_int_or_index(vm, arg)?; + let i: $T = get_int_or_index(vm, code, arg)?; i.pack_int::(data); Ok(()) } @@ -592,16 +598,28 @@ macro_rules! make_pack_prim_int { }; } -fn get_int_or_index(vm: &VirtualMachine, arg: PyObjectRef) -> PyResult +fn get_int_or_index(vm: &VirtualMachine, code: FormatType, arg: PyObjectRef) -> PyResult where - T: PrimInt + for<'a> TryFrom<&'a BigInt>, + T: PrimInt + fmt::Display + for<'a> TryFrom<&'a BigInt>, { let index = arg .try_index_opt(vm) .unwrap_or_else(|| Err(new_struct_error(vm, "required argument is not an integer")))?; - index - .try_to_primitive(vm) - .map_err(|_| new_struct_error(vm, "argument out of range")) + index.try_to_primitive(vm).map_err(|_| { + // A pointer is converted rather than checked against the range of a + // named format, so what it reports is the conversion failing. + let msg = if code == FormatType::VoidP { + "int too large to convert".to_owned() + } else { + format!( + "'{}' format requires {} <= number <= {}", + code as u8 as char, + T::min_value(), + T::max_value() + ) + }; + new_struct_error(vm, msg) + }) } make_pack_prim_int!(i8); @@ -620,6 +638,7 @@ macro_rules! make_pack_float { impl Packable for $T { fn pack( vm: &VirtualMachine, + _code: FormatType, arg: PyObjectRef, data: &mut [u8], ) -> PyResult<()> { @@ -648,7 +667,12 @@ make_pack_float!(f32, "f"); make_pack_float!(f64, "d"); impl Packable for f16 { - fn pack(vm: &VirtualMachine, arg: PyObjectRef, data: &mut [u8]) -> PyResult<()> { + fn pack( + vm: &VirtualMachine, + _code: FormatType, + arg: PyObjectRef, + data: &mut [u8], + ) -> PyResult<()> { let f_64 = ArgIntoFloat::try_from_object(vm, arg)?.into_float(); // "from_f64 should be preferred in any non-`const` context" except it gives the wrong result :/ let f_16 = Self::from_f64_const(f_64); @@ -666,8 +690,13 @@ impl Packable for f16 { } impl Packable for *mut raw::c_void { - fn pack(vm: &VirtualMachine, arg: PyObjectRef, data: &mut [u8]) -> PyResult<()> { - usize::pack::(vm, arg, data) + fn pack( + vm: &VirtualMachine, + code: FormatType, + arg: PyObjectRef, + data: &mut [u8], + ) -> PyResult<()> { + usize::pack::(vm, code, arg, data) } fn unpack(vm: &VirtualMachine, rdr: &[u8]) -> PyObjectRef { @@ -676,7 +705,12 @@ impl Packable for *mut raw::c_void { } impl Packable for bool { - fn pack(vm: &VirtualMachine, arg: PyObjectRef, data: &mut [u8]) -> PyResult<()> { + fn pack( + vm: &VirtualMachine, + _code: FormatType, + arg: PyObjectRef, + data: &mut [u8], + ) -> PyResult<()> { let v = ArgIntoBool::try_from_object(vm, arg)?.into_bool() as u8; v.pack_int::(data); Ok(()) @@ -688,7 +722,12 @@ impl Packable for bool { } } -fn pack_char(vm: &VirtualMachine, arg: PyObjectRef, data: &mut [u8]) -> PyResult<()> { +fn pack_char( + vm: &VirtualMachine, + _code: FormatType, + arg: PyObjectRef, + data: &mut [u8], +) -> PyResult<()> { let v = PyBytesRef::try_from_object(vm, arg)?; let ch = *v .as_bytes() diff --git a/crates/vm/src/builtins/bytearray.rs b/crates/vm/src/builtins/bytearray.rs index 9be51a37012..93046b4932e 100644 --- a/crates/vm/src/builtins/bytearray.rs +++ b/crates/vm/src/builtins/bytearray.rs @@ -354,7 +354,10 @@ impl PyByteArray { #[pymethod] fn join(&self, iter: ArgIterable, vm: &VirtualMachine) -> PyResult { - Ok(self.inner().join(iter, vm)?.into()) + // Driving the iterable runs Python, which can reach this bytearray, + // so the separator is taken by value rather than left borrowed. + let separator = self.inner().clone(); + Ok(separator.join(iter, vm)?.into()) } #[pymethod] @@ -497,8 +500,8 @@ impl PyByteArray { } #[pymethod] - fn zfill(&self, width: isize) -> Self { - self.inner().zfill(width).into() + fn zfill(&self, width: isize, vm: &VirtualMachine) -> PyResult { + Ok(self.inner().zfill(width, vm)?.into()) } #[pymethod] @@ -532,7 +535,10 @@ impl PyByteArray { } fn __mod__(&self, values: PyObjectRef, vm: &VirtualMachine) -> PyResult { - let formatted = self.inner().cformat(values, vm)?; + // Formatting calls the values' conversion methods, which can reach + // this bytearray, so the format is taken by value. + let format = self.inner().clone(); + let formatted = format.cformat(values, vm)?; Ok(formatted.into()) } @@ -778,8 +784,9 @@ impl BufferResizeGuard for PyByteArray { type Resizable<'a> = PyRwLockWriteGuard<'a, PyBytesInner>; fn try_resizable_opt(&self) -> Option> { - let w = self.inner.write(); - (self.exports.load(Ordering::SeqCst) == 0).then_some(w) + // An export is a borrow someone else still holds, so it is answered + // before the lock rather than by waiting on it. + (self.exports.load(Ordering::SeqCst) == 0).then(|| self.inner.write()) } } diff --git a/crates/vm/src/builtins/bytes.rs b/crates/vm/src/builtins/bytes.rs index d62b873bca7..48c0e431229 100644 --- a/crates/vm/src/builtins/bytes.rs +++ b/crates/vm/src/builtins/bytes.rs @@ -505,8 +505,8 @@ impl PyBytes { } #[pymethod] - fn zfill(&self, width: isize) -> Self { - self.inner.zfill(width).into() + fn zfill(&self, width: isize, vm: &VirtualMachine) -> PyResult { + Ok(self.inner.zfill(width, vm)?.into()) } #[pymethod] diff --git a/crates/vm/src/builtins/memory.rs b/crates/vm/src/builtins/memory.rs index 31f99715742..a4c29fe443c 100644 --- a/crates/vm/src/builtins/memory.rs +++ b/crates/vm/src/builtins/memory.rs @@ -1,6 +1,6 @@ use super::{ PositionIterInternal, PyBytes, PyBytesRef, PyGenericAlias, PyInt, PyListRef, PySlice, PyStr, - PyTuple, PyTupleRef, PyType, PyTypeRef, PyUtf8StrRef, iter::builtins_iter, + PyStrRef, PyTuple, PyTupleRef, PyType, PyTypeRef, PyUtf8StrRef, iter::builtins_iter, }; use crate::common::lock::LazyLock; use crate::{ @@ -192,6 +192,11 @@ impl PyMemoryView { } } + /// The object this view looks at, whose storage it borrows. + pub fn viewed_object(&self) -> &PyObject { + &self.buffer.obj + } + fn try_not_released(&self, vm: &VirtualMachine) -> PyResult<()> { if self.released.load() { Err(vm.new_value_error("operation forbidden on released memoryview object")) @@ -826,10 +831,32 @@ impl PyMemoryView { } #[pymethod] - fn tobytes(&self, vm: &VirtualMachine) -> PyResult { + fn tobytes(&self, args: ToBytesArgs, vm: &VirtualMachine) -> PyResult { self.try_not_released(vm)?; + let order = match &args.order { + None => Order::C, + Some(order) => match order.to_str() { + Some("C") => Order::C, + Some("F") => Order::Fortran, + Some("A") => Order::Any, + _ => return Err(vm.new_value_error("order must be 'C', 'F' or 'A'")), + }, + }; + let mut v = vec![]; - self.append_to(&mut v); + // 'A' asks for the memory as it is laid out, which is what appending a + // contiguous view does. Only a Fortran walk of a view that is not + // already Fortran-contiguous reorders anything, and a view of fewer + // than two dimensions has one layout under either name. + if order == Order::Fortran && self.desc.ndim() > 1 { + v.reserve(self.desc.len); + let bytes = &*self.buffer.obj_bytes(); + self.desc.for_each_segment_fortran(|range| { + v.extend_from_slice(&bytes[range.start as usize..range.end as usize]); + }); + } else { + self.append_to(&mut v); + } Ok(PyBytes::from(v).into_ref(&vm.ctx)) } @@ -925,10 +952,17 @@ impl PyMemoryView { fn cast_to_1d(&self, format: PyUtf8StrRef, vm: &VirtualMachine) -> PyResult { let format_str = format.as_str(); - if Self::native_fmtchar(format_str).is_none() { + let Some(dest_char) = Self::native_fmtchar(format_str) else { return Err(vm.new_value_error( "memoryview: destination format must be a native single character format prefixed with an optional '@'", )); + }; + // One side has to be bytes. Casting between two item types would + // reinterpret the items rather than re-divide the memory, and the + // source items were written by something that chose their type. + let source_is_bytes = Self::native_fmtchar(&self.desc.format).is_some_and(is_byte_fmtchar); + if !source_is_bytes && !is_byte_fmtchar(dest_char) { + return Err(vm.new_type_error("memoryview: cannot cast between two non-byte formats")); } let format_spec = Self::parse_format(format_str, vm)?; let itemsize = format_spec.size(); @@ -994,7 +1028,7 @@ impl PyMemoryView { let mut other = self.cast_to_1d(format, vm)?; let itemsize = other.desc.itemsize; - // 0 ndim is single item + // 0 ndim is single item, so the buffer has to be that one item if shape_ndim == 0 { if itemsize != other.desc.len { return Err( @@ -1002,7 +1036,6 @@ impl PyMemoryView { ); } other.desc.dim_desc = vec![]; - other.desc.len = itemsize; return Ok(other.into_ref(&vm.ctx)); } @@ -1010,7 +1043,19 @@ impl PyMemoryView { let mut dim_descriptor = Vec::with_capacity(shape_ndim); for x in shape { - let x = usize::try_from_borrowed_object(vm, x)?; + let x = x + .downcast_ref::() + .ok_or_else(|| { + vm.new_type_error("memoryview.cast(): elements of shape must be integers") + })? + .try_to_primitive::(vm) + .ok() + .filter(|x| *x > 0) + .ok_or_else(|| { + vm.new_value_error( + "memoryview.cast(): elements of shape must be integers > 0", + ) + })?; if x > isize::MAX as usize / product_shape { return Err(vm.new_value_error("memoryview.cast(): product(shape) > SSIZE_MAX")); @@ -1084,6 +1129,20 @@ impl Py { } } +#[derive(FromArgs)] +struct ToBytesArgs { + #[pyarg(any, default)] + order: Option, +} + +/// The layout a copy of a view is written in. +#[derive(PartialEq, Eq)] +enum Order { + C, + Fortran, + Any, +} + #[derive(FromArgs)] struct CastArgs { #[pyarg(any)] @@ -1242,7 +1301,9 @@ impl Hashable for PyMemoryView { if !zelf.desc.readonly { return Err(vm.new_value_error("cannot hash writable memoryview object")); } - if !matches!(&*zelf.desc.format, "B" | "b" | "c") { + // The hash is over the bytes, so it agrees with the hash of the same + // bytes only where an item is a byte. + if !Self::native_fmtchar(&zelf.desc.format).is_some_and(is_byte_fmtchar) { return Err( vm.new_value_error("memoryview: hashing is restricted to formats 'B', 'b' or 'c'") ); @@ -1496,6 +1557,10 @@ fn format_unpack( }) } +/// Whether `ch` names a format whose items are single bytes. +const fn is_byte_fmtchar(ch: u8) -> bool { + matches!(ch, b'c' | b'b' | b'B') +} fn is_equiv_shape(a: &BufferDescriptor, b: &BufferDescriptor) -> bool { if a.ndim() != b.ndim() { return false; diff --git a/crates/vm/src/builtins/str.rs b/crates/vm/src/builtins/str.rs index 8a95dcb648c..6e774f7e652 100644 --- a/crates/vm/src/builtins/str.rs +++ b/crates/vm/src/builtins/str.rs @@ -1320,11 +1320,13 @@ impl PyStr { } #[pymethod] - fn zfill(&self, width: isize) -> Wtf8Buf { - unsafe { - // SAFETY: this is safe-guaranteed because the original self.as_wtf8() is valid wtf8 - Wtf8Buf::from_bytes_unchecked(self.as_wtf8().py_zfill(width)) - } + fn zfill(&self, width: isize, vm: &VirtualMachine) -> PyResult { + let filled = self + .as_wtf8() + .py_zfill(width) + .ok_or_else(|| vm.new_memory_error(""))?; + // SAFETY: this is safe-guaranteed because the original self.as_wtf8() is valid wtf8 + Ok(unsafe { Wtf8Buf::from_bytes_unchecked(filled) }) } #[inline] @@ -1332,7 +1334,7 @@ impl PyStr { &self, width: isize, fillchar: OptionalArg, - pad: fn(&Wtf8, usize, CodePoint, usize) -> Wtf8Buf, + pad: fn(&Wtf8, usize, CodePoint, usize) -> Option, vm: &VirtualMachine, ) -> PyResult { let fillchar = fillchar.map_or(Ok(' '.into()), |ref s| { @@ -1340,11 +1342,11 @@ impl PyStr { vm.new_type_error("The fill character must be exactly one character long") }) })?; - Ok(if self.len() as isize >= width { - self.as_wtf8().to_owned() - } else { - pad(self.as_wtf8(), width as usize, fillchar, self.len()) - }) + if self.len() as isize >= width { + return Ok(self.as_wtf8().to_owned()); + } + pad(self.as_wtf8(), width as usize, fillchar, self.len()) + .ok_or_else(|| vm.new_memory_error("")) } #[pymethod] @@ -2214,6 +2216,12 @@ impl AnyStrContainer for String { Self::with_capacity(capacity) } + fn try_with_capacity(capacity: usize) -> Option { + let mut s = Self::new(); + s.try_reserve_exact(capacity).ok()?; + Some(s) + } + fn push_str(&mut self, other: &str) { Self::push_str(self, other) } @@ -2327,6 +2335,12 @@ impl AnyStrContainer for Wtf8Buf { Self::with_capacity(capacity) } + fn try_with_capacity(capacity: usize) -> Option { + let mut s = Self::new(); + s.try_reserve_exact(capacity).ok()?; + Some(s) + } + fn push_str(&mut self, other: &Wtf8) { self.push_wtf8(other) } @@ -2447,6 +2461,12 @@ impl AnyStrContainer for AsciiString { Self::with_capacity(capacity) } + fn try_with_capacity(capacity: usize) -> Option { + let mut v = Vec::new(); + v.try_reserve_exact(capacity).ok()?; + Some(Self::from(v)) + } + fn push_str(&mut self, other: &AsciiStr) { Self::push_str(self, other) } diff --git a/crates/vm/src/builtins/tuple.rs b/crates/vm/src/builtins/tuple.rs index 7af176840b7..d510e35326f 100644 --- a/crates/vm/src/builtins/tuple.rs +++ b/crates/vm/src/builtins/tuple.rs @@ -342,10 +342,6 @@ impl PyTuple { } } - pub(crate) fn new_marshal_placeholder(len: usize, ctx: &Context) -> PyRef { - Self::new_ref(vec![ctx.none(); len], ctx) - } - /// # Safety /// This tuple must be a marshal placeholder which has not escaped the /// decoder, and `index` must not have been replaced previously. diff --git a/crates/vm/src/bytes_inner.rs b/crates/vm/src/bytes_inner.rs index 3c79ed3295d..65a9dc0a01c 100644 --- a/crates/vm/src/bytes_inner.rs +++ b/crates/vm/src/bytes_inner.rs @@ -78,7 +78,7 @@ impl ByteInnerNewOptions { } else { size as usize }; - Ok(vec![0; size].into()) + Ok(vm.new_zeroed_bytes(size)?.into()) } fn handle_object_fallback(obj: PyObjectRef, vm: &VirtualMachine) -> PyResult { @@ -576,16 +576,15 @@ impl PyBytesInner { fn _pad( &self, options: ByteInnerPaddingOptions, - pad: fn(&[u8], usize, u8, usize) -> Vec, + pad: PadFn, vm: &VirtualMachine, ) -> PyResult> { let (width, fillchar) = options.get_value("center", vm)?; let len = self.len(); - Ok(if len as isize >= width { - Vec::from(&self.elements[..]) - } else { - pad(&self.elements, width as usize, fillchar, len) - }) + if len as isize >= width { + return Ok(Vec::from(&self.elements[..])); + } + pad(&self.elements, width as usize, fillchar, len).ok_or_else(|| vm.new_memory_error("")) } pub fn center( @@ -821,8 +820,10 @@ impl PyBytesInner { self.elements.py_bytes_splitlines(options, into_wrapper) } - pub fn zfill(&self, width: isize) -> Vec { - self.elements.py_zfill(width) + pub fn zfill(&self, width: isize, vm: &VirtualMachine) -> PyResult> { + self.elements + .py_zfill(width) + .ok_or_else(|| vm.new_memory_error("")) } // len(self)>=1, from="", len(to)>=1, max_count>=1 @@ -1077,11 +1078,21 @@ impl AnyStrContainer<[u8]> for Vec { Self::with_capacity(capacity) } + fn try_with_capacity(capacity: usize) -> Option { + let mut v = Self::new(); + v.try_reserve_exact(capacity).ok()?; + Some(v) + } + fn push_str(&mut self, other: &[u8]) { self.extend(other) } } +/// A padding function from `AnyStr`, returning `None` for a width whose result +/// cannot be allocated. +type PadFn = fn(&[u8], usize, u8, usize) -> Option>; + const ASCII_WHITESPACES: [u8; 6] = [0x20, 0x09, 0x0a, 0x0c, 0x0d, 0x0b]; impl anystr::AnyChar for u8 { diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 69417ee61b7..d102b9a6d8e 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -9362,9 +9362,14 @@ impl ExecutingFrame<'_> { if has_data_descr { // Check for member descriptor (slot access) + // The slot offset only means anything on the layout the + // descriptor was defined for; the specialized instruction + // guards on the type version alone, so what descr_get() + // checks on every access has to be checked here instead. if let Some(ref descr) = cls_attr && let Some(member_descr) = descr.downcast_ref::() && let MemberGetter::Offset(offset) = member_descr.member.getter + && cls.fast_issubclass(&member_descr.common.typ) { unsafe { self.code @@ -11099,9 +11104,12 @@ impl ExecutingFrame<'_> { if has_data_descr { // Check for member descriptor (slot access) + // As in the load specialization, the offset is only valid for + // instances of the type the descriptor belongs to. if let Some(ref descr) = cls_attr && let Some(member_descr) = descr.downcast_ref::() && let MemberGetter::Offset(offset) = member_descr.member.getter + && cls.fast_issubclass(&member_descr.common.typ) { unsafe { self.code diff --git a/crates/vm/src/function/buffer.rs b/crates/vm/src/function/buffer.rs index c73f27c041d..dba97e9c77f 100644 --- a/crates/vm/src/function/buffer.rs +++ b/crates/vm/src/function/buffer.rs @@ -49,11 +49,41 @@ impl ArgBytesLike { f(&self.borrow_buf()) } + /// The bytes to hand to an operation that may wait, and whatever keeps + /// them readable while it does. + /// + /// `borrow_buf` may answer with a lock that every other thread writing to + /// the same object waits on, and a thread waiting on a lock never reaches + /// a safepoint, so keeping one across a wait for a peer, a pipe or a + /// signal stops the world from being stopped at all. Bytes reached that + /// way are copied out first. Bytes that lock nothing -- an immutable + /// object's -- are borrowed where they lie, which is all CPython holds in + /// either case. + pub fn borrow_buf_unlocked(&self, vm: &VirtualMachine) -> PyResult> { + let borrowed = self.borrow_buf(); + if !borrowed.is_locked() { + return Ok(UnlockedBuf::Borrowed(borrowed)); + } + let mut copy = Vec::new(); + copy.try_reserve_exact(borrowed.len()) + .map_err(|_| vm.new_memory_error(""))?; + copy.extend_from_slice(&borrowed); + Ok(UnlockedBuf::Copied(copy)) + } + #[must_use] pub const fn len(&self) -> usize { self.0.desc.len } + /// The width of one item. Callers that read the buffer as bytes rather + /// than as whatever it holds have to ask, since a contiguous buffer of + /// wider items is contiguous all the same. + #[must_use] + pub const fn itemsize(&self) -> usize { + self.0.desc.itemsize + } + #[must_use] pub const fn is_empty(&self) -> bool { self.len() == 0 @@ -63,6 +93,16 @@ impl ArgBytesLike { pub fn as_object(&self) -> &PyObject { &self.0.obj } + + /// The object whose storage is borrowed while this buffer is read: a view + /// borrows the object it looks at, not itself. + #[must_use] + pub fn source_object(&self) -> &PyObject { + self.0 + .obj + .downcast_ref::() + .map_or(&self.0.obj, |view| view.viewed_object()) + } } impl From for PyBuffer { @@ -113,6 +153,24 @@ impl<'a> TryFromBorrowedObject<'a> for ArgContiguousBytesLike { } } +/// Bytes that stay readable across a wait, from [`ArgBytesLike::borrow_buf_unlocked`]. +#[derive(Debug)] +pub enum UnlockedBuf<'a> { + Borrowed(BorrowedValue<'a, [u8]>), + Copied(Vec), +} + +impl core::ops::Deref for UnlockedBuf<'_> { + type Target = [u8]; + + fn deref(&self) -> &[u8] { + match self { + Self::Borrowed(b) => b, + Self::Copied(v) => v, + } + } +} + /// A memory buffer, read-write access. Like the `w*` format code for `PyArg_Parse` in CPython. #[derive(Debug, Traverse)] pub struct ArgMemoryBuffer(PyBuffer); @@ -139,6 +197,16 @@ impl ArgMemoryBuffer { pub const fn is_empty(&self) -> bool { self.len() == 0 } + + /// The object whose storage is borrowed while this buffer is written: a + /// view borrows the object it looks at, not itself. + #[must_use] + pub fn source_object(&self) -> &PyObject { + self.0 + .obj + .downcast_ref::() + .map_or(&self.0.obj, |view| view.viewed_object()) + } } impl From for PyBuffer { diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs index a9b8c7be171..e5eb3758950 100644 --- a/crates/vm/src/gc_state.rs +++ b/crates/vm/src/gc_state.rs @@ -8,7 +8,6 @@ use crate::object::{GC_NO_OWNER, GC_PERMANENT, GC_UNTRACKED, GcLink, GcOwner}; use crate::{AsObject, PyObject, PyObjectRef}; use core::ptr::NonNull; use core::sync::atomic::{AtomicBool, AtomicU16, AtomicU32, AtomicUsize, Ordering}; -use std::collections::HashSet; fn elapsed_secs( #[cfg(target_arch = "wasm32")] _start: (), @@ -155,6 +154,45 @@ fn is_owned_by(obj: &PyObject, owner: GcOwner) -> bool { #[derive(Clone, Copy, PartialEq, Eq, Hash)] struct GcPtr(NonNull); +/// Hashing for the tables a collection keys by an object's address. +/// +/// The default hasher is SipHash, which buys resistance against a caller +/// choosing keys that collide. Nothing chooses these keys: they are addresses +/// this process handed out, and the tables live and die inside one collection. +/// What a collection needs from them is speed -- it hashes every tracked +/// object and every edge between them -- so this runs the address through a +/// handful of multiplies and shifts instead. The shifts are what earns the +/// speed: a table picks its bucket from the low bits, and an address arrives +/// with its low bits zeroed by alignment, so entropy has to be carried +/// downward or every object lands in the same few buckets. +#[derive(Default)] +struct GcPtrHasher(u64); + +impl core::hash::Hasher for GcPtrHasher { + fn finish(&self) -> u64 { + self.0 + } + + fn write_usize(&mut self, value: usize) { + let mut z = (value as u64).wrapping_add(0x9E37_79B9_7F4A_7C15); + z = (z ^ (z >> 30)).wrapping_mul(0xBF58_476D_1CE4_E5B9); + z = (z ^ (z >> 27)).wrapping_mul(0x94D0_49BB_1331_11EB); + self.0 = z ^ (z >> 31); + } + + fn write(&mut self, bytes: &[u8]) { + // Addresses reach this hasher through `write_usize`; a key hashed any + // other way still has to land somewhere sensible. + for &byte in bytes { + self.0 = (self.0 ^ u64::from(byte)).wrapping_mul(0x0100_0000_01B3); + } + } +} + +type GcBuildHasher = core::hash::BuildHasherDefault; +type GcSet = std::collections::HashSet; +type GcMap = std::collections::HashMap; + /// RAII barrier that parks every other thread for the pointer-reading phases /// of a collection and lets them run again before finalizers execute. /// @@ -556,14 +594,25 @@ impl GcState { retired.sort_unstable(); retired }; - let mut collecting: HashSet = HashSet::new(); + // The candidates and their reference counts go in one table, not a set + // beside a map: every edge in the heap is looked up here, and the two + // held the same keys, so a second table only bought a second hash of + // the same address. `candidate_ptrs` keeps them in a walkable order, + // since the counts are written while the candidates are read. + let mut gc_refs: GcMap = GcMap::default(); + let mut candidate_ptrs: Vec = Vec::new(); for gen_list in &gen_locks { for obj in gen_list.iter() { if retired.binary_search(&obj.gc_owner()).is_ok() { obj.set_gc_owner(GC_NO_OWNER); } - if obj.strong_count() > 0 && is_owned_by(obj, owner) { - collecting.insert(GcPtr(NonNull::from(obj))); + let strong_count = obj.strong_count(); + let ptr = GcPtr(NonNull::from(obj)); + if strong_count > 0 + && is_owned_by(obj, owner) + && gc_refs.insert(ptr, strong_count).is_none() + { + candidate_ptrs.push(ptr); } } } @@ -583,7 +632,7 @@ impl GcState { .retain(|tag| retired.binary_search(tag).is_err()); } - if collecting.is_empty() { + if candidate_ptrs.is_empty() { // Reset counts for generations whose objects were promoted away. // For gen2 (oldest), survivors stay in-place so don't reset gen2 count. let reset_end = if generation >= 2 { 2 } else { generation + 1 }; @@ -602,26 +651,10 @@ impl GcState { }; } - let candidates = collecting.len(); + let candidates = candidate_ptrs.len(); if debug.contains(GcDebugFlags::STATS) { - eprintln!( - "gc: collecting {} objects from generations 0..={}", - collecting.len(), - generation - ); - } - - // Step 2: Build gc_refs map (copy reference counts) - let mut gc_refs: std::collections::HashMap = std::collections::HashMap::new(); - - #[expect( - clippy::iter_over_hash_type, - reason = "Iteration order doesn't matter here" - )] - for &ptr in &collecting { - let obj = unsafe { ptr.0.as_ref() }; - gc_refs.insert(ptr, obj.strong_count()); + eprintln!("gc: collecting {candidates} objects from generations 0..={generation}"); } // Step 3: Subtract internal references @@ -630,32 +663,31 @@ impl GcState { // of each object's children. Without this, a dict whose write lock is // held during one traversal but not the other can yield inconsistent // results, causing live objects to be incorrectly collected. - let mut referents_map: std::collections::HashMap>> = - std::collections::HashMap::new(); + // + // Every object's referents go in one buffer, with each object holding + // the range that is its own: a vector each would be an allocation per + // tracked object, and the collection wants them all at once anyway. + let mut referent_ptrs: Vec> = Vec::new(); + let mut referent_ranges: GcMap = GcMap::default(); - #[expect( - clippy::iter_over_hash_type, - reason = "Iteration order doesn't matter here" - )] - for &ptr in &collecting { + for &ptr in &candidate_ptrs { let obj = unsafe { ptr.0.as_ref() }; if obj.strong_count() == 0 { continue; } - let referent_ptrs = unsafe { obj.gc_get_referent_ptrs() }; - referents_map.insert(ptr, referent_ptrs.clone()); - for child_ptr in referent_ptrs { - let gc_ptr = GcPtr(child_ptr); - if collecting.contains(&gc_ptr) - && let Some(refs) = gc_refs.get_mut(&gc_ptr) - { + let start = referent_ptrs.len(); + unsafe { obj.gc_extend_referent_ptrs(&mut referent_ptrs) }; + let end = referent_ptrs.len(); + for &child_ptr in &referent_ptrs[start..end] { + if let Some(refs) = gc_refs.get_mut(&GcPtr(child_ptr)) { *refs = refs.saturating_sub(1); } } + referent_ranges.insert(ptr, (start, end)); } // Step 4: Find reachable objects (gc_refs > 0) and traverse from them - let mut reachable: HashSet = HashSet::new(); + let mut reachable: GcSet = GcSet::default(); let mut worklist: Vec = Vec::new(); #[expect( @@ -672,16 +704,21 @@ impl GcState { while let Some(ptr) = worklist.pop() { let obj = unsafe { ptr.0.as_ref() }; if obj.is_gc_tracked() { - // Reuse the pre-computed referent pointers from step 3. - // For objects that were skipped in step 3 (strong_count was 0), - // compute them now as a fallback. - let referent_ptrs = referents_map - .get(&ptr) - .cloned() - .unwrap_or_else(|| unsafe { obj.gc_get_referent_ptrs() }); - for child_ptr in referent_ptrs { + // Reuse the pre-computed referent pointers from step 3, in + // place: copying them out again costs a second pass over every + // edge in the heap. Objects skipped in step 3 (strong_count was + // 0) have none stored and are traversed here instead. + let computed; + let children: &[NonNull] = match referent_ranges.get(&ptr) { + Some(&(start, end)) => &referent_ptrs[start..end], + None => { + computed = unsafe { obj.gc_get_referent_ptrs() }; + &computed + } + }; + for &child_ptr in children { let gc_ptr = GcPtr(child_ptr); - if collecting.contains(&gc_ptr) && reachable.insert(gc_ptr) { + if gc_refs.contains_key(&gc_ptr) && reachable.insert(gc_ptr) { worklist.push(gc_ptr); } } @@ -689,7 +726,11 @@ impl GcState { } // Step 5: Find unreachable objects - let unreachable: Vec = collecting.difference(&reachable).copied().collect(); + let unreachable: Vec = candidate_ptrs + .iter() + .filter(|ptr| !reachable.contains(ptr)) + .copied() + .collect(); // With the world stopped, every frame on any thread's call stack is a // live root that is externally referenced and must have been @@ -702,7 +743,7 @@ impl GcState { // set_current_frame_nosave), not top_frame. #[cfg(all(unix, feature = "threading", debug_assertions))] if stw.is_stopped() { - let unreachable_set: HashSet = unreachable.iter().copied().collect(); + let unreachable_set: GcSet = unreachable.iter().copied().collect(); let mut cur = crate::vm::thread::get_current_frame(); while !cur.is_null() { let iframe = unsafe { &*cur }; @@ -802,7 +843,7 @@ impl GcState { } // 6b: Record initial strong counts (for resurrection detection) - let initial_counts: std::collections::HashMap = unreachable_refs + let initial_counts: GcMap = unreachable_refs .iter() .map(|obj| { let ptr = GcPtr(core::ptr::NonNull::from(obj.as_ref())); @@ -833,8 +874,8 @@ impl GcState { } // Detect resurrection - let mut resurrected_set: HashSet = HashSet::new(); - let unreachable_set: HashSet = unreachable.iter().copied().collect(); + let mut resurrected_set: GcSet = GcSet::default(); + let unreachable_set: GcSet = unreachable.iter().copied().collect(); for obj in &unreachable_refs { let ptr = GcPtr(core::ptr::NonNull::from(obj.as_ref())); @@ -874,7 +915,7 @@ impl GcState { // Compute collected count (exclude instance dicts in truly_dead) let collected = { - let dead_ptrs: HashSet = truly_dead + let dead_ptrs: GcSet = truly_dead .iter() .map(|obj| obj.as_ref() as *const PyObject as usize) .collect(); @@ -932,10 +973,9 @@ impl GcState { // never be observable through the generation lists, or another // thread could obtain a strong reference via gc.get_objects() // and access the cleared payload. - let mut late_resurrected: HashSet = HashSet::new(); + let mut late_resurrected: GcSet = GcSet::default(); if !save_all { - let mut expected_counts: std::collections::HashMap = - std::collections::HashMap::new(); + let mut expected_counts: GcMap = GcMap::default(); for obj_ref in &truly_dead { let obj = obj_ref.as_ref(); if obj.is_gc_tracked() { @@ -949,8 +989,7 @@ impl GcState { // the dead set; any surplus in strong_count means another thread // grabbed a reference before untracking (late resurrection) and // the object must not be cleared. - let mut referents: std::collections::HashMap>> = - std::collections::HashMap::new(); + let mut referents: GcMap>> = GcMap::default(); for obj_ref in &truly_dead { let referent_ptrs = unsafe { obj_ref.gc_get_referent_ptrs() }; for child_ptr in &referent_ptrs { diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index 1e36d57ab31..bdacb7c5b83 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -1957,11 +1957,20 @@ impl PyObject { /// and its contents haven't been modified. pub unsafe fn gc_get_referent_ptrs(&self) -> Vec> { let mut result = Vec::new(); + unsafe { self.gc_extend_referent_ptrs(&mut result) }; + result + } + + /// Append this object's referents to `out`, for a caller that holds many + /// objects' referents in one buffer rather than one buffer each. + /// + /// # Safety + /// Same as [`Self::gc_get_referent_ptrs`]. + pub unsafe fn gc_extend_referent_ptrs(&self, out: &mut Vec>) { // Traverse the entire object including dict and slots self.0.traverse(&mut |child: &Self| { - result.push(NonNull::from(child)); + out.push(NonNull::from(child)); }); - result } /// Pop edges from this object for cycle breaking. diff --git a/crates/vm/src/protocol/buffer.rs b/crates/vm/src/protocol/buffer.rs index cf60775f76f..050c568b7ac 100644 --- a/crates/vm/src/protocol/buffer.rs +++ b/crates/vm/src/protocol/buffer.rs @@ -641,6 +641,47 @@ impl BufferDescriptor { } } + /// Visit each item's byte range with the *first* dimension varying + /// fastest, which is the order a Fortran-ordered copy is written in. + /// `for_each_segment` visits in the opposite order and can hand over whole + /// rows at once; here every item is its own range, since consecutive items + /// in this order are a row apart. + pub fn for_each_segment_fortran(&self, mut f: F) + where + F: FnMut(Range), + { + if self.len == 0 { + return; + } + if self.ndim() == 0 { + f(self.offset..self.offset + self.itemsize as isize); + return; + } + let mut indices = vec![0usize; self.ndim()]; + loop { + let pos = self.offset + + indices + .iter() + .zip_eq(self.dim_desc.iter()) + .map(|(&i, &(_, stride, suboffset))| i as isize * stride + suboffset) + .sum::(); + f(pos..pos + self.itemsize as isize); + + let mut dim = 0; + loop { + indices[dim] += 1; + if indices[dim] < self.dim_desc[dim].0 { + break; + } + indices[dim] = 0; + dim += 1; + if dim == self.ndim() { + return; + } + } + } + } + fn _for_each_segment(&self, mut index: isize, dim: usize, f: &mut F) where F: FnMut(Range), diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index f32fce314c4..9aad883b0d4 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -1258,7 +1258,7 @@ mod _io { let current_size = self.readahead() as usize; - let mut out = vec![0u8; n]; + let mut out = vm.new_zeroed_bytes(n)?; let mut remaining = n; let mut written = 0; if current_size > 0 { @@ -1673,7 +1673,7 @@ mod _io { check_writable(&raw, vm)?; } - data.buffer = vec![0; buffer_size]; + data.buffer = vm.new_zeroed_bytes(buffer_size)?; if Self::READABLE { data.reset_read(); @@ -1938,7 +1938,7 @@ mod _io { if data.writable() { data.flush_rewind(vm)?; } - let mut v = vec![0; n]; + let mut v = vm.new_zeroed_bytes(n)?; data.reset_read(); let r = data .raw_read(Either::A(Some(&mut v)), 0..n, vm)? @@ -3364,14 +3364,17 @@ mod _io { *snapshot = Some((cookie.dec_flags, input_chunk.clone())); let decoded = vm.call_method(decoder, "decode", (input_chunk, cookie.need_eof))?; let decoded = check_decoded(decoded, vm)?; - let pos_is_valid = decoded - .as_wtf8() - .is_code_point_boundary(cookie.bytes_to_skip as usize); + // The position is stored both as a count of characters and as + // an offset in bytes, so both have to land inside what was + // just decoded: everything read back from here indexes it. + let num_to_skip = cookie.num_to_skip(); + let pos_is_valid = num_to_skip.chars <= decoded.char_len() + && decoded.as_wtf8().is_code_point_boundary(num_to_skip.bytes); textio.set_decoded_chars(Some(decoded)); if !pos_is_valid { return Err(vm.new_os_error("can't restore logical file position")); } - textio.decoded_chars_used = cookie.num_to_skip(); + textio.decoded_chars_used = num_to_skip; } else { textio.snapshot = Some((cookie.dec_flags, PyBytes::from(vec![]).into_ref(&vm.ctx))) } @@ -4813,8 +4816,20 @@ mod _io { } #[pymethod] - fn readinto(&self, obj: ArgMemoryBuffer, vm: &VirtualMachine) -> PyResult { - let mut buf = self.buffer(vm)?; + fn readinto(zelf: &Py, obj: ArgMemoryBuffer, vm: &VirtualMachine) -> PyResult { + // Reading locks this object, and a destination that views it locks + // it too, so such a destination is filled after the read is done. + if obj.source_object().is(zelf.as_object()) { + let mut data = vm.new_zeroed_bytes(obj.len())?; + let ret = zelf + .buffer(vm)? + .cursor + .read(&mut data) + .map_err(|_| vm.new_value_error("Error readinto from Take"))?; + obj.borrow_buf_mut()[..ret].copy_from_slice(&data[..ret]); + return Ok(ret); + } + let mut buf = zelf.buffer(vm)?; let ret = buf .cursor .read(&mut obj.borrow_buf_mut()) @@ -5767,7 +5782,7 @@ mod fileio { } let handle = zelf.get_fd(vm)?; let bytes = if let Some(read_byte) = read_byte.to_usize() { - let mut bytes = vec![0; read_byte]; + let mut bytes = vm.new_zeroed_bytes(read_byte)?; // Loop on EINTR (PEP 475) let n = loop { match vm.allow_threads(|| host_io::read_once(handle, &mut bytes)) { @@ -5811,6 +5826,26 @@ mod fileio { Ok(Some(bytes)) } + /// One `read()` into `buf`, retried on EINTR (PEP 475). `None` on EAGAIN. + fn read_once_into( + zelf: &Py, + handle: crt_fd::Borrowed<'_>, + buf: &mut [u8], + vm: &VirtualMachine, + ) -> PyResult> { + loop { + match vm.allow_threads(|| host_io::read_once(handle, buf)) { + Ok(n) => return Ok(Some(n)), + Err(e) if host_io::is_interrupted_error(&e) => { + vm.check_signals()?; + } + // Non-blocking mode: return None if EAGAIN + Err(e) if host_io::is_would_block_error(&e) => return Ok(None), + Err(e) => return Err(Self::io_error(zelf, e, vm)), + } + } + } + #[pymethod] fn readinto( zelf: &Py, @@ -5826,24 +5861,28 @@ mod fileio { let handle = zelf.get_fd(vm)?; - let mut buf = obj.borrow_buf_mut(); - // Loop on EINTR (PEP 475) - let ret = loop { - match vm.allow_threads(|| host_io::read_once(handle, &mut buf)) { - Ok(n) => break n, - Err(e) if host_io::is_interrupted_error(&e) => { - vm.check_signals()?; - continue; - } - // Non-blocking mode: return None if EAGAIN - Err(e) if host_io::is_would_block_error(&e) => { - return Ok(None); - } - Err(e) => return Err(Self::io_error(zelf, e, vm)), - } - }; - - Ok(Some(ret)) + if host_io::reads_without_waiting(handle) { + // The read answers from the file itself, so it returns without + // waiting on anyone; write where the caller asked directly. + // Seekability is not the question -- a pipe on Windows seeks. + let mut buf = obj.borrow_buf_mut(); + return Self::read_once_into(zelf, handle, &mut buf, vm); + } + + // A pipe, socket or terminal answers only when the other end + // writes, which may be never. Holding the export for the whole + // call is what keeps the target from being resized meanwhile, as a + // Py_buffer does; but reaching its bytes takes a lock that every + // other thread touching the same object waits on, and a thread + // waiting on a lock never reaches a safepoint, so holding that one + // across the wait stops the world from being stopped at all. Read + // aside and take the lock for the copy. + let mut scratch = vm.new_zeroed_bytes(obj.len())?; + let ret = Self::read_once_into(zelf, handle, &mut scratch, vm)?; + if let Some(n) = ret { + obj.borrow_buf_mut()[..n].copy_from_slice(&scratch[..n]); + } + Ok(ret) } #[pymethod] @@ -5861,9 +5900,14 @@ mod fileio { let handle = zelf.get_fd(vm)?; + // A pipe, socket or terminal takes the bytes only when the other + // end makes room, which may be never; see readinto above for what + // holding the source's lock across that wait costs. + let buf = obj.borrow_buf_unlocked(vm)?; + // Loop on EINTR (PEP 475) let len = loop { - match obj.with_ref(|b| vm.allow_threads(|| host_io::write_once(handle, b))) { + match vm.allow_threads(|| host_io::write_once(handle, &buf)) { Ok(n) => break n, Err(e) if host_io::is_interrupted_error(&e) => { vm.check_signals()?; diff --git a/crates/vm/src/stdlib/_thread.rs b/crates/vm/src/stdlib/_thread.rs index de942f947d3..79dce3d21ce 100644 --- a/crates/vm/src/stdlib/_thread.rs +++ b/crates/vm/src/stdlib/_thread.rs @@ -1207,9 +1207,9 @@ pub(crate) mod _thread { // fall back to top_iframe (may be a stack-allocated frame). let top = slot.top_frame.load(Ordering::Relaxed); if let Some(p) = core::ptr::NonNull::new(top) { - let py = unsafe { - &*Py::::from_payload_ptr(p.as_ptr()) - }; + // SAFETY: world stopped -> the owning thread is parked + // with this frame on its chain, so it is alive. + let py = unsafe { p.as_ref() }; Some((*id, py.to_owned())) } else { // Stack-allocated frame: materialize from top_iframe. diff --git a/crates/vm/src/stdlib/atexit.rs b/crates/vm/src/stdlib/atexit.rs index 891f8e5437b..0260b0f115d 100644 --- a/crates/vm/src/stdlib/atexit.rs +++ b/crates/vm/src/stdlib/atexit.rs @@ -3,7 +3,9 @@ pub(crate) use atexit::module_def; #[pymodule] mod atexit { - use crate::{AsObject, PyObjectRef, PyResult, VirtualMachine, function::FuncArgs}; + use crate::{ + AsObject, PyObjectRef, PyResult, VirtualMachine, common::rc::PyRc, function::FuncArgs, + }; #[pyfunction] fn register(func: PyObjectRef, args: FuncArgs, vm: &VirtualMachine) -> PyObjectRef { @@ -11,7 +13,7 @@ mod atexit { vm.state .atexit_funcs .lock() - .insert(0, Box::new((func.clone(), args))); + .insert(0, PyRc::new((func.clone(), args))); func } @@ -29,24 +31,26 @@ mod atexit { funcs.len() as isize - 1 }; while i >= 0 { - let (cb, entry_ptr) = { + let entry = { let funcs = vm.state.atexit_funcs.lock(); if i as usize >= funcs.len() { i = funcs.len() as isize; i -= 1; continue; } - let entry = &funcs[i as usize]; - (entry.0.clone(), &**entry as *const (PyObjectRef, FuncArgs)) + // Keep the entry alive for as long as it is being compared, so + // it cannot be dropped and have its address handed to a + // callback registered from within __eq__. + funcs[i as usize].clone() }; // Lock released: __eq__ can safely call atexit functions - let eq = vm.bool_eq(&func, &cb)?; + let eq = vm.bool_eq(&func, &entry.0)?; if eq { // The entry may have moved during __eq__. Search backward by identity. let mut funcs = vm.state.atexit_funcs.lock(); let mut j = (funcs.len() as isize - 1).min(i); while j >= 0 { - if core::ptr::eq(&**funcs.get(j as usize).unwrap(), entry_ptr) { + if PyRc::ptr_eq(funcs.get(j as usize).unwrap(), &entry) { funcs.remove(j as usize); i = j; break; @@ -70,7 +74,7 @@ mod atexit { let funcs: Vec<_> = core::mem::take(&mut *vm.state.atexit_funcs.lock()); // Callbacks stored in LIFO order, iterate forward for entry in funcs { - let (func, args) = *entry; + let (func, args) = PyRc::try_unwrap(entry).unwrap_or_else(|e| (*e).clone()); if let Err(e) = func.call(args, vm) { let exit = e.fast_isinstance(vm.ctx.exceptions.system_exit); let msg = func diff --git a/crates/vm/src/stdlib/marshal.rs b/crates/vm/src/stdlib/marshal.rs index ca92b444a4c..08a8f589a77 100644 --- a/crates/vm/src/stdlib/marshal.rs +++ b/crates/vm/src/stdlib/marshal.rs @@ -116,9 +116,6 @@ mod decl { )?; } - if !allow_code { - check_no_code(&value, vm)?; - } check_exact_type(&value, vm)?; let mut buf = Vec::new(); let mut refs = if version >= 3 { @@ -126,7 +123,7 @@ mod decl { } else { None }; - write_object(&mut buf, &value, &mut refs, version, vm)?; + write_object(&mut buf, &value, &mut refs, version, allow_code, vm)?; Ok(PyBytes::from(buf)) } @@ -185,6 +182,7 @@ mod decl { obj: &PyObjectRef, refs: &mut Option, version: i32, + allow_code: bool, vm: &VirtualMachine, ) -> PyResult<()> { write_object_depth( @@ -192,6 +190,7 @@ mod decl { obj, refs, version, + allow_code, vm, marshal::MAX_MARSHAL_STACK_DEPTH, ) @@ -202,6 +201,7 @@ mod decl { obj: &PyObjectRef, refs: &mut Option, version: i32, + allow_code: bool, vm: &VirtualMachine, depth: usize, ) -> PyResult<()> { @@ -322,20 +322,20 @@ mod decl { buf.write_u8(b'('); buf.write_u32(t.len() as u32); for elem in t.as_slice() { - write_object_depth(buf, elem, refs, version, vm, depth - 1)?; + write_object_depth(buf, elem, refs, version, allow_code, vm, depth - 1)?; } } else if let Some(l) = obj.downcast_ref::() { buf.write_u8(b'['); let items = l.borrow_vec(); buf.write_u32(items.len() as u32); for elem in items.iter() { - write_object_depth(buf, elem, refs, version, vm, depth - 1)?; + write_object_depth(buf, elem, refs, version, allow_code, vm, depth - 1)?; } } else if let Some(d) = obj.downcast_ref::() { buf.write_u8(b'{'); for (k, v) in d { - write_object_depth(buf, &k, refs, version, vm, depth - 1)?; - write_object_depth(buf, &v, refs, version, vm, depth - 1)?; + write_object_depth(buf, &k, refs, version, allow_code, vm, depth - 1)?; + write_object_depth(buf, &v, refs, version, allow_code, vm, depth - 1)?; } buf.write_u8(b'0'); // TYPE_NULL terminator } else if let Some(s) = obj.downcast_ref::() { @@ -343,16 +343,19 @@ mod decl { let elems = s.elements(); buf.write_u32(elems.len() as u32); for elem in &elems { - write_object_depth(buf, elem, refs, version, vm, depth - 1)?; + write_object_depth(buf, elem, refs, version, allow_code, vm, depth - 1)?; } } else if let Some(s) = obj.downcast_ref::() { buf.write_u8(b'>'); let elems = s.elements(); buf.write_u32(elems.len() as u32); for elem in &elems { - write_object_depth(buf, elem, refs, version, vm, depth - 1)?; + write_object_depth(buf, elem, refs, version, allow_code, vm, depth - 1)?; } } else if let Some(co) = obj.downcast_ref::() { + if !allow_code { + return Err(vm.new_value_error("marshalling code objects is disallowed")); + } buf.write_u8(b'c'); // `Literal` holds the exact object a constant was built from, so // route `co_consts` back through the object writer: it reaches the @@ -360,7 +363,7 @@ mod decl { // reference table the reader indexes against. marshal::serialize_code_with(buf, &co.code, |buf, constant| { let constant = PyObjectRef::from(constant.clone()); - write_object_depth(buf, &constant, refs, version, vm, depth - 1) + write_object_depth(buf, &constant, refs, version, allow_code, vm, depth - 1) })?; } else if let Some(sl) = obj.downcast_ref::() { if version < 5 { @@ -373,15 +376,17 @@ mod decl { sl.start.as_ref().unwrap_or(&none), refs, version, + allow_code, vm, depth - 1, )?; - write_object_depth(buf, &sl.stop, refs, version, vm, depth - 1)?; + write_object_depth(buf, &sl.stop, refs, version, allow_code, vm, depth - 1)?; write_object_depth( buf, sl.step.as_ref().unwrap_or(&none), refs, version, + allow_code, vm, depth - 1, )?; @@ -431,14 +436,36 @@ mod decl { struct PyMarshalBag<'a> { vm: &'a VirtualMachine, pending_error: &'a RefCell>, + allow_code: bool, } impl<'a> PyMarshalBag<'a> { fn new( vm: &'a VirtualMachine, pending_error: &'a RefCell>, + allow_code: bool, ) -> Self { - Self { vm, pending_error } + Self { + vm, + pending_error, + allow_code, + } + } + + /// Room for a container the decoder publishes before it reads what + /// goes in it. The length is the input's to choose, so the room is + /// asked for rather than assumed: a length no allocator can serve is + /// a MemoryError, not an aborted process. + fn placeholder_elements( + &self, + len: usize, + ) -> Result, marshal::MarshalError> { + let mut elements = Vec::new(); + elements + .try_reserve_exact(len) + .map_err(|_| self.remember_python_error(self.vm.new_memory_error("")))?; + elements.resize(len, self.vm.ctx.none()); + Ok(elements) } fn remember_python_error(&self, error: PyBaseExceptionRef) -> marshal::MarshalError { @@ -484,8 +511,12 @@ mod decl { fn make_tuple(&self, elements: impl Iterator) -> Self::Value { self.vm.ctx.new_tuple(elements.collect()).into() } - fn make_tuple_placeholder(&self, len: usize) -> Option { - Some(PyTuple::new_marshal_placeholder(len, &self.vm.ctx).into()) + fn make_tuple_placeholder( + &self, + len: usize, + ) -> Result, marshal::MarshalError> { + let elements = self.placeholder_elements(len)?; + Ok(Some(PyTuple::new_ref(elements, &self.vm.ctx).into())) } fn set_tuple_item( &self, @@ -501,8 +532,14 @@ mod decl { unsafe { tuple.set_marshal_item(index, value) }; Ok(()) } - fn make_code(&self, code: CodeObject) -> Self::Value { - crate::builtins::PyCode::new_ref_with_bag(self.vm, code).into() + fn make_code(&self, code: CodeObject) -> Result { + if !self.allow_code { + return Err(self.remember_python_error( + self.vm + .new_value_error("unmarshalling code objects is disallowed"), + )); + } + Ok(crate::builtins::PyCode::new_ref_with_bag(self.vm, code).into()) } fn make_stop_iter(&self) -> Result { Ok(self.vm.ctx.exceptions.stop_iteration.to_owned().into()) @@ -513,8 +550,12 @@ mod decl { ) -> Result { Ok(self.vm.ctx.new_list(it.collect()).into()) } - fn make_list_placeholder(&self, len: usize) -> Option { - Some(self.vm.ctx.new_list(vec![self.vm.ctx.none(); len]).into()) + fn make_list_placeholder( + &self, + len: usize, + ) -> Result, marshal::MarshalError> { + let elements = self.placeholder_elements(len)?; + Ok(Some(self.vm.ctx.new_list(elements).into())) } fn set_list_item( &self, @@ -635,13 +676,20 @@ mod decl { fn deserialize_value( rdr: &mut impl marshal::Read, + allow_code: bool, vm: &VirtualMachine, ) -> PyResult { let pending_error = RefCell::new(None); - match marshal::deserialize_value(rdr, PyMarshalBag::new(vm, &pending_error)) { + match marshal::deserialize_value(rdr, PyMarshalBag::new(vm, &pending_error, allow_code)) { Ok(value) => Ok(value), Err(error) => Err(pending_error.into_inner().unwrap_or_else(|| match error { marshal::MarshalError::Eof => vm.new_eof_error("marshal data too short"), + error @ marshal::MarshalError::NullObject => vm.new_type_error(error.to_string()), + error @ (marshal::MarshalError::BadSize(_) + | marshal::MarshalError::UnknownType + | marshal::MarshalError::InvalidRef) => { + vm.new_value_error(format!("bad marshal data ({error})")) + } _ => vm.new_value_error("bad marshal data"), })), } @@ -661,11 +709,7 @@ mod decl { let LoadsArgs { data, allow_code } = args; let buf = data.borrow_buf(); - let result = deserialize_value(&mut &buf[..], vm)?; - if !allow_code { - check_no_code(&result, vm)?; - } - Ok(result) + deserialize_value(&mut &buf[..], allow_code, vm) } #[derive(FromArgs)] @@ -685,54 +729,25 @@ mod decl { .try_into_value::(vm)?; let read_res = vm.call_method(&args.f, "read", ())?; let bytes = ArgBytesLike::try_from_object(vm, read_res)?; - let buf = bytes.borrow_buf(); - let mut rdr: &[u8] = &buf; - let len_before = rdr.len(); - let result = deserialize_value(&mut rdr, vm)?; - let consumed = len_before - rdr.len(); + // The borrow ends here: seek() below is the caller's, and reaching the + // same buffer from it would deadlock on a borrow still held. + let (result, consumed) = { + let buf = bytes.borrow_buf(); + let mut rdr: &[u8] = &buf; + let len_before = rdr.len(); + let result = deserialize_value(&mut rdr, args.allow_code, vm)?; + (result, len_before - rdr.len()) + }; // Seek file to just after the consumed bytes let new_pos = tell_before + consumed as i64; vm.call_method(&args.f, "seek", (new_pos,))?; - if !args.allow_code { - check_no_code(&result, vm)?; - } Ok(result) } /// Reject subclasses of marshallable types (int, float, complex, tuple, etc.). - /// Recursively check that no code objects are present. - fn check_no_code(obj: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - if obj.downcast_ref::().is_some() { - return Err(vm.new_value_error("unmarshalling code objects is disallowed")); - } - if let Some(tup) = obj.downcast_ref::() { - for elem in tup.as_slice() { - check_no_code(elem, vm)?; - } - } else if let Some(list) = obj.downcast_ref::() { - for elem in list.borrow_vec().iter() { - check_no_code(elem, vm)?; - } - } else if let Some(set) = obj.downcast_ref::() { - for elem in set.elements() { - check_no_code(&elem, vm)?; - } - } else if let Some(fset) = obj.downcast_ref::() { - for elem in fset.elements() { - check_no_code(&elem, vm)?; - } - } else if let Some(dict) = obj.downcast_ref::() { - for (k, v) in dict { - check_no_code(&k, vm)?; - check_no_code(&v, vm)?; - } - } - Ok(()) - } - fn check_exact_type(obj: &PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { let cls = obj.class(); // bool is a subclass of int but is marshallable diff --git a/crates/vm/src/stdlib/typevar.rs b/crates/vm/src/stdlib/typevar.rs index 3e2581406e8..b784d8799f6 100644 --- a/crates/vm/src/stdlib/typevar.rs +++ b/crates/vm/src/stdlib/typevar.rs @@ -923,11 +923,12 @@ pub(crate) mod typevar { impl Representable for ParamSpecArgs { #[inline(always)] fn repr_str(zelf: &crate::Py, vm: &VirtualMachine) -> PyResult { - // Check if origin is a ParamSpec - if let Ok(name) = zelf.__origin__.get_attr("__name__", vm) { - return Ok(format!("{name}.args", name = name.str(vm)?)); + // A ParamSpec origin is named; anything else is shown by its repr, + // which carries the recursion guard a Rust `{:?}` walk does not. + if let Some(param_spec) = zelf.__origin__.downcast_ref::() { + return Ok(format!("{}.args", param_spec.__name__().str_utf8(vm)?)); } - Ok(format!("{:?}.args", zelf.__origin__)) + Ok(format!("{}.args", zelf.__origin__.repr(vm)?)) } } @@ -986,11 +987,12 @@ pub(crate) mod typevar { impl Representable for ParamSpecKwargs { #[inline(always)] fn repr_str(zelf: &crate::Py, vm: &VirtualMachine) -> PyResult { - // Check if origin is a ParamSpec - if let Ok(name) = zelf.__origin__.get_attr("__name__", vm) { - return Ok(format!("{name}.kwargs", name = name.str(vm)?)); + // A ParamSpec origin is named; anything else is shown by its repr, + // which carries the recursion guard a Rust `{:?}` walk does not. + if let Some(param_spec) = zelf.__origin__.downcast_ref::() { + return Ok(format!("{}.kwargs", param_spec.__name__().str_utf8(vm)?)); } - Ok(format!("{:?}.kwargs", zelf.__origin__)) + Ok(format!("{}.kwargs", zelf.__origin__.repr(vm)?)) } } diff --git a/crates/vm/src/types/slot.rs b/crates/vm/src/types/slot.rs index fc9d1b04885..c0b9142c780 100644 --- a/crates/vm/src/types/slot.rs +++ b/crates/vm/src/types/slot.rs @@ -527,7 +527,11 @@ pub fn hash_not_implemented(zelf: &PyObject, vm: &VirtualMachine) -> PyResult PyResult { - vm.call_special_method(zelf, identifier!(vm, __call__), args) + // `__call__` can name the object being called, and dispatching it pushes no + // Python frame, so nothing else counts the nesting. + vm.with_recursion("while calling a Python object", || { + vm.call_special_method(zelf, identifier!(vm, __call__), args) + }) } fn getattro_wrapper(zelf: &PyObject, name: &Py, vm: &VirtualMachine) -> PyResult { @@ -616,7 +620,11 @@ fn descr_get_wrapper( cls: Option, vm: &VirtualMachine, ) -> PyResult { - vm.call_special_method(&zelf, identifier!(vm, __get__), (obj, cls)) + // A descriptor whose `__get__` is the descriptor itself resolves it by + // fetching `__get__` again, and none of that pushes a Python frame. + vm.with_recursion("while calling a Python object", || { + vm.call_special_method(&zelf, identifier!(vm, __get__), (obj, cls)) + }) } fn descr_set_wrapper( diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index c3861797b24..54d3e813eec 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -100,6 +100,11 @@ pub struct VirtualMachine { pub state: PyRc, pub initialized: bool, recursion_depth: Cell, + /// Depth of native recursion that pushes no Python frame, counted only + /// where the stack pointer cannot be read. Everywhere else the native + /// stack itself answers, and nothing needs counting. + #[cfg(any(miri, target_env = "musl"))] + native_recursion_depth: Cell, /// C stack soft limit for detecting stack overflow (like c_stack_soft_limit) #[cfg_attr(any(miri, target_env = "musl"), allow(dead_code))] c_stack_soft_limit: Cell, @@ -384,7 +389,7 @@ impl StopTheWorldState { /// is only ever `try_lock`'d. The active requester therefore force-parks /// this thread, finishes its whole stop→start span, releases the exclusion, /// and only then does this thread resume and acquire it. - fn acquire_exclusion(&self) { + fn acquire_exclusion(&self, state: &PyGlobalState) { if self .exclusion .compare_exchange(false, true, Ordering::AcqRel, Ordering::Relaxed) @@ -393,7 +398,7 @@ impl StopTheWorldState { return; } loop { - crate::vm::thread::suspend_if_needed(self); + crate::vm::thread::suspend_if_needed(state); std::thread::yield_now(); if self .exclusion @@ -421,7 +426,7 @@ impl StopTheWorldState { /// drives the stop→start span at a time; it is released by /// `start_the_world`/`reset_after_fork`. pub fn stop_the_world(&self, state: &PyGlobalState) { - self.acquire_exclusion(); + self.acquire_exclusion(state); let start = std::time::Instant::now(); let requester_ident = crate::stdlib::_thread::get_ident(); self.requester.store(requester_ident, Ordering::Relaxed); @@ -759,7 +764,10 @@ pub struct PyGlobalState { pub stacksize: AtomicCell, pub thread_count: AtomicCell, pub hash_secret: HashSecret, - pub atexit_funcs: PyMutex>>, + /// Registered `atexit` callbacks, newest first. Shared ownership so + /// `atexit.unregister` can keep the entry it is comparing alive while the + /// list is unlocked, and still recognize it afterwards by identity. + pub atexit_funcs: PyMutex>>, pub codec_registry: CodecsRegistry, pub finalizing: AtomicBool, pub warnings: WarningsState, @@ -991,6 +999,8 @@ impl VirtualMachine { state, initialized: false, recursion_depth: Cell::new(0), + #[cfg(any(miri, target_env = "musl"))] + native_recursion_depth: Cell::new(0), c_stack_soft_limit: Cell::new(Self::calculate_c_stack_soft_limit()), async_gen_firstiter: RefCell::new(None), async_gen_finalizer: RefCell::new(None), @@ -2004,6 +2014,14 @@ impl VirtualMachine { const STACK_MARGIN_BYTES: usize = (if cfg!(debug_assertions) { 16384 } else { 4096 }) * core::mem::size_of::(); + /// How deep native recursion may go where the stack cannot be measured + /// (`Py_C_RECURSION_LIMIT`). A native step costs far more stack than a + /// Python one and debug builds cost more again, so this sits well under + /// what a default stack holds rather than at what it would just fit. + #[cfg(any(miri, target_env = "musl"))] + const NATIVE_RECURSION_LIMIT_UNMEASURED: usize = + if cfg!(debug_assertions) { 500 } else { 1500 }; + /// Get the stack boundaries using platform-specific APIs. /// Returns (base, top) where base is the lowest address and top is the highest. #[cfg(all(not(miri), not(target_env = "musl"), windows))] @@ -2105,16 +2123,34 @@ impl VirtualMachine { /// Used to run the body of a (possibly) recursive function. It will raise a /// RecursionError if recursive functions are nested far too many times, /// preventing a stack overflow. + /// `Py_EnterRecursiveCall`: bounds native recursion that pushes no Python + /// frame, against the native stack. That is a separate budget from the + /// frame limit `sys.setrecursionlimit()` sets, so nesting counted here does + /// not come out of what Python code has left to call with. pub fn with_recursion PyResult>(&self, _where: &str, f: F) -> PyResult { - self.check_recursive_call(_where)?; - - // Native stack guard: check C stack like _Py_MakeRecCheck - if self.check_c_stack_overflow() { - return Err(self.new_recursion_error(_where.to_string())); + // `check_c_stack_overflow()` answers no unconditionally where the stack + // pointer cannot be read, which would leave this guard with nothing to + // stop. A count of the nesting stands in for the measurement there. + #[cfg(any(miri, target_env = "musl"))] + let counted_too_deep = + self.native_recursion_depth.get() >= Self::NATIVE_RECURSION_LIMIT_UNMEASURED; + #[cfg(not(any(miri, target_env = "musl")))] + let counted_too_deep = false; + + if counted_too_deep || self.check_c_stack_overflow() { + return Err( + self.new_recursion_error(format!("maximum recursion depth exceeded {_where}")) + ); } - self.recursion_depth.update(|d| d + 1); - scopeguard::defer! { self.recursion_depth.update(|d| d - 1) } + #[cfg(any(miri, target_env = "musl"))] + let _native_depth_guard = { + self.native_recursion_depth.update(|d| d + 1); + scopeguard::guard((), |()| { + self.native_recursion_depth.update(|d| d.saturating_sub(1)) + }) + }; + f() } @@ -2603,12 +2639,28 @@ impl VirtualMachine { // Objects/listobject.c. Each branch takes an atomic snapshot to avoid // race conditions from concurrent mutation (no GIL). let cls = value.class(); - let list_borrow; let slice = if cls.is(self.ctx.types.tuple_type) { value.downcast_ref::().unwrap().as_slice() } else if cls.is(self.ctx.types.list_type) { - list_borrow = value.downcast_ref::().unwrap().borrow_vec(); - &list_borrow + // The list is re-read on every step, the way map_iterable_object() + // does it: func() runs Python, which can mutate or even clear the + // same list, and a borrow held across that call deadlocks it. + let list = value.downcast_ref::().unwrap(); + let mut results = Vec::new(); + let mut i = 0; + loop { + let elem = { + let elements = list.borrow_vec(); + let Some(elem) = elements.get(i) else { + break; + }; + elem.clone() + // free the lock + }; + results.push(func(elem)?); + i += 1; + } + return Ok(results); } else if cls.is(self.ctx.types.dict_type) { let keys = value.downcast_ref::().unwrap().keys_vec(); return keys.into_iter().map(func).collect(); @@ -2797,7 +2849,7 @@ impl VirtualMachine { // Suspend this thread if stop-the-world is in progress #[cfg(feature = "threading")] - thread::suspend_if_needed(&self.state.stop_the_world); + thread::suspend_if_needed(&self.state); // Pass a QSBR checkpoint if requested (deferred memory reclamation). #[cfg(feature = "threading")] diff --git a/crates/vm/src/vm/thread.rs b/crates/vm/src/vm/thread.rs index 3b378acd6d7..3f83d88fe70 100644 --- a/crates/vm/src/vm/thread.rs +++ b/crates/vm/src/vm/thread.rs @@ -5,11 +5,13 @@ use crate::builtins::PyBaseExceptionRef; #[cfg(feature = "threading")] use alloc::sync::Arc; -#[cfg(all(unix, feature = "threading"))] -use crate::frame::FrameObject; use crate::frame::InterpreterFrame; +#[cfg(feature = "threading")] +use crate::vm::PyGlobalState; use crate::{AsObject, PyObject, VirtualMachine}; #[cfg(all(unix, feature = "threading"))] +use crate::{Py, frame::FrameObject}; +#[cfg(all(unix, feature = "threading"))] use core::sync::atomic::AtomicPtr; use core::{ cell::{Cell, RefCell}, @@ -44,7 +46,7 @@ pub struct ThreadSlot { /// thread at a safepoint and supplies the happens-before edge, so the /// pointer and the frames it reaches are quiescent and alive at read time. #[cfg(unix)] - pub top_frame: AtomicPtr, + pub top_frame: AtomicPtr>, /// Raw InterpreterFrame pointer, published alongside top_frame so /// cross-thread readers (sys._current_frames) can materialize /// stack-allocated frames that have no FrameObject. @@ -114,7 +116,7 @@ thread_local! { /// initialized; the `Arc` in `CURRENT_THREAD_SLOT` keeps the /// pointee alive until `cleanup_current_thread_frames` clears this. #[cfg(all(unix, feature = "threading"))] - static CURRENT_TOP_FRAME_SLOT: Cell<*const AtomicPtr> = + static CURRENT_TOP_FRAME_SLOT: Cell<*const AtomicPtr>> = const { Cell::new(core::ptr::null()) }; /// Cached pointer to this thread's `ThreadSlot::top_iframe` for the hot @@ -536,9 +538,10 @@ fn attach_thread(vm: &VirtualMachine) { // a thread doing rapid allow_threads calls from re-attaching and running // past the requester forever, which would stall stop-the-world. Done // outside the CURRENT_THREAD_SLOT borrow above because suspend re-borrows - // it. Safe against a concurrent start_the_world: suspend_if_needed only - // parks while the request is still live and self-recovers otherwise. - suspend_if_needed(&vm.state.stop_the_world); + // it. Safe against a concurrent start_the_world: suspend_if_needed decides + // whether to park under the registry lock, so it never parks after the + // request has been withdrawn. + suspend_if_needed(&vm.state); } /// Transition ATTACHED → DETACHED (like `_PyThreadState_Detach`). @@ -605,102 +608,111 @@ pub fn allow_threads(_vm: &VirtualMachine, f: impl FnOnce() -> R) -> R { /// Transitions ATTACHED → SUSPENDED and waits until released /// (like `_PyThreadState_Suspend` + `_PyThreadState_Attach`). #[cfg(feature = "threading")] -pub fn suspend_if_needed(stw: &super::StopTheWorldState) { +pub fn suspend_if_needed(state: &PyGlobalState) { let should_suspend = CURRENT_THREAD_SLOT.with(|slot| { slot.borrow() .as_ref() .is_some_and(|s| s.stop_requested.load(Ordering::Relaxed)) }); - if !should_suspend { - return; + if should_suspend { + do_suspend(state); } - - if !stw.requested.load(Ordering::Acquire) { - CURRENT_THREAD_SLOT.with(|slot| { - if let Some(s) = slot.borrow().as_ref() { - s.stop_requested.store(false, Ordering::Release); - } - }); - return; - } - - do_suspend(stw); } #[cfg(feature = "threading")] #[cold] -fn do_suspend(stw: &super::StopTheWorldState) { +fn do_suspend(state: &PyGlobalState) { + let stw = &state.stop_the_world; CURRENT_THREAD_SLOT.with(|slot| { - if let Some(s) = slot.borrow().as_ref() { - // ATTACHED → SUSPENDED - match s.state.compare_exchange( - THREAD_ATTACHED, - THREAD_SUSPENDED, - Ordering::AcqRel, - Ordering::Acquire, - ) { - Ok(_) => { - // Consumed this thread's stop request bit. - s.stop_requested.store(false, Ordering::Release); - } - Err(THREAD_DETACHED) => { - // Leaving VM; caller will re-check on next entry. - super::stw_trace(format_args!("suspend skip DETACHED")); - return; - } - Err(THREAD_SUSPENDED) => { - // Already parked by another path. - s.stop_requested.store(false, Ordering::Release); - super::stw_trace(format_args!("suspend skip already-suspended")); - return; - } - Err(state) => { - debug_assert!(false, "unexpected thread state in suspend: {state}"); - return; - } + let borrowed = slot.borrow(); + let Some(s) = borrowed.as_ref() else { + return; + }; + + // Decide whether to park while holding the thread registry. Both edges + // of `requested` are written under that lock: `init_thread_countdown` + // sets it, and `start_the_world` clears it and then releases every + // SUSPENDED thread without letting go. Publishing SUSPENDED here is + // therefore either seen by that release pass or never reached, which + // leaves the requester the only writer that takes a thread out of + // SUSPENDED. A completion check that observed this thread parked cannot + // then be invalidated by the thread resuming on its own. + let park = { + let _registry = state.thread_frames.lock(); + if stw.requested.load(Ordering::Acquire) { + Some(s.state.compare_exchange( + THREAD_ATTACHED, + THREAD_SUSPENDED, + Ordering::AcqRel, + Ordering::Acquire, + )) + } else { + // The stop already ended; this thread's request bit is stale. + s.stop_requested.store(false, Ordering::Release); + None } - super::stw_trace(format_args!("suspend ATTACHED->SUSPENDED")); + }; - // Re-check: if start_the_world already ran (cleared `requested`), - // no one will set us back to DETACHED — we must self-recover. - if !stw.requested.load(Ordering::Acquire) { - s.state.store(THREAD_ATTACHED, Ordering::Release); + match park { + None => { + super::stw_trace(format_args!("suspend skip not-requested")); + return; + } + Some(Ok(_)) => { + // Consumed this thread's stop request bit. + s.stop_requested.store(false, Ordering::Release); + } + Some(Err(THREAD_DETACHED)) => { + // Leaving VM; caller will re-check on next entry. + super::stw_trace(format_args!("suspend skip DETACHED")); + return; + } + Some(Err(THREAD_SUSPENDED)) => { + // Already parked by another path. s.stop_requested.store(false, Ordering::Release); - super::stw_trace(format_args!("suspend abort requested-cleared")); + super::stw_trace(format_args!("suspend skip already-suspended")); return; } + Some(Err(state)) => { + debug_assert!(false, "unexpected thread state in suspend: {state}"); + return; + } + } + super::stw_trace(format_args!("suspend ATTACHED->SUSPENDED")); - // Notify the stop-the-world requester that we've parked - stw.notify_suspended(); - super::stw_trace(format_args!("suspend notified-requester")); + // Notify the stop-the-world requester that we've parked. The registry + // is released first: the requester's wait loop takes the notify mutex + // and then the registry, so taking them the other way round here would + // invert the order. + stw.notify_suspended(); + super::stw_trace(format_args!("suspend notified-requester")); - // Wait until start_the_world sets us back to DETACHED - let wait_yields = wait_while_suspended(s); - stw.add_suspend_wait_yields(wait_yields); + // Wait until start_the_world sets us back to DETACHED + let wait_yields = wait_while_suspended(s); + stw.add_suspend_wait_yields(wait_yields); - // Re-attach (DETACHED → ATTACHED), tstate_wait_attach CAS loop. - loop { - match s.state.compare_exchange( - THREAD_DETACHED, - THREAD_ATTACHED, - Ordering::AcqRel, - Ordering::Acquire, - ) { - Ok(_) => break, - Err(THREAD_SUSPENDED) => { - let extra_wait = wait_while_suspended(s); - stw.add_suspend_wait_yields(extra_wait); - } - Err(THREAD_ATTACHED) => break, - Err(state) => { - debug_assert!(false, "unexpected post-suspend state: {state}"); - break; - } + // Re-attach (DETACHED → ATTACHED), tstate_wait_attach CAS loop. + loop { + match s.state.compare_exchange( + THREAD_DETACHED, + THREAD_ATTACHED, + Ordering::AcqRel, + Ordering::Acquire, + ) { + Ok(_) => break, + Err(THREAD_SUSPENDED) => { + let extra_wait = wait_while_suspended(s); + stw.add_suspend_wait_yields(extra_wait); + } + Err(THREAD_ATTACHED) => break, + Err(state) => { + debug_assert!(false, "unexpected post-suspend state: {state}"); + break; } } - s.stop_requested.store(false, Ordering::Release); - super::stw_trace(format_args!("suspend resume -> ATTACHED")); } + s.stop_requested.store(false, Ordering::Release); + super::stw_trace(format_args!("suspend resume -> ATTACHED")); }); } @@ -818,11 +830,8 @@ pub fn set_current_frame(frame: *const InterpreterFrame) -> *const InterpreterFr core::ptr::null_mut() } else { let frame_obj = unsafe { (*frame).frame_obj() }; - // The payload address, which is what the cross-thread - // reader hands to `Py::from_payload_ptr`. The `Py` address - // would be off by the object header. frame_obj.map_or(core::ptr::null_mut(), |py| { - core::ptr::from_ref::(py).cast_mut() + py as *const Py as *mut Py }) }; unsafe { &*slot }.store(fo_ptr, Ordering::Relaxed); @@ -971,7 +980,7 @@ pub fn reinit_frame_slot_after_fork(vm: &VirtualMachine) { core::ptr::null_mut() } else { match unsafe { (*top_iframe).frame_obj() } { - Some(fo) => core::ptr::from_ref::(fo).cast_mut(), + Some(fo) => fo as *const Py as *mut Py, None => core::ptr::null_mut(), } } @@ -1163,6 +1172,8 @@ impl VirtualMachine { state: self.state.clone(), initialized: self.initialized, recursion_depth: Cell::new(0), + #[cfg(any(miri, target_env = "musl"))] + native_recursion_depth: Cell::new(0), c_stack_soft_limit: Cell::new(Self::calculate_c_stack_soft_limit()), async_gen_firstiter: RefCell::new(None), async_gen_finalizer: RefCell::new(None), diff --git a/crates/vm/src/vm/vm_ops.rs b/crates/vm/src/vm/vm_ops.rs index 692444fc7de..dc31e508218 100644 --- a/crates/vm/src/vm/vm_ops.rs +++ b/crates/vm/src/vm/vm_ops.rs @@ -168,6 +168,27 @@ impl VirtualMachine { } } + /// `vec![0; len]` for a length that came from Python, where a request too + /// large to satisfy is a `MemoryError` rather than an aborted process. + /// + /// The bytes are left for the allocator to zero, so a large request costs + /// no more than the pages that are actually written to. + pub fn new_zeroed_bytes(&self, len: usize) -> PyResult> { + if len == 0 { + return Ok(Vec::new()); + } + let layout = + core::alloc::Layout::array::(len).map_err(|_| self.new_memory_error(""))?; + // SAFETY: `len` is not zero, so neither is the layout's size. + let ptr = unsafe { alloc::alloc::alloc_zeroed(layout) }; + if ptr.is_null() { + return Err(self.new_memory_error("")); + } + // SAFETY: `ptr` was just allocated by the global allocator for exactly + // this many bytes, and every one of them is initialized to zero. + Ok(unsafe { Vec::from_raw_parts(ptr, len, len) }) + } + /// Calling scheme used for binary operations: /// /// Order operations are tried until either a valid result or error: diff --git a/extra_tests/snippets/builtin_bytes.py b/extra_tests/snippets/builtin_bytes.py index 4f861364488..3cbed79c069 100644 --- a/extra_tests/snippets/builtin_bytes.py +++ b/extra_tests/snippets/builtin_bytes.py @@ -747,3 +747,22 @@ def __new__(cls, value): assert "123A".istitle(), f"{s}" assert not "123a".istitle(), f"{s}" assert not "123A\ta".istitle(), f"{s}" + + +def test_huge_size(): + # sizes that cannot be allocated are MemoryError, not an aborted process + for factory in (bytes, bytearray): + assert_raises(MemoryError, lambda factory=factory: factory(2**62)) + for meth in ("center", "ljust", "rjust", "zfill"): + assert_raises( + MemoryError, + lambda factory=factory, meth=meth: getattr(factory(b"a"), meth)( + 1 << 62 + ), + ) + assert_raises( + OverflowError, lambda factory=factory: factory(b"\ta").expandtabs(2**31) + ) + + +test_huge_size() diff --git a/extra_tests/snippets/builtin_hash.py b/extra_tests/snippets/builtin_hash.py index b3128cecc5a..818ee523f30 100644 --- a/extra_tests/snippets/builtin_hash.py +++ b/extra_tests/snippets/builtin_hash.py @@ -35,9 +35,10 @@ def __hash__(self): # slot dispatch is what recurses, so that is where the depth is checked. if sys.implementation.name == "rustpython": - # CPython, which also runs this snippet, survives this depth unguarded. + # Deep enough to reach the native stack guard; CPython, which also runs + # this snippet, dies on the same value. deep_tuple = () - for _ in range(sys.getrecursionlimit() * 2): + for _ in range(100_000): deep_tuple = (deep_tuple,) with assert_raises(RecursionError): hash(deep_tuple) diff --git a/extra_tests/snippets/builtin_memoryview.py b/extra_tests/snippets/builtin_memoryview.py index 8a3a194d96d..34928041cd2 100644 --- a/extra_tests/snippets/builtin_memoryview.py +++ b/extra_tests/snippets/builtin_memoryview.py @@ -514,3 +514,164 @@ def test_fortran_contiguity(): test_fortran_contiguity() + + +def test_cast_arguments(): + # cast() takes a native single character format, optionally '@'-prefixed; + # a zero-size format used to reach a division by zero. + assert memoryview(b"abcd").cast("@i").itemsize == 4 + for fmt in ("0s", "4s", " 0; a 0 used to divide by zero while + # checking the product against SSIZE_MAX + for shape in ([0], [0, 4], [4, 0], [-1, 4], [0, 0]): + assert_raises( + ValueError, lambda shape=shape: memoryview(b"abcd").cast("B", shape) + ) + + class Index: + def __index__(self): + return 4 + + for shape in ([2.0, 2], [Index()], ["4"]): + assert_raises( + TypeError, lambda shape=shape: memoryview(b"abcd").cast("B", shape) + ) + + assert memoryview(b"abcd").cast("B", [True, 4]).tolist() == [[97, 98, 99, 100]] + + +test_cast_arguments() + + +def test_negative_stride(): + # A reversed view starts at its last byte, so walking it from there runs + # off the front of the exported slice. + assert memoryview(b"dcba") == memoryview(b"abcd")[::-1] + assert memoryview(b"abcd")[::-1] == memoryview(b"dcba") + assert not memoryview(b"abcd") == memoryview(b"abcd")[::-1] + + b = bytearray(b"____") + memoryview(b)[0:4] = memoryview(b"abcd")[::-1] + assert b == bytearray(b"dcba"), b + + a = array.array("i", [1, 2, 3]) + assert memoryview(array.array("i", [3, 2, 1])) == memoryview(a)[::-1] + assert memoryview(a)[::-1].tolist() == [3, 2, 1] + + +test_negative_stride() + + +def test_write_through_same_object(): + # Reading the source and writing the destination lock the same object + # when they overlap, and converting a value runs Python that can reach it. + b = bytearray(b"abcd") + memoryview(b)[0:4] = b + assert b == bytearray(b"abcd"), b + + b = bytearray(b"abcd") + memoryview(b)[0:4] = memoryview(b)[::-1] + assert b == bytearray(b"dcba"), b + + b = bytearray(b"abcd") + memoryview(b)[0:2] = memoryview(b)[2:4] + assert b == bytearray(b"cdcd"), b + + b = bytearray(b"abcd") + view = memoryview(b) + + class Index: + def __index__(self): + view[1] = 66 + return 65 + + view[0] = Index() + assert b == bytearray(b"ABcd"), b + + +test_write_through_same_object() + + +def test_cast_between_non_byte_formats(): + # A cast re-divides bytes into items; going from one item type straight to + # another would reinterpret what is already there. + view = memoryview(b"abcd").cast("i") + for fmt in ("h", "i", "f"): + try: + view.cast(fmt) + except TypeError as e: + assert "cannot cast between two non-byte formats" in str(e), e + else: + raise AssertionError(f"expected TypeError for cast to {fmt!r}") + + # Either side being bytes is allowed. + assert view.cast("B").tolist() == [97, 98, 99, 100] + assert view.cast("b").format == "b" + assert view.cast("c").tolist() == [b"a", b"b", b"c", b"d"] + assert memoryview(b"abcd").cast("c").cast("i").format == "i" + + +def test_cast_to_zero_dim(): + # A zero-dimensional view holds exactly one item, so the buffer has to be + # that one item and no more. + assert memoryview(b"abcd").cast("I", shape=()).tobytes() == b"abcd" + assert memoryview(b"a").cast("B", shape=()).tobytes() == b"a" + + for source, fmt in ((b"abcd", "B"), (b"abcdefgh", "I"), (b"ab", "b")): + try: + memoryview(source).cast(fmt, shape=()) + except TypeError as e: + assert "product(shape) * itemsize != buffer size" in str(e), e + else: + raise AssertionError(f"expected TypeError for {source!r} as {fmt!r}") + + +def test_hash_restricted_to_byte_formats(): + # The hash is over the bytes, so it agrees with the hash of those bytes + # only where an item is a byte. + data = b"abcdefgh" + assert hash(memoryview(data)) == hash(data) + assert hash(memoryview(data).cast("c")) == hash(data) + assert hash(memoryview(data).cast("b")) == hash(data) + + for fmt in ("I", "i", "h", "d"): + try: + hash(memoryview(data).cast(fmt)) + except ValueError as e: + assert "hashing is restricted to formats" in str(e), e + else: + raise AssertionError(f"expected ValueError for format {fmt!r}") + + +def test_tobytes_order(): + view = memoryview(b"abcdefgh") + for order in (None, "C", "F", "A"): + assert view.tobytes(order=order) == b"abcdefgh", order + + # A multidimensional view is laid out C-contiguously, so a Fortran-ordered + # copy walks it down the columns instead. + grid = memoryview(b"abcdefgh").cast("B", shape=(2, 4)) + assert grid.tolist() == [[97, 98, 99, 100], [101, 102, 103, 104]] + assert grid.tobytes() == b"abcdefgh" + assert grid.tobytes(order="C") == b"abcdefgh" + assert grid.tobytes(order="A") == b"abcdefgh" + assert grid.tobytes(order="F") == b"aebfcgdh" + + cube = memoryview(b"abcdefgh").cast("B", shape=(2, 2, 2)) + assert cube.tobytes(order="F") == b"aecgbfdh" + + for order in ("Z", "c", "f", ""): + try: + view.tobytes(order=order) + except ValueError as e: + assert str(e) == "order must be 'C', 'F' or 'A'", e + else: + raise AssertionError(f"expected ValueError for order {order!r}") + + +test_cast_between_non_byte_formats() +test_cast_to_zero_dim() +test_hash_restricted_to_byte_formats() +test_tobytes_order() diff --git a/extra_tests/snippets/builtin_str.py b/extra_tests/snippets/builtin_str.py index 6eead5ddbfb..684bd66a1ff 100644 --- a/extra_tests/snippets/builtin_str.py +++ b/extra_tests/snippets/builtin_str.py @@ -900,3 +900,18 @@ class MyString(str): assert id(b) != id(b * 1) assert id(b) != id(1 * b) assert id(b) != id(b * 2) + + +def test_huge_width(): + # A width that cannot be allocated is a MemoryError, not an aborted + # process, and a tabsize wider than a C int does not fit at all. + for meth in ("center", "ljust", "rjust", "zfill"): + assert_raises(MemoryError, lambda meth=meth: getattr("a", meth)(1 << 62)) + assert_raises(OverflowError, lambda: "\ta".expandtabs(1 << 62)) + assert_raises(OverflowError, lambda: "\ta".expandtabs(2**31)) + # The widest tabsize that still fits is accepted. With no tab to expand + # there is nothing to lay out, so the width is never allocated. + assert "a".expandtabs(2**31 - 1) == "a" + + +test_huge_width() diff --git a/extra_tests/snippets/builtin_type.py b/extra_tests/snippets/builtin_type.py index 8cb0a09a215..15a330aea19 100644 --- a/extra_tests/snippets/builtin_type.py +++ b/extra_tests/snippets/builtin_type.py @@ -687,3 +687,33 @@ def foo(): code = compile(stmts, "", "exec") assert code.co_names == ("blah", "foo") + + +# A slot descriptor carries the layout it was defined for. Reached from another +# class, it has to report that rather than read the slot at its own offset, +# whether the access is fresh or has been seen often enough to be specialized. + + +class WideSlots: + __slots__ = ("s0", "s1", "s2", "s3", "s4", "s5", "s6", "s7") + + +class NarrowSlots: + __slots__ = ("only",) + + +class NoSlots: + __slots__ = () + + +NarrowSlots.borrowed = WideSlots.__dict__["s7"] +NoSlots.borrowed = WideSlots.__dict__["s7"] + +for owner in (NarrowSlots(), NoSlots()): + for _ in range(1000): + with assert_raises(TypeError): + owner.borrowed + with assert_raises(TypeError): + owner.borrowed = 1 + with assert_raises(TypeError): + del owner.borrowed diff --git a/extra_tests/snippets/recursion.py b/extra_tests/snippets/recursion.py index 2d3b2205d68..4b61a74b438 100644 --- a/extra_tests/snippets/recursion.py +++ b/extra_tests/snippets/recursion.py @@ -11,3 +11,36 @@ class Foo(object): # Since the default __str__ implementation calls __repr__ and __repr__ is # actually __str__, str(foo) should raise a RecursionError. assert_raises(RecursionError, str, foo) + + +# A __call__ that is the object being called dispatches through the call slot +# again, and none of that pushes a Python frame. + + +class Caller: + pass + + +caller = Caller() +Caller.__call__ = caller +assert_raises(RecursionError, caller) + + +# The same shape through the descriptor protocol: resolving the attribute +# fetches __get__, which is the descriptor itself. + + +class Descr: + pass + + +descr = Descr() +Descr.__get__ = descr +Descr.x = descr +try: + descr.x +except (RecursionError, TypeError): + # RecursionError here, TypeError from the call of a non-callable elsewhere + pass +else: + raise AssertionError("descr.x should not resolve") diff --git a/extra_tests/snippets/stdlib_array.py b/extra_tests/snippets/stdlib_array.py index ed2a8f22369..c2de6ac1ec8 100644 --- a/extra_tests/snippets/stdlib_array.py +++ b/extra_tests/snippets/stdlib_array.py @@ -143,3 +143,36 @@ def write(self, chunk): arr = array("b", range(128)) arr.tofile(_ReenteringWriter(arr)) assert len(arr) == 129 + + +def test_setitem_reentrant(): + # Converting the value runs Python, which can reach the array, so the + # array is not locked while it happens. + a = array("i", [1, 2, 3]) + + class Index: + def __index__(self): + a[1] = 9 + return 7 + + a[0] = Index() + assert a == array("i", [7, 9, 3]), a + + +test_setitem_reentrant() + + +def test_frombytes_of_itself(): + # Resizing is refused while a buffer is exported, before any lock is taken. + # The typecode is "b" so the view's items are bytes and the resize is what + # the call is refused for. + a = array("b", [1, 2, 3]) + m = memoryview(a) + with assert_raises(BufferError): + a.frombytes(m) + del m + + # A view of wider items is not a source of bytes at all. + wide = array("i", [1, 2, 3]) + with assert_raises(TypeError): + wide.frombytes(memoryview(wide)) diff --git a/extra_tests/snippets/stdlib_asyncio.py b/extra_tests/snippets/stdlib_asyncio.py index d54f84564a3..a6a55509036 100644 --- a/extra_tests/snippets/stdlib_asyncio.py +++ b/extra_tests/snippets/stdlib_asyncio.py @@ -72,4 +72,31 @@ def __new__(cls, *args): asyncio.InvalidStateError = saved_invalid_state_error asyncio.exceptions.InvalidStateError = saved_invalid_state_error +# The awaited-by set is built with the waiter's __hash__, which can come back +# to the same future; the field must not be locked while that runs. + + +class Reentrant: + def __hash__(self): + _asyncio.future_add_to_awaited_by(awaited, Reentrant()) + return 1 + + def __eq__(self, other): + return self is other + + +awaited = _asyncio.Future(loop=object()) +_asyncio.future_add_to_awaited_by(awaited, Reentrant()) +with assert_raises(RecursionError): + # converting the single waiter into a set hashes both of them + _asyncio.future_add_to_awaited_by(awaited, Reentrant()) + +plain = _asyncio.Future(loop=object()) +waiter = object() +_asyncio.future_add_to_awaited_by(plain, waiter) +_asyncio.future_add_to_awaited_by(plain, object()) +assert waiter in plain._asyncio_awaited_by +_asyncio.future_discard_from_awaited_by(plain, waiter) +assert waiter not in plain._asyncio_awaited_by + print("ok") diff --git a/extra_tests/snippets/stdlib_atexit.py b/extra_tests/snippets/stdlib_atexit.py new file mode 100644 index 00000000000..de490c569df --- /dev/null +++ b/extra_tests/snippets/stdlib_atexit.py @@ -0,0 +1,101 @@ +"""atexit.unregister() compares callbacks with arbitrary Python code. + +The comparison runs with the callback list unlocked, so __eq__ may clear it +and register something new. unregister() then has to tell whether the entry +it compared is still there, and must not mistake a later registration that +happens to occupy the same storage for that entry. +""" + +import atexit + + +def make(name): + def f(): + ran.append(name) + + f.tag = name + return f + + +ran = [] +a, b, c, d = (make(n) for n in "abcd") + + +class Probe: + def __init__(self, action=None, result=True): + self.action = action + self.result = result + self.seen = [] + + def __eq__(self, other): + self.seen.append(getattr(other, "tag", "?")) + if self.action is not None: + self.action() + return self.result + + +def remaining(): + del ran[:] + atexit._run_exitfuncs() + return list(ran) + + +# A callback the probe does not match is left alone. +atexit._clear() +atexit.register(a) +atexit.register(b) +probe = Probe(result=False) +atexit.unregister(probe) +assert probe.seen == ["a", "b"], probe.seen +assert remaining() == ["b", "a"], ran + +# Matching callbacks are dropped, oldest compared first. +atexit._clear() +atexit.register(a) +atexit.register(b) +atexit.register(c) +probe = Probe(result=True) +atexit.unregister(probe) +assert probe.seen == ["a", "b", "c"], probe.seen +assert atexit._ncallbacks() == 0 +assert remaining() == [], ran + +# __eq__ empties the list: there is nothing left to drop. +atexit._clear() +atexit.register(a) +atexit.register(b) +probe = Probe(action=atexit._clear, result=True) +atexit.unregister(probe) +assert probe.seen == ["a"], probe.seen +assert remaining() == [], ran + +# __eq__ empties the list and registers a replacement. The replacement is a +# different callback, so it survives however its storage was reused. +atexit._clear() +atexit.register(a) +atexit.register(b) +atexit.register(c) + + +def replace(): + atexit._clear() + atexit.register(d) + + +probe = Probe(action=replace, result=True) +atexit.unregister(probe) +assert probe.seen == ["a", "d"], probe.seen +assert remaining() == ["d"], ran + +# __eq__ registers without clearing: every entry the walk had already passed +# stays, and so does each newly registered one. +atexit._clear() +atexit.register(a) +atexit.register(b) +probe = Probe(action=lambda: atexit.register(c), result=True) +atexit.unregister(probe) +assert probe.seen == ["a", "c"], probe.seen +assert remaining() == ["c", "c", "b", "a"], ran + +atexit._clear() +print("ok") diff --git a/extra_tests/snippets/stdlib_hashlib.py b/extra_tests/snippets/stdlib_hashlib.py index a463941b29a..f3400aed57d 100644 --- a/extra_tests/snippets/stdlib_hashlib.py +++ b/extra_tests/snippets/stdlib_hashlib.py @@ -2,6 +2,8 @@ import _sha1 import hashlib +from testutils import assert_raises + # print(hashlib.md5) h = hashlib.md5() h.update(b"a") @@ -56,3 +58,9 @@ assert _md5.md5(b"").hexdigest() == "d41d8cd98f00b204e9800998ecf8427e" assert _sha1.sha1(b"").hexdigest() == "da39a3ee5e6b4b0d3255bfef95601890afd80709" + +# a derived key wider than a C int does not fit, and never gets allocated. +# Which OverflowError comes out depends on the width of a C long: where it is +# narrower than the length asked for, converting the argument fails first. +with assert_raises(OverflowError): + hashlib.pbkdf2_hmac("sha256", b"password", b"salt", 1, 2**62) diff --git a/extra_tests/snippets/stdlib_io.py b/extra_tests/snippets/stdlib_io.py index f17eae5b172..8346ddbb62d 100644 --- a/extra_tests/snippets/stdlib_io.py +++ b/extra_tests/snippets/stdlib_io.py @@ -197,3 +197,48 @@ def __index__(self): f"cannot fit '{truncated_non_ascii_type_name}' into an index-sized integer", lambda: setattr(textio, "_CHUNK_SIZE", NonAsciiNamedChunkSize()), ) + + +# A buffer size or read size that cannot be allocated is a MemoryError, not an +# aborted process. +assert_raises(MemoryError, lambda: BufferedReader(BytesIO(b"a"), buffer_size=2**62)) +assert_raises(MemoryError, lambda: BufferedReader(BytesIO(b"a")).read(2**62)) +assert_raises(MemoryError, lambda: BufferedReader(BytesIO(b"a")).read1(2**62)) + + +def _text_cookie( + start_pos=0, + dec_flags=0, + bytes_to_feed=0, + chars_to_skip=0, + need_eof=0, + bytes_to_skip=0, +): + packed = ( + start_pos.to_bytes(8, "little", signed=True) + + dec_flags.to_bytes(4, "little", signed=True) + + bytes_to_feed.to_bytes(4, "little", signed=True) + + chars_to_skip.to_bytes(4, "little", signed=True) + + bytes([need_eof]) + + bytes_to_skip.to_bytes(4, "little", signed=True) + ) + return int.from_bytes(packed, "little") + + +# A cookie names a position both in characters and in bytes, and everything +# read back from it indexes what was decoded, so a position past the end is +# refused rather than stored. +for _bad in ( + _text_cookie(bytes_to_feed=10, chars_to_skip=1000, bytes_to_skip=0), + _text_cookie(bytes_to_feed=10, chars_to_skip=100000, bytes_to_skip=3), + _text_cookie(bytes_to_feed=10, chars_to_skip=1, bytes_to_skip=1000), +): + _textio = TextIOWrapper(BytesIO(b"hello world " * 20), encoding="utf-8") + _textio.read(1) + try: + _textio.seek(_bad) + except (OSError, OverflowError): + pass + else: + assert _textio.read(50) is not None + _textio.tell() diff --git a/extra_tests/snippets/stdlib_io_blocking_buffer.py b/extra_tests/snippets/stdlib_io_blocking_buffer.py new file mode 100644 index 00000000000..2119111dc2e --- /dev/null +++ b/extra_tests/snippets/stdlib_io_blocking_buffer.py @@ -0,0 +1,176 @@ +"""Transfers that wait for a peer must not hold the buffer they were given. + +A pipe or a socket answers when the other end does, which may be never. The +buffer is exported for the whole call, so it cannot be resized meanwhile, but +nothing else about it changes: another thread can still read it, write to it, +and the interpreter can still stop the world. An implementation that holds the +buffer's storage for the duration of the wait takes all of that away, and a +thread parked on that storage never reaches a safepoint, so a collection that +wants every thread stopped ends up waiting for the peer too. +""" + +import gc +import os +import socket +import threading +import time + +# The peer acts after DELAY; the checks below have to finish well inside it. +DELAY = 1.0 +SLACK = DELAY / 2 + + +def measure(buf, expected_len, writable): + """Time each operation on `buf` that does not need the peer, separately, so + a failure names the one that waited rather than the group.""" + elapsed = {} + + def timed(name, operation): + start = time.monotonic() + value = operation() + elapsed[name] = time.monotonic() - start + return value + + assert timed("len", lambda: len(buf)) == expected_len, len(buf) + assert isinstance(timed("bytes", lambda: bytes(buf)), bytes) + if writable: + timed("setitem", lambda: buf.__setitem__(0, buf[0])) + timed("gc.collect", gc.collect) + return elapsed + + +def run(buf, blocking_call, release_peer, writable): + started = threading.Event() + expected_len = len(buf) + result = [] + + def transfer(): + started.set() + result.append(blocking_call(buf)) + + def peer(): + time.sleep(DELAY) + release_peer() + + threads = [threading.Thread(target=transfer), threading.Thread(target=peer)] + for t in threads: + t.start() + started.wait() + time.sleep(0.2) # the transfer is now waiting on its peer + + elapsed = measure(buf, expected_len, writable) + waited = ["%s %.2fs" % item for item in elapsed.items() if item[1] >= SLACK] + assert not waited, "waited on the peer: " + ", ".join(waited) + + # The transfer is still in flight, so its export is still held and the + # buffer cannot be resized. An operating system that took the whole + # transfer without a peer leaves nothing here to observe. + assert not result, "the transfer finished without its peer" + try: + buf.append(0) + except BufferError: + pass + else: + raise AssertionError("append during an export should raise BufferError") + + for t in threads: + t.join() + return result[0] + + +# --- reading: the buffer is written into, so nothing else may touch it at all + + +read_fd, write_fd = os.pipe() +pipe = open(read_fd, "rb", buffering=0) +try: + target = bytearray(16) + n = run(target, pipe.readinto, lambda: os.write(write_fd, b"pipe"), writable=False) + assert n == 4, n + assert bytes(target[:4]) == b"pipe", bytes(target) +finally: + pipe.close() + os.close(write_fd) + +if hasattr(socket, "socketpair"): + left, right = socket.socketpair() + try: + target = bytearray(16) + n = run(target, left.recv_into, lambda: right.send(b"socket"), writable=False) + assert n == 6, n + assert bytes(target[:6]) == b"socket", bytes(target) + finally: + left.close() + right.close() + + +# --- writing: the buffer is only read, so it stays writable meanwhile + + +read_fd, write_fd = os.pipe() +sink = open(write_fd, "wb", buffering=0) +try: + # More than any pipe will hold, so the write cannot finish on its own. + source = bytearray(4 * 1024 * 1024) + drained = [] + + def drain(): + with open(read_fd, "rb", buffering=0) as f: + while True: + chunk = f.read(1 << 16) + if not chunk: + break + drained.append(len(chunk)) + + reader = threading.Thread(target=drain, daemon=True) + # One unbuffered write() reports what it transferred, which a signal can + # cut short, so the reader is measured against that rather than the source. + written = run(source, sink.write, reader.start, writable=True) + sink.close() + reader.join() + assert sum(drained) == written, (sum(drained), written) +finally: + if not sink.closed: + sink.close() + +if hasattr(socket, "socketpair"): + left, right = socket.socketpair() + try: + # How much a connection holds before it makes the sender wait is the + # operating system's to decide, and asking for a small send buffer does + # not settle it -- a socketpair is already connected, and on Windows it + # is a loopback pair whose receiver has a window of its own. So fill it + # until it refuses rather than guess a size that outruns it. + left.setblocking(False) + filled = 0 + while True: + try: + filled += left.send(bytes(1 << 16)) + except (BlockingIOError, InterruptedError): + break + left.setblocking(True) + + source = bytearray(1 << 16) + received = [] + + def receive(): + wanted = filled + len(source) + while sum(received) < wanted: + chunk = right.recv(1 << 16) + if not chunk: + break + received.append(len(chunk)) + + reader = threading.Thread(target=receive, daemon=True) + run(source, left.sendall, reader.start, writable=True) + reader.join() + assert sum(received) == filled + len(source), ( + sum(received), + filled, + len(source), + ) + finally: + left.close() + right.close() + +print("ok") diff --git a/extra_tests/snippets/stdlib_io_bytesio.py b/extra_tests/snippets/stdlib_io_bytesio.py index ba8ae20015e..9344c50d947 100644 --- a/extra_tests/snippets/stdlib_io_bytesio.py +++ b/extra_tests/snippets/stdlib_io_bytesio.py @@ -106,3 +106,11 @@ def test_07(): test_05() test_06() test_07() + + +# Reading into a buffer that views this same object locks it twice unless the +# read finishes first. +_bio = BytesIO(b"x" * 60) +assert _bio.readinto(_bio.getbuffer()) == 60 +_bio = BytesIO(b"x" * 60) +assert _bio.readinto(memoryview(_bio.getbuffer())) == 60 diff --git a/extra_tests/snippets/stdlib_marshal.py b/extra_tests/snippets/stdlib_marshal.py index 8881d3e0a7b..4e224fb313f 100644 --- a/extra_tests/snippets/stdlib_marshal.py +++ b/extra_tests/snippets/stdlib_marshal.py @@ -96,5 +96,63 @@ def test_roundtrip_shared_co_const(self): self.assertIs(loaded_code.co_consts[0], loaded_shared) +class AllowCodeTests(unittest.TestCase): + """allow_code is answered where a code object is written or read, so a + graph that walks back on itself is not a second walk of its own.""" + + def test_recursive_value(self): + recursive = [] + recursive.append(recursive) + loaded = marshal.loads( + marshal.dumps(recursive, allow_code=False), allow_code=False + ) + self.assertIs(loaded[0], loaded) + + def test_too_deeply_nested(self): + nested = [] + for _ in range(100_000): + nested = [nested] + with self.assertRaises(ValueError): + marshal.dumps(nested, allow_code=False) + + def test_code_is_rejected(self): + code = compile("1", "", "exec") + for value in (code, [code], (code,), {0: code}): + with self.assertRaises(ValueError): + marshal.dumps(value, allow_code=False) + data = marshal.dumps(value) + with self.assertRaises(ValueError): + marshal.loads(data, allow_code=False) + + +class BadDataTests(unittest.TestCase): + def test_container_size_out_of_range(self): + import struct + + # a length is signed, so the top bit set is out of range rather than + # four billion items to reserve room for + for marker in b"([<>": + data = bytes([marker | 0x80]) + struct.pack("H", "'H' format requires 0 <= number <= 65535"), + (">i", "'i' format requires -2147483648 <= number <= 2147483647"), + ("N", "'N' format requires 0 <= number <= 18446744073709551615"), + ("P", "int too large to convert"), +): + try: + struct.pack(fmt, 10**30) + except struct.error as e: + assert str(e) == message, (fmt, str(e)) + else: + raise AssertionError(f"expected struct.error for {fmt!r}") + +try: + struct.pack("B", "x") +except struct.error as e: + assert str(e) == "required argument is not an integer", e +else: + raise AssertionError("expected struct.error") + + +# __init__ reads a new format into a Struct that already holds one. +s = struct.Struct(">h") +s.__init__(">hh") +assert s.format == ">hh" +assert s.size == 4 +assert s.pack(1, 2) == b"\x00\x01\x00\x02" +assert s.unpack(b"\x00\x01\x00\x02") == (1, 2) + +# A format that cannot be read leaves the Struct as it was. +for bad in ("\udc00", "$"): + with assert_raises((UnicodeEncodeError, struct.error)): + s.__init__(bad) + assert s.format == ">hh" + assert s.pack(1, 2) == b"\x00\x01\x00\x02" + + +# A subclass may do its own __init__ and pass the format up. +class BigShort(struct.Struct): + def __init__(self): + super().__init__(">h") + + +assert BigShort().pack(12345) == b"\x30\x39" + +# Until __init__ runs there is no format to answer with. +blank = struct.Struct.__new__(struct.Struct) +assert blank.size == -1 +for call in ( + lambda: blank.format, + lambda: blank.pack(1), + lambda: blank.unpack(b"aa"), + lambda: blank.unpack_from(b"aaaa"), + lambda: blank.pack_into(bytearray(4), 0, 1), + lambda: blank.iter_unpack(b"aa"), + lambda: repr(blank), +): + with assert_raises(RuntimeError): + call() diff --git a/extra_tests/snippets/stdlib_threading_current_frames.py b/extra_tests/snippets/stdlib_threading_current_frames.py new file mode 100644 index 00000000000..e93a222148e --- /dev/null +++ b/extra_tests/snippets/stdlib_threading_current_frames.py @@ -0,0 +1,100 @@ +"""Take sys._current_frames() while other threads are running Python. + +The frame each thread is executing is published for cross-thread readers, and +_current_frames() takes a reference to it with the world stopped. A reader that +disagrees with the publisher about what the published pointer addresses reads +and reference-counts the wrong memory, which corrupts a neighbouring object +rather than failing at the read: the damage surfaces later, in the thread that +owns it, as a crash or a wedge. + +Workers therefore run ordinary Python calls (which publish a frame) in a tight +loop while the main thread hammers _current_frames(). +""" + +import sys +import threading +import time + +DURATION = 1.5 + + +def leaf(): + return sum(range(8)) + + +def nest(n): + if n: + return nest(n - 1) + return leaf() + + +def worker(stop): + while not stop.is_set(): + nest(16) + + +def frames_are_sane(frames): + # Every key is a thread id, every value a frame of this process. + for tid, frame in frames.items(): + assert isinstance(tid, int), tid + assert tid > 0, tid + assert type(frame).__name__ == "frame", frame + assert isinstance(frame.f_lineno, int), frame + assert isinstance(frame.f_code.co_name, str), frame + + +# The main thread sees itself where it stands. +me = sys._current_frames()[threading.get_ident()] +assert me is sys._getframe(), me + +stop = threading.Event() +threads = [threading.Thread(target=worker, args=(stop,)) for _ in range(4)] +for t in threads: + t.start() + +deadline = time.time() + DURATION +calls = 0 +while time.time() < deadline: + frames_are_sane(sys._current_frames()) + calls += 1 +stop.set() +for t in threads: + t.join() + +assert calls > 0, calls + + +# A thread parked in a call the main thread can name is reported inside it, +# with its callers reachable through f_back. +entered = threading.Event() +leave = threading.Event() +seen = [] + + +def g456(): + seen.append(threading.get_ident()) + entered.set() + leave.wait() + + +def f123(): + g456() + + +t = threading.Thread(target=f123) +t.start() +entered.wait() +try: + chain = [] + frame = sys._current_frames()[seen[0]] + while frame is not None: + chain.append(frame.f_code.co_name) + frame = frame.f_back + assert "g456" in chain, chain + assert "f123" in chain, chain + assert chain.index("g456") < chain.index("f123"), chain +finally: + leave.set() + t.join() + +print("ok") diff --git a/extra_tests/snippets/stdlib_types.py b/extra_tests/snippets/stdlib_types.py index 335069811a8..4bccd2985bf 100644 --- a/extra_tests/snippets/stdlib_types.py +++ b/extra_tests/snippets/stdlib_types.py @@ -47,14 +47,14 @@ def _run_missing_type_params_regression(): list[self_referential] nested = [0] - for _ in range(sys.getrecursionlimit() * 2): + for _ in range(100_000): nested = [nested] with assert_raises(RecursionError): list[nested] # hashing an alias walks the same shape deep_alias = int - for _ in range(sys.getrecursionlimit() * 2): + for _ in range(100_000): deep_alias = list[deep_alias] with assert_raises(RecursionError): hash(deep_alias) diff --git a/extra_tests/snippets/stdlib_typing.py b/extra_tests/snippets/stdlib_typing.py index 98d368c02cd..4082d683f8d 100644 --- a/extra_tests/snippets/stdlib_typing.py +++ b/extra_tests/snippets/stdlib_typing.py @@ -45,3 +45,21 @@ def method(self, value: Union[int, float]) -> Union[str, bytes]: assert _typing._idfunc(1) == 1 with assert_raises(TypeError): _typing._idfunc() + + +# ParamSpecArgs shows a non-ParamSpec origin by its repr, which is where the +# recursion guard lives; nesting them deeply must not walk the native stack. + +from typing import ParamSpec, ParamSpecArgs + +spec = ParamSpec("spec") +assert repr(spec.args) == "spec.args" +assert repr(spec.kwargs) == "spec.kwargs" + +nested = object() +for _ in range(2000): + nested = ParamSpecArgs(nested) +try: + repr(nested) +except RecursionError: + pass