From fcd4f4c8ae18b220168147b8d2c3fd5946636255 Mon Sep 17 00:00:00 2001 From: dot Date: Mon, 12 Oct 2026 10:39:35 +0900 Subject: [PATCH 1/7] Release bytearray initializer storage and check deletion indices Reset initializer storage before zero-count callbacks and returns. Validate integer deletion indices with a nonblocking snapshot before export rejection where possible, then revalidate under the resize guard. Preserve fast export rejection when contended to avoid codec callback deadlocks. Add Python regressions for exported-empty rejection, initializer capacity release, callback ordering, and ordinary deletion error precedence. Assisted-by: OpenAI Codex:model identifier unavailable --- crates/vm/src/builtins/bytearray.rs | 27 ++++++- .../builtin_bytearray_init_exports.py | 70 +++++++++++++++++++ 2 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 extra_tests/snippets/builtin_bytearray_init_exports.py diff --git a/crates/vm/src/builtins/bytearray.rs b/crates/vm/src/builtins/bytearray.rs index 4904f96fb..3186df3c0 100644 --- a/crates/vm/src/builtins/bytearray.rs +++ b/crates/vm/src/builtins/bytearray.rs @@ -140,7 +140,7 @@ impl PyByteArray { pub fn _delitem(&self, needle: &PyObject, vm: &VirtualMachine) -> PyResult<()> { match SequenceIndex::try_from_borrowed_object::(vm, needle)? { - SequenceIndex::Int(i) => self.try_resizable(vm)?.elements.delitem_by_index(vm, i), + SequenceIndex::Int(i) => self.delitem_by_index(i, vm), SequenceIndex::Slice(slice) => { // TODO: delete 0 elements don't need resizable self.try_resizable(vm)?.elements.delitem_by_slice(vm, slice) @@ -148,6 +148,27 @@ impl PyByteArray { } } + fn delitem_by_index(&self, index: isize, vm: &VirtualMachine) -> PyResult<()> { + // Validate before export rejection when the bytes are available without + // waiting. A codec callback may already hold a read guard with a writer + // queued, so a blocking read here could deadlock before that rejection. + let valid = self + .inner + .try_read() + .map(|inner| inner.elements.wrap_index(index).is_some()); + if valid == Some(false) { + return Err(vm.new_index_error("bytearray index out of range")); + } + let mut inner = self.try_resizable(vm)?; + // Another thread may have resized the bytearray between the locks. + let index = inner + .elements + .wrap_index(index) + .ok_or_else(|| vm.new_index_error("bytearray index out of range"))?; + inner.elements.remove(index); + Ok(()) + } + fn irepeat(zelf: &Py, n: isize, vm: &VirtualMachine) -> PyResult<()> { if n == 1 { return Ok(()); @@ -777,7 +798,7 @@ impl Initializer for PyByteArray { fn init(zelf: &Py, options: Self::Args, vm: &VirtualMachine) -> PyResult<()> { // Reject exports and clear existing contents before converting source. - zelf.try_resizable(vm)?.elements.clear(); + zelf.try_resizable(vm)?.elements = Vec::new(); // Conversion can call Python, so do not hold the lock across it and // check again for exports acquired by a callback or another thread. let (mut inner, zero_count) = @@ -918,7 +939,7 @@ impl AsSequence for PyByteArray { if let Some(value) = value { zelf._setitem_by_index(i, &value, vm) } else { - zelf.try_resizable(vm)?.elements.delitem_by_index(vm, i) + zelf.delitem_by_index(i, vm) } }), contains: atomic_func!(|seq, other, vm| { diff --git a/extra_tests/snippets/builtin_bytearray_init_exports.py b/extra_tests/snippets/builtin_bytearray_init_exports.py new file mode 100644 index 000000000..7587462c3 --- /dev/null +++ b/extra_tests/snippets/builtin_bytearray_init_exports.py @@ -0,0 +1,70 @@ +"""Initializer storage and integer-deletion ordering with buffer exports.""" + +from testutils import assert_raises + + +class Zero: + def __init__(self, value): + self.value = value + self.observations = [] + + def __index__(self): + self.observations.append((len(self.value), self.value.__alloc__())) + return 0 + + +# Final CPython rejects even an empty exported initializer before conversion. +for kind in ("missing", "integer", "callback"): + value = bytearray() + source = Zero(value) + with memoryview(value) as view: + with assert_raises(BufferError): + if kind == "missing": + value.__init__() + elif kind == "integer": + value.__init__(0) + else: + value.__init__(source) + assert value == b"" + assert view.tobytes() == b"" + assert source.observations == [] + +# Clearing an initializer must release its old storage before callbacks and +# before the successful zero-count early return. +for kind in ("missing", "integer", "callback"): + value = bytearray(b"x" * 64) + source = Zero(value) + assert value.__alloc__() > 0 + if kind == "missing": + value.__init__() + elif kind == "integer": + value.__init__(0) + else: + value.__init__(source) + assert source.observations == [(0, 0)] + assert value == b"" + assert value.__alloc__() == 0 + +# An invalid integer index is rejected before checking whether resizing is +# blocked, and neither error path may alter the exported bytes. +for initial, indices in ((b"abc", (3, -4)), (b"", (0, -1))): + for index in indices: + value = bytearray(initial) + with memoryview(value) as view: + try: + del value[index] + except IndexError as exc: + assert str(exc) == "bytearray index out of range" + else: + raise AssertionError("invalid index was accepted") + assert value == initial + assert view.tobytes() == initial + +for index in (0, -1): + value = bytearray(b"abc") + with memoryview(value) as view: + with assert_raises(BufferError): + del value[index] + assert view.tobytes() == b"abc" + del value[index] + assert value == (b"bc" if index == 0 else b"ab") From 9fdae0a3f7b443d38589c8d36497f823212e1c40 Mon Sep 17 00:00:00 2001 From: dot Date: Mon, 12 Oct 2026 11:20:19 +0900 Subject: [PATCH 2/7] Fix bytearray concatenation buffer callback reentrancy Acquire both operand exports before borrowing their bytes and release them in CPython order. Cover reentrant initialization, mutable callbacks and export cleanup through public concatenation APIs. Assisted-by: OpenAI Codex:model identifier unavailable --- crates/vm/src/builtins/bytearray.rs | 41 +++++++++- .../builtin_bytearray_concat_exports.py | 79 +++++++++++++++++++ 2 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 extra_tests/snippets/builtin_bytearray_concat_exports.py diff --git a/crates/vm/src/builtins/bytearray.rs b/crates/vm/src/builtins/bytearray.rs index 3186df3c0..819399612 100644 --- a/crates/vm/src/builtins/bytearray.rs +++ b/crates/vm/src/builtins/bytearray.rs @@ -918,10 +918,43 @@ impl AsSequence for PyByteArray { static AS_SEQUENCE: PySequenceMethods = PySequenceMethods { length: atomic_func!(|seq, _vm| Ok(PyByteArray::sequence_downcast(seq).__len__())), concat: atomic_func!(|seq, other, vm| { - PyByteArray::sequence_downcast(seq) - .inner() - .concat(other, vm) - .map(|x| PyByteArray::from(x).into_pyobject(vm)) + let concat_error = || { + vm.new_type_error(format!( + "can't concat {} to {}", + other.class().name(), + seq.obj.class().name() + )) + }; + // Protect the left operand before the right buffer callback, + // without holding either operand's lock across Python calls. + let left = PyBuffer::from_object(vm, seq.obj, BufferFlags::SIMPLE) + .map_err(|_| concat_error())?; + let right = match PyBuffer::from_object(vm, other, BufferFlags::SIMPLE) { + Ok(buffer) => buffer, + Err(_) => { + let error = concat_error(); + drop(left); + return Err(error); + } + }; + let result = (|| { + let len = left + .desc + .len + .checked_add(right.desc.len) + .ok_or_else(|| vm.no_memory_error())?; + let mut bytes = Vec::new(); + bytes + .try_reserve_exact(len) + .map_err(|_| vm.no_memory_error())?; + left.append_to(&mut bytes); + right.append_to(&mut bytes); + Ok(PyByteArray::from(bytes).into_pyobject(vm)) + })(); + // Buffer release can run Python too; all slice guards are gone. + drop(left); + drop(right); + result }), repeat: atomic_func!(|seq, n, vm| { PyByteArray::sequence_downcast(seq) diff --git a/extra_tests/snippets/builtin_bytearray_concat_exports.py b/extra_tests/snippets/builtin_bytearray_concat_exports.py new file mode 100644 index 000000000..b40889989 --- /dev/null +++ b/extra_tests/snippets/builtin_bytearray_concat_exports.py @@ -0,0 +1,79 @@ +import operator + +from testutils import assert_raises + +for concat in (operator.add, operator.concat, bytearray.__add__): + target = bytearray(b"a") + events = [] + + class ReentrantBuffer: + def __buffer__(self, flags): + events.append("acquire") + with assert_raises(BufferError): + target.__init__(object()) + with assert_raises(BufferError): + target.append(120) + target[0] = ord("A") + return memoryview(b"b") + + def __release_buffer__(self, view): + events.append("release") + # The left operand's export must already have been released. + target.append(99) + + assert concat(target, ReentrantBuffer()) == bytearray(b"Ab") + assert events == ["acquire", "release"] + assert target == bytearray(b"Ac") + target.append(100) + + class FailingBuffer: + def __buffer__(self, flags): + raise ValueError("buffer acquisition failed") + + with assert_raises(TypeError) as error: + concat(target, FailingBuffer()) + assert str(error.exception) == "can't concat FailingBuffer to bytearray" + target.append(101) + + events = [] + + class Left(bytearray): + def __buffer__(self, flags): + events.append("left acquire") + return memoryview(b"L") + + def __release_buffer__(self, view): + events.append("left release") + + class Right: + def __buffer__(self, flags): + events.append("right acquire") + return memoryview(b"R") + + def __release_buffer__(self, view): + events.append("right release") + + assert concat(Left(b"ignored"), Right()) == bytearray(b"LR") + assert events == [ + "left acquire", + "right acquire", + "left release", + "right release", + ] + + events = [] + with assert_raises(TypeError) as error: + concat(Left(), FailingBuffer()) + assert str(error.exception) == "can't concat FailingBuffer to Left" + assert events == ["left acquire", "left release"] + + class FailingLeft(bytearray): + def __buffer__(self, flags): + events.append("left acquire") + raise ValueError("left acquisition failed") + + events = [] + with assert_raises(TypeError) as error: + concat(FailingLeft(), Right()) + assert str(error.exception) == "can't concat Right to FailingLeft" + assert events == ["left acquire"] From a0fec1f8c0a6ad66a10cd751c18bc261f0b5a3a6 Mon Sep 17 00:00:00 2001 From: dot Date: Mon, 12 Oct 2026 12:00:20 +0900 Subject: [PATCH 3/7] Stage bytearray buffer callbacks outside receiver locks Keep operation-specific export and release ordering for comparisons, searches, affix tuples and translation. Resolve padding arguments before borrowing the receiver, and retain buffer owners through computation without holding bytearray locks across Python callbacks. Add bounded Python regressions for callback mutation, reentrant initialization, lazy affixes, subclass protocols and ordered cleanup. Assisted-by: OpenAI Codex:model identifier unavailable --- crates/vm/src/anystr.rs | 13 + crates/vm/src/builtins/bytearray.rs | 230 ++++++++++++++--- crates/vm/src/bytes_inner.rs | 32 ++- .../builtin_bytearray_callback_exports.py | 237 ++++++++++++++++++ 4 files changed, 467 insertions(+), 45 deletions(-) create mode 100644 extra_tests/snippets/builtin_bytearray_callback_exports.py diff --git a/crates/vm/src/anystr.rs b/crates/vm/src/anystr.rs index b9ce61250..1b7b20641 100644 --- a/crates/vm/src/anystr.rs +++ b/crates/vm/src/anystr.rs @@ -47,6 +47,19 @@ pub(crate) struct StartsEndsWithArgs { } impl StartsEndsWithArgs { + pub(crate) fn parts(&self, len: usize) -> (&PyObject, Option>) { + let range = if self.start.is_some() || self.end.is_some() { + Some(adjust_indices( + self.start.as_ref().map(AsRef::as_ref), + self.end.as_ref().map(AsRef::as_ref), + len, + )) + } else { + None + }; + (&self.affix, range) + } + pub(crate) fn get_value(self, len: usize) -> (PyObjectRef, Option>) { let range = if self.start.is_some() || self.end.is_some() { Some(adjust_indices( diff --git a/crates/vm/src/builtins/bytearray.rs b/crates/vm/src/builtins/bytearray.rs index 819399612..c463489df 100644 --- a/crates/vm/src/builtins/bytearray.rs +++ b/crates/vm/src/builtins/bytearray.rs @@ -6,7 +6,7 @@ use super::{ use crate::{ AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, TryFromObject, VirtualMachine, - anystr::{self, AnyStr}, + anystr::{self, AnyStr, StringRange}, atomic_func, byte::{bytearray_extend_from_object, bytearray_from_object, value_from_object}, bytes_inner::{ @@ -65,6 +65,21 @@ impl PyPayload for PyByteArray { } } +// Acquisitions can call Python, and releases must follow acquisition order. +// No bytearray lock may be held when this owner is dropped. +#[derive(Default)] +struct ByteArrayBuffers { + first: Option, + second: Option, +} + +impl Drop for ByteArrayBuffers { + fn drop(&mut self) { + drop(self.first.take()); + drop(self.second.take()); + } +} + /// Fill bytearray class methods dictionary. pub(crate) fn init(context: &'static Context) { PyByteArray::extend_class(context, context.types.bytearray_type); @@ -92,6 +107,105 @@ impl PyByteArray { PyDetachingRwLockWriteGuard::map(self.inner.write(), |inner| &mut inner.elements) } + fn with_export(&self, f: impl FnOnce(usize) -> R) -> R { + self.exports.fetch_add(1, Ordering::Release); + scopeguard::defer! { self.exports.fetch_sub(1, Ordering::Release); } + // Reading the length also waits for any earlier resize to finish. + f(self.__len__()) + } + + fn buffer_snapshot(buffer: &PyBuffer, vm: &VirtualMachine) -> PyResult> { + let mut bytes = Vec::new(); + bytes + .try_reserve_exact(buffer.desc.len) + .map_err(|_| vm.no_memory_error())?; + buffer.append_to(&mut bytes); + Ok(bytes) + } + + fn search_exported( + &self, + options: ByteInnerFindOptions, + search: impl FnOnce(&[u8], &[u8], core::ops::Range) -> R, + vm: &VirtualMachine, + ) -> PyResult { + self.with_export(|len| { + let (sub, range) = options.parts(len); + let buffer = if sub.check_buffer() { + Some(PyBuffer::from_object(vm, sub, BufferFlags::SIMPLE)?) + } else { + None + }; + let needle = match &buffer { + Some(buffer) => Self::buffer_snapshot(buffer, vm)?, + None => ByteInnerSub::try_from_object(vm, sub.to_owned())?.into_vec(vm)?, + }; + let result = search(&self.borrow_buf(), &needle, range); + drop(buffer); + Ok(result) + }) + } + + fn starts_ends_with( + &self, + options: anystr::StartsEndsWithArgs, + ends: bool, + vm: &VirtualMachine, + ) -> PyResult { + self.with_export(|len| { + let (affix, range) = options.parts(len); + let tailmatch = |item: &PyObject| -> PyResult { + // bytes subclasses use their bytes directly, as in tailmatch. + let buffer = if item.downcast_ref::().is_some() { + None + } else { + Some(PyBuffer::from_object(vm, item, BufferFlags::SIMPLE)?) + }; + let needle = match &buffer { + Some(buffer) => Self::buffer_snapshot(buffer, vm)?, + None => item.downcast_ref::().unwrap().as_bytes().to_vec(), + }; + let result = { + let bytes = self.borrow_buf(); + let slice = match &range { + Some(range) if !range.is_normal() => None, + Some(range) => bytes.get(range.clone()), + None => Some(&*bytes), + }; + slice.is_some_and(|slice| { + if ends { + slice.ends_with(&needle) + } else { + slice.starts_with(&needle) + } + }) + }; + drop(buffer); + Ok(result) + }; + if let Some(tuple) = affix.downcast_ref::() { + for item in tuple { + if tailmatch(item)? { + return Ok(true); + } + } + Ok(false) + } else { + tailmatch(affix).map_err(|error| { + if error.fast_isinstance(vm.ctx.exceptions.type_error) { + vm.new_type_error(format!( + "{} first arg must be bytes or a tuple of bytes, not {}", + if ends { "endswith" } else { "startswith" }, + affix.class().name() + )) + } else { + error + } + }) + } + }) + } + fn repeat(&self, value: isize, vm: &VirtualMachine) -> PyResult { self.inner().mul(value, vm).map(|x| x.into()) } @@ -512,7 +626,11 @@ impl Py { options: ByteInnerPaddingOptions, vm: &VirtualMachine, ) -> PyResult { - Ok(self.inner().center(options, vm)?.into()) + let (width, fillchar) = options.get_value("center", vm)?; + Ok(self + .inner() + .pad_with(width, fillchar, AnyStr::py_center, vm)? + .into()) } #[pymethod] @@ -521,7 +639,11 @@ impl Py { options: ByteInnerPaddingOptions, vm: &VirtualMachine, ) -> PyResult { - Ok(self.inner().ljust(options, vm)?.into()) + let (width, fillchar) = options.get_value("ljust", vm)?; + Ok(self + .inner() + .pad_with(width, fillchar, AnyStr::py_ljust, vm)? + .into()) } #[pymethod] @@ -530,14 +652,20 @@ impl Py { options: ByteInnerPaddingOptions, vm: &VirtualMachine, ) -> PyResult { - Ok(self.inner().rjust(options, vm)?.into()) + let (width, fillchar) = options.get_value("rjust", vm)?; + Ok(self + .inner() + .pad_with(width, fillchar, AnyStr::py_rjust, vm)? + .into()) } #[pymethod] fn count(&self, options: ByteInnerFindOptions, vm: &VirtualMachine) -> PyResult { - self.exports.fetch_add(1, Ordering::Release); - scopeguard::defer! { self.exports.fetch_sub(1, Ordering::Release); } - self.inner().count(options, vm) + self.search_exported( + options, + |bytes, needle, range| bytes.py_count(needle, range, |h, n| h.find_iter(n).count()), + vm, + ) } #[pymethod] @@ -555,19 +683,7 @@ impl Py { #[pymethod] fn endswith(&self, options: anystr::StartsEndsWithArgs, vm: &VirtualMachine) -> PyResult { - let borrowed = self.borrow_buf(); - let (affix, substr) = - match options.prepare(&*borrowed, borrowed.len(), |s, r| s.get_bytes(r)) { - Some(x) => x, - None => return Ok(false), - }; - substr.py_starts_ends_with( - &affix, - "endswith", - "bytes", - |s, x: PyBytesInner| s.ends_with(x.as_bytes()), - vm, - ) + self.starts_ends_with(options, true, vm) } #[pymethod] @@ -576,19 +692,7 @@ impl Py { options: anystr::StartsEndsWithArgs, vm: &VirtualMachine, ) -> PyResult { - let borrowed = self.borrow_buf(); - let (affix, substr) = - match options.prepare(&*borrowed, borrowed.len(), |s, r| s.get_bytes(r)) { - Some(x) => x, - None => return Ok(false), - }; - substr.py_starts_ends_with( - &affix, - "startswith", - "bytes", - |s, x: PyBytesInner| s.starts_with(x.as_bytes()), - vm, - ) + self.starts_ends_with(options, false, vm) } /// Converting the needle runs Python, which must not resize this @@ -602,9 +706,11 @@ impl Py { where F: Fn(&[u8], &[u8]) -> Option, { - self.exports.fetch_add(1, Ordering::Release); - scopeguard::defer! { self.exports.fetch_sub(1, Ordering::Release); } - self.inner().find(options, find, vm) + self.search_exported( + options, + |bytes, needle, range| bytes.py_find(needle, range, find), + vm, + ) } #[pymethod] @@ -637,7 +743,28 @@ impl Py { options: ByteInnerTranslateOptions, vm: &VirtualMachine, ) -> PyResult { - Ok(self.inner().translate(options, vm)?.into()) + let mut buffers = ByteArrayBuffers::default(); + if let Some(table) = &options.table { + let buffer = PyBuffer::from_object(vm, table, BufferFlags::SIMPLE)?; + if buffer.desc.len != 256 { + return Err(vm.new_value_error("translation table must be 256 characters long")); + } + buffers.first = Some(buffer); + } + buffers.second = Some(PyBuffer::from_object( + vm, + &options.delete, + BufferFlags::SIMPLE, + )?); + // Both acquisition callbacks finish before any input bytes are read. + let table = match &buffers.first { + Some(buffer) => PyByteArray::buffer_snapshot(buffer, vm)?, + None => (0..=u8::MAX).collect(), + }; + let delete = PyByteArray::buffer_snapshot(buffers.second.as_ref().unwrap(), vm)?; + let result = self.inner().translate_with(&table, &delete).into(); + drop(buffers); + Ok(result) } #[pymethod] @@ -820,10 +947,33 @@ impl Comparable for PyByteArray { op: PyComparisonOp, vm: &VirtualMachine, ) -> PyResult { - if let Some(res) = op.identical_optimization(zelf, other) { - return Ok(res.into()); + if !zelf.as_object().check_buffer() || !other.check_buffer() { + return Ok(PyComparisonValue::NotImplemented); } - Ok(zelf.inner().cmp(other, op, vm)) + let first = match PyBuffer::from_object(vm, zelf.as_object(), BufferFlags::SIMPLE) { + Ok(buffer) => buffer, + Err(_) => return Ok(PyComparisonValue::NotImplemented), + }; + let mut buffers = ByteArrayBuffers { + first: Some(first), + second: None, + }; + buffers.second = match PyBuffer::from_object(vm, other, BufferFlags::SIMPLE) { + Ok(buffer) => Some(buffer), + Err(_) => return Ok(PyComparisonValue::NotImplemented), + }; + let left = buffers.first.as_ref().unwrap(); + let right = buffers.second.as_ref().unwrap(); + let result = if left.desc.len != right.desc.len && op.eval_eq(|| false).is_some() { + op.eval_eq(|| false).unwrap() + } else { + let left = Self::buffer_snapshot(left, vm)?; + let right = Self::buffer_snapshot(right, vm)?; + op.eval_eq(|| left == right) + .unwrap_or_else(|| op.eval_ord(left.cmp(&right))) + }; + drop(buffers); + Ok(result.into()) } } diff --git a/crates/vm/src/bytes_inner.rs b/crates/vm/src/bytes_inner.rs index 48f9bd1d4..8c52948c8 100644 --- a/crates/vm/src/bytes_inner.rs +++ b/crates/vm/src/bytes_inner.rs @@ -315,7 +315,7 @@ impl ByteInnerSub { } } - fn into_vec(self, vm: &VirtualMachine) -> PyResult> { + pub(crate) fn into_vec(self, vm: &VirtualMachine) -> PyResult> { Ok(match self { Self::Buffer(buffer) => buffer.elements, Self::Byte(int) => vec![int.as_bigint().byte_or(vm)?], @@ -334,6 +334,15 @@ pub struct ByteInnerFindOptions { } impl ByteInnerFindOptions { + pub(crate) fn parts(&self, len: usize) -> (&PyObject, core::ops::Range) { + let range = anystr::adjust_indices( + self.start.as_ref().map(AsRef::as_ref), + self.end.as_ref().map(AsRef::as_ref), + len, + ); + (&self.sub, range) + } + pub fn get_value( self, len: usize, @@ -360,7 +369,7 @@ pub struct ByteInnerPaddingOptions { } impl ByteInnerPaddingOptions { - fn get_value(self, fn_name: &str, vm: &VirtualMachine) -> PyResult<(isize, u8)> { + pub(crate) fn get_value(self, fn_name: &str, vm: &VirtualMachine) -> PyResult<(isize, u8)> { let fillchar = { let v = self.fillchar; try_as_bytes(v.clone(), |bytes| bytes.iter().copied().exactly_one().ok()) @@ -381,9 +390,9 @@ impl ByteInnerPaddingOptions { #[derive(FromArgs)] pub struct ByteInnerTranslateOptions { #[pyarg(positional)] - table: Option, + pub(crate) table: Option, #[pyarg(any, default = b"")] - delete: PyObjectRef, + pub(crate) delete: PyObjectRef, } impl ByteInnerTranslateOptions { @@ -721,6 +730,16 @@ impl PyBytesInner { vm: &VirtualMachine, ) -> PyResult> { let (width, fillchar) = options.get_value("center", vm)?; + self.pad_with(width, fillchar, pad, vm) + } + + pub(crate) fn pad_with( + &self, + width: isize, + fillchar: u8, + pad: PadFn, + vm: &VirtualMachine, + ) -> PyResult> { let len = self.len(); if len as isize >= width { return Ok(Vec::from(&self.elements[..])); @@ -796,7 +815,10 @@ impl PyBytesInner { vm: &VirtualMachine, ) -> PyResult> { let (table, delete) = options.get_value(vm)?; + Ok(self.translate_with(&table, &delete)) + } + pub(crate) fn translate_with(&self, table: &[u8], delete: &[u8]) -> Vec { let mut res = if delete.is_empty() { Vec::with_capacity(self.elements.len()) } else { @@ -809,7 +831,7 @@ impl PyBytesInner { } } - Ok(res) + res } pub fn strip(&self, chars: Option) -> Vec { diff --git a/extra_tests/snippets/builtin_bytearray_callback_exports.py b/extra_tests/snippets/builtin_bytearray_callback_exports.py new file mode 100644 index 000000000..1e80928dc --- /dev/null +++ b/extra_tests/snippets/builtin_bytearray_callback_exports.py @@ -0,0 +1,237 @@ +import operator + +from testutils import assert_raises + + +def check_comparison(): + for compare, expected in ( + (operator.eq, False), + (operator.ne, True), + (operator.lt, True), + (operator.le, True), + (operator.gt, False), + (operator.ge, False), + ): + target = bytearray(b"a") + events = [] + + class Other: + def __buffer__(self, flags): + events.append("acquire") + with assert_raises(BufferError): + target.__init__(object()) + target[0] = ord("A") + return memoryview(b"B") + + def __release_buffer__(self, view): + events.append("release") + target[0] = ord("Z") + target.append(99) + + assert compare(target, Other()) is expected + assert target == b"Zc" + assert events == ["acquire", "release"] + + events = [] + + class Left(bytearray): + def __buffer__(self, flags): + events.append("acquire") + return memoryview(b"L") + + def __release_buffer__(self, view): + events.append("release") + + left = Left(b"ignored") + assert left.__eq__(left) is True + assert events == ["acquire", "acquire", "release", "release"] + events.clear() + assert left.__eq__(object()) is NotImplemented + assert events == [] + + class Failing: + def __buffer__(self, flags): + raise ValueError("no buffer") + + assert left.__eq__(Failing()) is NotImplemented + assert events == ["acquire", "release"] + left.append(0) + + +def check_search(): + for method, expected in ( + ("find", 0), + ("index", 0), + ("rfind", 2), + ("rindex", 2), + ("count", 2), + ): + target = bytearray(b"xxx") + events = [] + + class Needle: + def __buffer__(self, flags): + events.append("acquire") + with assert_raises(BufferError): + target.__init__(object()) + target[:] = b"aba" + return memoryview(b"a") + + def __release_buffer__(self, view): + events.append("release") + with assert_raises(BufferError): + target.append(0) + target[:] = b"zzz" + + assert getattr(target, method)(Needle()) == expected + assert target == b"zzz" + assert events == ["acquire", "release"] + target.append(0) + + target = bytearray(b"xxx") + + class IndexNeedle: + def __index__(self): + with assert_raises(BufferError): + target.__init__(object()) + target[:] = b"aba" + return ord("a") + + assert getattr(target, method)(IndexNeedle()) == expected + + target = bytearray(b"ab") + + class Start: + def __index__(self): + target.extend(b"ab") + return 1 + + assert target.count(ord("a"), Start()) == 1 + assert target == b"abab" + + +def check_affixes(): + for method in ("startswith", "endswith"): + target = bytearray(b"aaa") + events = [] + + class Affix: + def __init__(self, value, after): + self.value = value + self.after = after + + def __buffer__(self, flags): + events.append(self.value) + with assert_raises(BufferError): + target.__init__(object()) + target[0] = target[0] + return memoryview(self.value) + + def __release_buffer__(self, view): + events.append("release") + with assert_raises(BufferError): + target.append(0) + target[:] = self.after + + class Unvisited: + def __buffer__(self, flags): + raise AssertionError("tuple must short-circuit") + + assert getattr(target, method)( + (Affix(b"x", b"yyy"), Affix(b"y", b"zzz"), Unvisited()) + ) + assert target == b"zzz" + assert events == [b"x", "release", b"y", "release"] + target.append(0) + + target = bytearray(b"aaa") + events.clear() + assert getattr(target, method)(Affix(b"a", b"zzz")) + assert target == b"zzz" + assert events == [b"a", "release"] + + events.clear() + assert not getattr(target, method)(Affix(b"z", b"aaa"), 100) + assert events == [b"z", "release"] + with assert_raises(TypeError): + getattr(target, method)((Affix(b"x", b"aaa"), object())) + target.append(0) + + class BytesAffix(bytes): + def __buffer__(self, flags): + raise AssertionError("bytes subclasses use their stored bytes") + + assert getattr(bytearray(b"a"), method)(BytesAffix(b"a")) + + +def check_translate(): + target = bytearray(b"x") + table = bytearray(range(256)) + events = [] + + class Table: + def __buffer__(self, flags): + events.append("table acquire") + target.__init__(b"a") + return memoryview(table) + + def __release_buffer__(self, view): + events.append("table release") + target.__init__(b"t") + + class Delete: + def __buffer__(self, flags): + events.append("delete acquire") + with assert_raises(BufferError): + table.append(0) + table[ord("a")] = ord("b") + target.__init__(b"aa") + return memoryview(b"") + + def __release_buffer__(self, view): + events.append("delete release") + target.__init__(b"d") + + assert target.translate(Table(), delete=Delete()) == b"bb" + assert target == b"d" + assert events == [ + "table acquire", + "delete acquire", + "table release", + "delete release", + ] + table.append(0) + table.pop() + events.clear() + + class BadDelete: + def __buffer__(self, flags): + events.append("delete failure") + with assert_raises(BufferError): + table.append(0) + raise ValueError("bad delete") + + with assert_raises(ValueError): + target.translate(Table(), delete=BadDelete()) + assert events == ["table acquire", "delete failure", "table release"] + table.append(0) + + +def check_padding(): + for method in ("center", "ljust", "rjust"): + target = bytearray(b"x") + assert getattr(target, method)(3, target) == b"xxx" + + class Width: + def __index__(self): + target.__init__(b"y") + return 3 + + assert getattr(target, method)(Width(), target) == b"yyy" + + +check_comparison() +check_search() +check_affixes() +check_translate() +check_padding() From 896e07d9fa95dc0283ef185c61ab67595a004e65 Mon Sep 17 00:00:00 2001 From: youknowdot Date: Sun, 11 Oct 2026 21:36:37 -0700 Subject: [PATCH 4/7] Implement mkfifo with directory descriptor support (#9056) Support directory-relative FIFO creation. Retry interrupted path opens while releasing the interpreter and honoring signal-handler exceptions; preserve raw directory descriptors across callbacks. Assisted-by: OpenAI Codex:model identifier unavailable --- crates/host_env/src/crt_fd.rs | 8 +- crates/host_env/src/posix.rs | 61 +++++++++++++++ crates/vm/src/stdlib/_io.rs | 19 +++-- crates/vm/src/stdlib/os.rs | 118 +++++++++++++++++++++------- crates/vm/src/stdlib/posix.rs | 92 ++++++++++++++++++++++ extra_tests/snippets/stdlib_os.py | 125 ++++++++++++++++++++++++++++++ 6 files changed, 385 insertions(+), 38 deletions(-) diff --git a/crates/host_env/src/crt_fd.rs b/crates/host_env/src/crt_fd.rs index cdedf177b..c9af64612 100644 --- a/crates/host_env/src/crt_fd.rs +++ b/crates/host_env/src/crt_fd.rs @@ -331,7 +331,13 @@ pub fn wopen(path: &widestring::WideCStr, flags: i32, mode: i32) -> io::Result, path: &ffi::CStr, flags: i32, mode: i32) -> io::Result { - cvt_fd(unsafe { c::openat(dir.as_raw(), path.as_ptr(), flags, mode) }) + openat_raw(dir.as_raw(), path, flags, mode) +} + +#[cfg(all(any(unix, target_os = "wasi"), not(target_os = "redox")))] +// Accept a descriptor number without asserting it stays open across calls. +pub fn openat_raw(dir: Raw, path: &ffi::CStr, flags: i32, mode: i32) -> io::Result { + cvt_fd(unsafe { c::openat(dir, path.as_ptr(), flags, mode) }) } pub fn fsync(fd: Borrowed<'_>) -> io::Result<()> { diff --git a/crates/host_env/src/posix.rs b/crates/host_env/src/posix.rs index ae7c0d3de..053148ca4 100644 --- a/crates/host_env/src/posix.rs +++ b/crates/host_env/src/posix.rs @@ -569,6 +569,67 @@ pub fn statvfs_fd(fd: i32) -> std::io::Result { } } +#[cfg(not(target_os = "redox"))] +pub fn mkfifo(path: &CStr, mode: libc::mode_t) -> std::io::Result<()> { + let ret = unsafe { libc::mkfifo(path.as_ptr(), mode) }; + if ret == 0 { + Ok(()) + } else { + Err(std::io::Error::last_os_error()) + } +} + +#[cfg(target_vendor = "apple")] +type MkfifoAt = unsafe extern "C" fn(libc::c_int, *const libc::c_char, libc::mode_t) -> libc::c_int; + +#[cfg(target_vendor = "apple")] +fn mkfifoat_symbol() -> Option { + static MKFIFOAT: std::sync::OnceLock> = std::sync::OnceLock::new(); + *MKFIFOAT.get_or_init(|| { + // mkfifoat was added in macOS 13 / iOS 16. Resolve it at runtime so + // ordinary mkfifo remains usable on older Apple systems. + let symbol = unsafe { libc::dlsym(libc::RTLD_DEFAULT, c"mkfifoat".as_ptr()) }; + if symbol.is_null() { + None + } else { + // SAFETY: the system symbol has the MkfifoAt signature and stays + // loaded for the lifetime of the process. + Some(unsafe { core::mem::transmute::<*mut libc::c_void, MkfifoAt>(symbol) }) + } + }) +} + +#[cfg(not(target_os = "redox"))] +pub fn has_mkfifoat() -> bool { + #[cfg(target_vendor = "apple")] + { + mkfifoat_symbol().is_some() + } + #[cfg(not(target_vendor = "apple"))] + { + !cfg!(target_os = "android") + } +} + +#[cfg(not(any(target_os = "redox", target_os = "android")))] +pub fn mkfifoat(dir_fd: i32, path: &CStr, mode: libc::mode_t) -> std::io::Result<()> { + #[cfg(target_vendor = "apple")] + let mkfifoat = mkfifoat_symbol().ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::Unsupported, + "mkfifoat unavailable on this platform", + ) + })?; + #[cfg(not(target_vendor = "apple"))] + let mkfifoat = libc::mkfifoat; + let ret = unsafe { mkfifoat(dir_fd, path.as_ptr(), mode) }; + if ret == 0 { + Ok(()) + } else { + Err(std::io::Error::last_os_error()) + } +} + #[cfg(not(target_os = "redox"))] pub fn mknod(path: &CStr, mode: libc::mode_t, device: libc::dev_t) -> std::io::Result<()> { let ret = unsafe { libc::mknod(path.as_ptr(), mode, device) }; diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index 1d86717a2..5bcd8f64d 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -5930,7 +5930,7 @@ mod _io { #[pymodule] mod fileio { use super::{_io::*, Offset, iobase_finalize}; - use crate::host_env::crt_fd; + use crate::host_env::{crt_fd, os::ErrorExt}; use crate::{ AsObject, Py, PyObject, PyObjectRef, PyPayload, PyResult, TryFromObject, VirtualMachine, builtins::{PyBaseExceptionRef, PyUtf8Str, PyUtf8StrRef}, @@ -6044,14 +6044,19 @@ mod fileio { } else { let path = OsPath::try_from_fspath(name.clone(), vm)?; #[cfg(any(unix, target_os = "wasi"))] - let fd = host_io::open_path(&path.clone().into_cstring(vm)?, flags, 0o666); + let raw_path = path.clone().into_cstring(vm)?; #[cfg(windows)] - let fd = host_io::open_path(&path.to_wide_cstring(vm)?, flags, 0o666); + let raw_path = path.to_wide_cstring(vm)?; let filename = OsPathOrFd::Path(path); - match fd { - Ok(fd) => (fd.into_raw(), Some(filename)), - Err(e) => { - return Err(OSErrorBuilder::with_filename_from_errno(&e, filename, vm)); + loop { + match vm.allow_threads(|| host_io::open_path(&raw_path, flags, 0o666)) { + Ok(fd) => break (fd.into_raw(), Some(filename)), + Err(e) if e.posix_errno() == libc::EINTR => vm.check_signals()?, + Err(e) => { + return Err(OSErrorBuilder::with_filename_from_errno( + &e, filename, vm, + )); + } } } } diff --git a/crates/vm/src/stdlib/os.rs b/crates/vm/src/stdlib/os.rs index b873cb4c2..7419c8f92 100644 --- a/crates/vm/src/stdlib/os.rs +++ b/crates/vm/src/stdlib/os.rs @@ -199,7 +199,10 @@ impl ToPyObject for crt_fd::Borrowed<'_> { #[pymodule(sub, name = "posix")] pub(super) mod _os { - use super::{DirFd, DstDirFd, FollowSymlinks, RawMode, SrcDirFd, SupportFunc}; + use super::{ + AT_FDCWD, DefaultDirFd, DirFd, DirFdKeyword, DstDirFd, FollowSymlinks, RawMode, SrcDirFd, + SupportFunc, warn_if_bool_fd, + }; #[cfg(not(windows))] use crate::exceptions; use crate::host_env::fileutils::StatStruct; @@ -212,9 +215,11 @@ pub(super) mod _os { }, class::PyClassDef, common::lock::{OnceCell, PyRwLock}, - convert::{IntoPyException, ToPyObject}, + convert::{IntoPyException, ToPyException, ToPyObject}, exceptions::{OSErrorBuilder, ToOSErrorBuilder}, - function::{ArgBytesLike, ArgMemoryBuffer, FsPath, FuncArgs}, + function::{ + ArgBytesLike, ArgMemoryBuffer, ArgumentError, FromArgs, FsPath, FuncArgs, Param, + }, host_env::crt_fd, ospath::{OsPath, OsPathOrFd, OutputMode, PathConverter}, protocol::PyIterReturn, @@ -276,20 +281,57 @@ pub(super) mod _os { crt_fd::closerange(fd_low, fd_high); } + // Signal callbacks can close this descriptor between open attempts. + #[cfg(any(unix, windows, target_os = "wasi"))] + struct OpenDirFd(crt_fd::Raw); + + #[cfg(any(unix, windows, target_os = "wasi"))] + impl FromArgs for OpenDirFd { + const PARAMS: Option<&'static [Param]> = Some(DefaultDirFd::PARAMS); + + fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result { + let fd = match args.take_keyword("dir_fd") { + Some(o) if vm.is_none(&o) => AT_FDCWD, + None => AT_FDCWD, + Some(o) => { + warn_if_bool_fd(&o, vm).map_err(Into::::into)?; + let fd = o.try_index_opt(vm).unwrap_or_else(|| { + Err(vm.new_type_error(format!( + "argument should be integer or None, not {}", + o.class().name() + ))) + })?; + fd.try_to_primitive(vm)? + } + }; + if fd == -1 { + return Err(io::Error::from_raw_os_error(libc::EBADF) + .to_pyexception(vm) + .into()); + } + if !OPEN_DIR_FD && fd != AT_FDCWD { + return Err(vm + .new_not_implemented_error("dir_fd unavailable on this platform") + .into()); + } + Ok(Self(fd)) + } + } + #[cfg(any(unix, windows, target_os = "wasi"))] #[derive(FromArgs)] - struct OpenArgs<'fd> { + struct OpenArgs { path: OsPath, flags: i32, #[pyarg(any, default = 0o777)] mode: i32, #[pyarg(flatten)] - dir_fd: DirFd<'fd, { OPEN_DIR_FD as usize }>, + dir_fd: OpenDirFd, } #[pyfunction] - fn open(args: OpenArgs<'_>, vm: &VirtualMachine) -> PyResult { - os_open(args.path, args.flags, args.mode, args.dir_fd, vm) + fn open(args: OpenArgs, vm: &VirtualMachine) -> PyResult { + os_open(args.path, args.flags, args.mode, args.dir_fd.0, vm) } #[cfg(any(unix, windows, target_os = "wasi"))] @@ -297,34 +339,50 @@ pub(super) mod _os { name: OsPath, flags: i32, mode: i32, - dir_fd: DirFd<'_, { OPEN_DIR_FD as usize }>, + dir_fd: crt_fd::Raw, vm: &VirtualMachine, ) -> PyResult { + use crate::host_env::os::ErrorExt; + #[cfg(windows)] - let fd = { - let [] = dir_fd.0; - let name = name.to_wide_cstring(vm)?; - let flags = flags | crate::host_env::os::O_NOINHERIT; - crt_fd::wopen(&name, flags, mode) - }; + let raw_name = name.to_wide_cstring(vm)?; #[cfg(not(windows))] - let fd = { - let name = name.clone().into_cstring(vm)?; - #[cfg(not(target_os = "wasi"))] - let flags = flags | crate::host_env::os::O_CLOEXEC; - #[cfg(not(target_os = "redox"))] - if let Some(dir_fd) = dir_fd.get_opt() { - crt_fd::openat(dir_fd, &name, flags, mode) - } else { - crt_fd::open(&name, flags, mode) + let raw_name = name.clone().into_cstring(vm)?; + #[cfg(windows)] + let flags = flags | crate::host_env::os::O_NOINHERIT; + #[cfg(all(not(windows), not(target_os = "wasi")))] + let flags = flags | crate::host_env::os::O_CLOEXEC; + + loop { + let fd = vm.allow_threads(|| { + #[cfg(windows)] + { + let _ = dir_fd; + crt_fd::wopen(&raw_name, flags, mode) + } + #[cfg(not(windows))] + { + #[cfg(not(target_os = "redox"))] + if dir_fd != AT_FDCWD { + crt_fd::openat_raw(dir_fd, &raw_name, flags, mode) + } else { + crt_fd::open(&raw_name, flags, mode) + } + #[cfg(target_os = "redox")] + { + let _ = dir_fd; + crt_fd::open(&raw_name, flags, mode) + } + } + }); + match fd { + Ok(fd) => return Ok(fd), + Err(err) if err.posix_errno() == libc::EINTR => vm.check_signals()?, + Err(err) => { + return Err(OSErrorBuilder::with_filename_from_errno(&err, name, vm)); + } } - #[cfg(target_os = "redox")] - { - let [] = dir_fd.0; - crt_fd::open(&name, flags, mode) - } - }; - fd.map_err(|err| OSErrorBuilder::with_filename_from_errno(&err, name, vm)) + } } #[derive(FromArgs)] diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs index 96bb00667..388771acb 100644 --- a/crates/vm/src/stdlib/posix.rs +++ b/crates/vm/src/stdlib/posix.rs @@ -1012,6 +1012,91 @@ pub mod module { } } + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + struct MkfifoPath(OsPath); + + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + impl TryFromObject for MkfifoPath { + fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + crate::ospath::PathConverter::new() + .function("mkfifo") + .try_path(obj, vm) + .map(Self) + } + } + + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + struct MkfifoMode(i32); + + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + impl TryFromObject for MkfifoMode { + fn try_from_object(vm: &VirtualMachine, obj: PyObjectRef) -> PyResult { + let mode = obj.try_index(vm)?; + i32::try_from(mode.as_bigint()) + .map(Self) + .map_err(|_| vm.new_overflow_error("Python int too large to convert to C int")) + } + } + + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + #[derive(FromArgs)] + struct MkfifoArgs { + #[pyarg(any)] + path: MkfifoPath, + #[pyarg(any, default = MkfifoMode(0o666), py_default = "438")] + mode: MkfifoMode, + #[pyarg(named, optional, py_default = "None")] + dir_fd: OptionalArg, + } + + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + #[pyfunction] + fn mkfifo(args: MkfifoArgs, vm: &VirtualMachine) -> PyResult<()> { + let c_path = args.path.0.into_cstring(vm)?; + let mode = args.mode.0 as libc::mode_t; + // Keep a raw C int: even -1 is valid with an absolute path, for which + // mkfifoat ignores dir_fd. BorrowedFd cannot represent that value. + let dir_fd = match args.dir_fd { + OptionalArg::Present(fd) if !vm.is_none(&fd) => { + warn_if_bool_fd(&fd, vm)?; + let fd = fd.try_index_opt(vm).unwrap_or_else(|| { + Err(vm.new_type_error(format!( + "argument should be integer or None, not {}", + fd.class().name() + ))) + })?; + let fd = i32::try_from(fd.as_bigint()).map_err(|_| { + let message = if fd.as_bigint().sign() == malachite_bigint::Sign::Minus { + "fd is less than minimum" + } else { + "fd is greater than maximum" + }; + vm.new_overflow_error(message) + })?; + (fd != rustpython_host_env::os::AT_FDCWD).then_some(fd) + } + _ => None, + }; + if dir_fd.is_some() && !host_posix::has_mkfifoat() { + return Err(vm.new_not_implemented_error("dir_fd unavailable on this platform")); + } + + loop { + let result = vm.allow_threads(|| { + #[cfg(not(target_os = "android"))] + if let Some(dir_fd) = dir_fd { + return host_posix::mkfifoat(dir_fd, &c_path, mode); + } + host_posix::mkfifo(&c_path, mode) + }); + match result { + Ok(()) => return Ok(()), + Err(err) if err.raw_os_error() == Some(libc::EINTR) => vm.check_signals()?, + Err(err) => return Err(err.into_pyexception(vm)), + } + } + } + #[cfg(not(target_os = "redox"))] const MKNOD_DIR_FD: bool = cfg!(not(target_vendor = "apple")); @@ -2243,6 +2328,13 @@ pub mod module { SupportFunc::new("lchown", None, None, None), #[cfg(not(target_os = "redox"))] SupportFunc::new("fchown", Some(true), None, Some(true)), + #[cfg(not(any(target_os = "redox", target_os = "wasi")))] + SupportFunc::new( + "mkfifo", + Some(false), + Some(host_posix::has_mkfifoat()), + Some(false), + ), #[cfg(not(target_os = "redox"))] SupportFunc::new("mknod", Some(true), Some(MKNOD_DIR_FD), Some(false)), SupportFunc::new("umask", Some(false), Some(false), Some(false)), diff --git a/extra_tests/snippets/stdlib_os.py b/extra_tests/snippets/stdlib_os.py index df7c723c9..50ce16be1 100644 --- a/extra_tests/snippets/stdlib_os.py +++ b/extra_tests/snippets/stdlib_os.py @@ -633,3 +633,128 @@ assert os.stat_result(tuple(range(10))).st_atime == 7 assert os.stat_result(tuple(range(10)), {"st_atime": 1.5}).st_atime == 1.5 with assert_raises(TypeError): os.stat_result(tuple(range(10)), ["st_atime"]) + + +def mkfifo_paths_and_directory_descriptors(): + import errno + import inspect + import tempfile + import warnings + + assert str(inspect.signature(os.mkfifo)) == "(path, mode=438, *, dir_fd=None)" + assert os.mkfifo not in os.supports_fd + assert os.mkfifo not in os.supports_follow_symlinks + if sys.platform.startswith("linux"): + assert os.mkfifo in os.supports_dir_fd + + class Path: + def __init__(self, value): + self.value = value + + def __fspath__(self): + return self.value + + class Index: + def __init__(self, value): + self.value = value + + def __index__(self): + return self.value + + class BadIndex: + def __index__(self): + raise RuntimeError("mkfifo index marker") + + # Never open a FIFO: creating, inspecting and unlinking it cannot wait for + # another process to connect. All paths belong to this temporary directory. + with tempfile.TemporaryDirectory(prefix="rustpython-mkfifo-") as directory: + path = os.path.join(directory, "fifo") + for value in (path, os.fsencode(path), Path(path), Path(os.fsencode(path))): + assert os.mkfifo(path=value, mode=0) is None + info = os.stat(path) + assert stat.S_ISFIFO(info.st_mode) + assert stat.S_IMODE(info.st_mode) == 0 + with assert_raises(FileExistsError) as error: + os.mkfifo(value) + assert error.exception.errno == errno.EEXIST + assert error.exception.filename is None + os.unlink(path) + + assert os.mkfifo(path, dir_fd=None) is None + assert stat.S_ISFIFO(os.stat(path).st_mode) + assert stat.S_IMODE(os.stat(path).st_mode) & ~0o666 == 0 + os.unlink(path) + + for value in (None, 1.5): + assert_raises(TypeError, os.mkfifo, path, value) + for value in (2**31, -(2**31) - 1, 2**100, -(2**100)): + with assert_raises(OverflowError) as error: + os.mkfifo(path, value) + assert str(error.exception) == "Python int too large to convert to C int" + with assert_raises(OverflowError) as error: + os.mkfifo(path, dir_fd=value) + expected = ( + "fd is greater than maximum" if value > 0 else "fd is less than minimum" + ) + assert str(error.exception) == expected + for value in (1.5, "fd"): + assert_raises(TypeError, os.mkfifo, path, dir_fd=value) + for value in (None, 1): + with assert_raises(TypeError) as error: + os.mkfifo(value) + expected = "NoneType" if value is None else "int" + assert str(error.exception) == ( + "mkfifo: path should be string, bytes or os.PathLike, not " + expected + ) + assert_raises(TypeError, os.mkfifo, Path(None)) + for value in (path + "\0", os.fsencode(path) + b"\0"): + with assert_raises(ValueError) as error: + os.mkfifo(value) + assert str(error.exception) == "mkfifo: embedded null character in path" + assert_raises(TypeError, os.mkfifo, path, 0o600, None) + for options in ({"mode": BadIndex()}, {"dir_fd": BadIndex()}): + with assert_raises(RuntimeError) as error: + os.mkfifo(path, **options) + assert str(error.exception) == "mkfifo index marker" + with assert_raises(FileNotFoundError) as error: + os.mkfifo(os.path.join(directory, "missing", "fifo")) + assert error.exception.errno == errno.ENOENT + assert error.exception.filename is None + + directory_fd = os.open(directory, os.O_RDONLY) + try: + if os.mkfifo in os.supports_dir_fd: + assert ( + os.mkfifo(Path(b"fifo"), Index(0o600), dir_fd=Index(directory_fd)) + is None + ) + assert stat.S_ISFIFO(os.stat(path).st_mode) + assert stat.S_IMODE(os.stat(path).st_mode) & ~0o600 == 0 + os.unlink(path) + for value in (-1, -(2**31)): + assert os.mkfifo(path, dir_fd=value) is None + assert stat.S_ISFIFO(os.stat(path).st_mode) + os.unlink(path) + with assert_raises(OSError) as error: + os.mkfifo("fifo", dir_fd=-1) + assert error.exception.errno == errno.EBADF + assert error.exception.filename is None + with warnings.catch_warnings(record=True) as records: + warnings.simplefilter("always") + assert os.mkfifo(path, dir_fd=True) is None + assert len(records) == 1 + assert records[0].category is RuntimeWarning + assert str(records[0].message) == "bool is used as a file descriptor" + os.unlink(path) + else: + with assert_raises(NotImplementedError) as error: + os.mkfifo(path, dir_fd=directory_fd) + assert str(error.exception) == "dir_fd unavailable on this platform" + finally: + os.close(directory_fd) + + +if sys.platform.startswith("linux"): + assert hasattr(os, "mkfifo") +if hasattr(os, "mkfifo"): + mkfifo_paths_and_directory_descriptors() From 9ea138c8891ebf0bdd7865c629d11466d768d2a9 Mon Sep 17 00:00:00 2001 From: youknowdot Date: Sun, 11 Oct 2026 21:38:23 -0700 Subject: [PATCH 5/7] Canonicalize f-string field spacing in annotations (#9077) Ignore incidental formatted-value whitespace while retaining brace-leading safeguards and literal/debug text. Include compile-only annotation-constant coverage. Assisted-by: OpenAI Codex:model identifier unavailable --- crates/codegen/src/unparse.rs | 14 +------------- extra_tests/snippets/syntax_annotations.py | 13 +++++++++++++ 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/crates/codegen/src/unparse.rs b/crates/codegen/src/unparse.rs index 868f1b1ba..7c139e77a 100644 --- a/crates/codegen/src/unparse.rs +++ b/crates/codegen/src/unparse.rs @@ -628,19 +628,7 @@ impl<'a, 'b, 'c> Unparser<'a, 'b, 'c> { // put a space to avoid escaping the bracket "{ " } else { - // Preserve leading whitespace between '{' and the expression - let source_text = self.source.source_text(); - let start = val.range().start().to_usize(); - if start > 0 - && source_text - .as_bytes() - .get(start - 1) - .is_some_and(|b| b.is_ascii_whitespace()) - { - "{ " - } else { - "{" - } + "{" }; self.p(brace)?; self.p(&buffered)?; diff --git a/extra_tests/snippets/syntax_annotations.py b/extra_tests/snippets/syntax_annotations.py index 9b7d0c0ce..b6859a89b 100644 --- a/extra_tests/snippets/syntax_annotations.py +++ b/extra_tests/snippets/syntax_annotations.py @@ -1,5 +1,18 @@ from typing import get_type_hints +# Future f-string annotations canonicalize fields, not their literal padding. +for formatted, expected in ( + ("f'{ value }'", "f'{value}'"), + ("f'{ value = }'", "f' value = {value!r}'"), + ("f'{ value : >8}'", "f'{value: >8}'"), + ("f'{ {1, 2} }'", "f'{ {1, 2}}'"), +): + formatted_code = compile( + "from __future__ import annotations\nx: " + formatted, "", "exec" + ) + assert expected in formatted_code.co_consts + + def func(s: str) -> int: return int(s) From f163e1371018275e40b4651108818d722b02368c Mon Sep 17 00:00:00 2001 From: dot Date: Mon, 12 Oct 2026 14:24:38 +0900 Subject: [PATCH 6/7] Consolidate bytearray regressions in the builtin snippet Preserve all existing regression bodies and coverage while following the requested snippet organization. Assisted-by: OpenAI Codex:model identifier unavailable --- extra_tests/snippets/builtin_bytearray.py | 591 ++++++++++++++++++ .../builtin_bytearray_callback_exports.py | 237 ------- .../builtin_bytearray_concat_exports.py | 79 --- .../snippets/builtin_bytearray_exports.py | 136 ---- .../builtin_bytearray_init_exports.py | 70 --- .../snippets/builtin_bytearray_threading.py | 55 -- 6 files changed, 591 insertions(+), 577 deletions(-) delete mode 100644 extra_tests/snippets/builtin_bytearray_callback_exports.py delete mode 100644 extra_tests/snippets/builtin_bytearray_concat_exports.py delete mode 100644 extra_tests/snippets/builtin_bytearray_exports.py delete mode 100644 extra_tests/snippets/builtin_bytearray_init_exports.py delete mode 100644 extra_tests/snippets/builtin_bytearray_threading.py diff --git a/extra_tests/snippets/builtin_bytearray.py b/extra_tests/snippets/builtin_bytearray.py index d897ec947..2f2793a76 100644 --- a/extra_tests/snippets/builtin_bytearray.py +++ b/extra_tests/snippets/builtin_bytearray.py @@ -913,3 +913,594 @@ for i in range(-1, 2, 1): empty = bytearray() empty *= sys.maxsize assert empty == bytearray() + + +def test_bytearray_exports(): + """Resizing paths must respect outstanding bytearray exports.""" + + import io + + from testutils import assert_raises + + for args in ((), (b"abc",), (b"x",), (b"",), (object(),)): + value = bytearray(b"abc") + with memoryview(value) as view: + with assert_raises(BufferError): + value.__init__(*args) + assert value == b"abc" + assert view.tobytes() == b"abc" + value.__init__(b"released") + assert value == b"released" + + value = bytearray(b"abc") + with assert_raises(TypeError): + value.__init__(object()) + assert value == b"" + value = bytearray(b"abc") + value.__init__(value) + assert value == b"" + + class ExportDuringInit: + def __init__(self, count, payload=b""): + self.count = count + self.payload = payload + self.calls = 0 + + def __index__(self): + self.calls += 1 + value.extend(self.payload) + self.view = memoryview(value) + return self.count + + value = bytearray(b"abc") + source = ExportDuringInit(2) + with assert_raises(BufferError): + value.__init__(source) + assert value == b"" + assert source.view.tobytes() == b"" + assert source.calls == 1 + source.view.release() + value.extend(b"released") + assert value == b"released" + + value = bytearray(b"abc") + source = ExportDuringInit(0) + value.__init__(source) + assert value == b"" + assert source.view.tobytes() == b"" + source.view.release() + + value = bytearray(b"abc") + source = ExportDuringInit(0, b"xy") + value.__init__(source) + assert value == b"xy" + assert source.view.tobytes() == b"xy" + assert source.calls == 1 + source.view.release() + + class FailingIndex: + def __index__(self): + assert value == b"" + raise ValueError("conversion failed") + + value = bytearray(b"abc") + with assert_raises(ValueError): + value.__init__(FailingIndex()) + assert value == b"" + + for key in (0, slice(0, 1)): + value = bytearray(b"abc") + with memoryview(value) as view: + with assert_raises(BufferError): + del value[key] + assert view.tobytes() == b"abc" + del value[key] + assert value == b"bc" + + class Reader(io.RawIOBase): + def __init__(self, count, retain, payload=b"abcd"): + self.count = count + self.retain = retain + self.payload = payload + + def readinto(self, buffer): + buffer[:] = self.payload + self.buffer = buffer + if self.retain: + self.view = memoryview(buffer) + return self.count + + for count in (0, 2, 4): + reader = Reader(count, False) + assert reader.read(4) == b"abcd"[:count] + assert reader.buffer == b"" + + reader = Reader(count, True) + with assert_raises(BufferError): + reader.read(4) + assert reader.view.tobytes() == b"abcd" + reader.view.release() + reader.buffer.clear() + + for count in (-1, 5): + reader = Reader(count, True) + with assert_raises(ValueError): + reader.read(4) + assert reader.view.tobytes() == b"abcd" + reader.view.release() + + reader = Reader(None, True) + assert reader.read(4) is None + assert reader.view.tobytes() == b"abcd" + reader.view.release() + + # The callback can shrink the temporary buffer, so slicing by the original + # requested size without checking the current buffer is not safe. + reader = Reader(2, False, b"xy") + assert reader.read(4) == b"xy" + assert reader.buffer == b"" + + reader = Reader(4, False, b"xy") + result = reader.read(4) + assert len(result) == 4 + assert result[:2] == b"xy" + # The contents of the newly grown region are unspecified by CPython. + assert reader.buffer == b"" + + +test_bytearray_exports() + + +def test_bytearray_threading(): + """Acquiring a bytearray export must be atomic with respect to resizing.""" + + import threading + + def exercise_exports(): + payload = b"x" * 65536 + value = bytearray(payload) + barrier = threading.Barrier(3, timeout=5) + errors = [] + + def worker(operation): + try: + for _ in range(256): + barrier.wait() + operation() + barrier.wait() + except BaseException as exc: + errors.append(exc) + barrier.abort() + + def resize(): + try: + value.clear() + value.extend(payload) + except BufferError: + pass + + def decode(): + result = value.decode("ascii") + assert result in ("", "x" * len(payload)) + + def export(): + with memoryview(value) as view: + size = len(view) + assert size in (0, len(payload)) + # The acquired export keeps its original length and contents even + # if the resizing thread runs between these two reads. + assert view.tobytes() == payload[:size] + assert len(view) == size + + threads = [ + threading.Thread(target=worker, args=(operation,), daemon=True) + for operation in (resize, decode, export) + ] + for thread in threads: + thread.start() + for thread in threads: + thread.join(timeout=10) + assert not any(thread.is_alive() for thread in threads), "worker did not finish" + if errors: + raise errors[0] + + exercise_exports() + + +test_bytearray_threading() + + +def test_bytearray_init_exports(): + """Initializer storage and integer-deletion ordering with buffer exports.""" + + from testutils import assert_raises + + class Zero: + def __init__(self, value): + self.value = value + self.observations = [] + + def __index__(self): + self.observations.append((len(self.value), self.value.__alloc__())) + return 0 + + # Final CPython rejects even an empty exported initializer before conversion. + for kind in ("missing", "integer", "callback"): + value = bytearray() + source = Zero(value) + with memoryview(value) as view: + with assert_raises(BufferError): + if kind == "missing": + value.__init__() + elif kind == "integer": + value.__init__(0) + else: + value.__init__(source) + assert value == b"" + assert view.tobytes() == b"" + assert source.observations == [] + + # Clearing an initializer must release its old storage before callbacks and + # before the successful zero-count early return. + for kind in ("missing", "integer", "callback"): + value = bytearray(b"x" * 64) + source = Zero(value) + assert value.__alloc__() > 0 + if kind == "missing": + value.__init__() + elif kind == "integer": + value.__init__(0) + else: + value.__init__(source) + assert source.observations == [(0, 0)] + assert value == b"" + assert value.__alloc__() == 0 + + # An invalid integer index is rejected before checking whether resizing is + # blocked, and neither error path may alter the exported bytes. + for initial, indices in ((b"abc", (3, -4)), (b"", (0, -1))): + for index in indices: + value = bytearray(initial) + with memoryview(value) as view: + try: + del value[index] + except IndexError as exc: + assert str(exc) == "bytearray index out of range" + else: + raise AssertionError("invalid index was accepted") + assert value == initial + assert view.tobytes() == initial + + for index in (0, -1): + value = bytearray(b"abc") + with memoryview(value) as view: + with assert_raises(BufferError): + del value[index] + assert view.tobytes() == b"abc" + del value[index] + assert value == (b"bc" if index == 0 else b"ab") + + +test_bytearray_init_exports() + + +def test_bytearray_concat_exports(): + import operator + + from testutils import assert_raises + + for concat in (operator.add, operator.concat, bytearray.__add__): + target = bytearray(b"a") + events = [] + + class ReentrantBuffer: + def __buffer__(self, flags): + events.append("acquire") + with assert_raises(BufferError): + target.__init__(object()) + with assert_raises(BufferError): + target.append(120) + target[0] = ord("A") + return memoryview(b"b") + + def __release_buffer__(self, view): + events.append("release") + # The left operand's export must already have been released. + target.append(99) + + assert concat(target, ReentrantBuffer()) == bytearray(b"Ab") + assert events == ["acquire", "release"] + assert target == bytearray(b"Ac") + target.append(100) + + class FailingBuffer: + def __buffer__(self, flags): + raise ValueError("buffer acquisition failed") + + with assert_raises(TypeError) as error: + concat(target, FailingBuffer()) + assert str(error.exception) == "can't concat FailingBuffer to bytearray" + target.append(101) + + events = [] + + class Left(bytearray): + def __buffer__(self, flags): + events.append("left acquire") + return memoryview(b"L") + + def __release_buffer__(self, view): + events.append("left release") + + class Right: + def __buffer__(self, flags): + events.append("right acquire") + return memoryview(b"R") + + def __release_buffer__(self, view): + events.append("right release") + + assert concat(Left(b"ignored"), Right()) == bytearray(b"LR") + assert events == [ + "left acquire", + "right acquire", + "left release", + "right release", + ] + + events = [] + with assert_raises(TypeError) as error: + concat(Left(), FailingBuffer()) + assert str(error.exception) == "can't concat FailingBuffer to Left" + assert events == ["left acquire", "left release"] + + class FailingLeft(bytearray): + def __buffer__(self, flags): + events.append("left acquire") + raise ValueError("left acquisition failed") + + events = [] + with assert_raises(TypeError) as error: + concat(FailingLeft(), Right()) + assert str(error.exception) == "can't concat Right to FailingLeft" + assert events == ["left acquire"] + + +test_bytearray_concat_exports() + + +def test_bytearray_callback_exports(): + import operator + + from testutils import assert_raises + + def check_comparison(): + for compare, expected in ( + (operator.eq, False), + (operator.ne, True), + (operator.lt, True), + (operator.le, True), + (operator.gt, False), + (operator.ge, False), + ): + target = bytearray(b"a") + events = [] + + class Other: + def __buffer__(self, flags): + events.append("acquire") + with assert_raises(BufferError): + target.__init__(object()) + target[0] = ord("A") + return memoryview(b"B") + + def __release_buffer__(self, view): + events.append("release") + target[0] = ord("Z") + target.append(99) + + assert compare(target, Other()) is expected + assert target == b"Zc" + assert events == ["acquire", "release"] + + events = [] + + class Left(bytearray): + def __buffer__(self, flags): + events.append("acquire") + return memoryview(b"L") + + def __release_buffer__(self, view): + events.append("release") + + left = Left(b"ignored") + assert left.__eq__(left) is True + assert events == ["acquire", "acquire", "release", "release"] + events.clear() + assert left.__eq__(object()) is NotImplemented + assert events == [] + + class Failing: + def __buffer__(self, flags): + raise ValueError("no buffer") + + assert left.__eq__(Failing()) is NotImplemented + assert events == ["acquire", "release"] + left.append(0) + + def check_search(): + for method, expected in ( + ("find", 0), + ("index", 0), + ("rfind", 2), + ("rindex", 2), + ("count", 2), + ): + target = bytearray(b"xxx") + events = [] + + class Needle: + def __buffer__(self, flags): + events.append("acquire") + with assert_raises(BufferError): + target.__init__(object()) + target[:] = b"aba" + return memoryview(b"a") + + def __release_buffer__(self, view): + events.append("release") + with assert_raises(BufferError): + target.append(0) + target[:] = b"zzz" + + assert getattr(target, method)(Needle()) == expected + assert target == b"zzz" + assert events == ["acquire", "release"] + target.append(0) + + target = bytearray(b"xxx") + + class IndexNeedle: + def __index__(self): + with assert_raises(BufferError): + target.__init__(object()) + target[:] = b"aba" + return ord("a") + + assert getattr(target, method)(IndexNeedle()) == expected + + target = bytearray(b"ab") + + class Start: + def __index__(self): + target.extend(b"ab") + return 1 + + assert target.count(ord("a"), Start()) == 1 + assert target == b"abab" + + def check_affixes(): + for method in ("startswith", "endswith"): + target = bytearray(b"aaa") + events = [] + + class Affix: + def __init__(self, value, after): + self.value = value + self.after = after + + def __buffer__(self, flags): + events.append(self.value) + with assert_raises(BufferError): + target.__init__(object()) + target[0] = target[0] + return memoryview(self.value) + + def __release_buffer__(self, view): + events.append("release") + with assert_raises(BufferError): + target.append(0) + target[:] = self.after + + class Unvisited: + def __buffer__(self, flags): + raise AssertionError("tuple must short-circuit") + + assert getattr(target, method)( + (Affix(b"x", b"yyy"), Affix(b"y", b"zzz"), Unvisited()) + ) + assert target == b"zzz" + assert events == [b"x", "release", b"y", "release"] + target.append(0) + + target = bytearray(b"aaa") + events.clear() + assert getattr(target, method)(Affix(b"a", b"zzz")) + assert target == b"zzz" + assert events == [b"a", "release"] + + events.clear() + assert not getattr(target, method)(Affix(b"z", b"aaa"), 100) + assert events == [b"z", "release"] + with assert_raises(TypeError): + getattr(target, method)((Affix(b"x", b"aaa"), object())) + target.append(0) + + class BytesAffix(bytes): + def __buffer__(self, flags): + raise AssertionError("bytes subclasses use their stored bytes") + + assert getattr(bytearray(b"a"), method)(BytesAffix(b"a")) + + def check_translate(): + target = bytearray(b"x") + table = bytearray(range(256)) + events = [] + + class Table: + def __buffer__(self, flags): + events.append("table acquire") + target.__init__(b"a") + return memoryview(table) + + def __release_buffer__(self, view): + events.append("table release") + target.__init__(b"t") + + class Delete: + def __buffer__(self, flags): + events.append("delete acquire") + with assert_raises(BufferError): + table.append(0) + table[ord("a")] = ord("b") + target.__init__(b"aa") + return memoryview(b"") + + def __release_buffer__(self, view): + events.append("delete release") + target.__init__(b"d") + + assert target.translate(Table(), delete=Delete()) == b"bb" + assert target == b"d" + assert events == [ + "table acquire", + "delete acquire", + "table release", + "delete release", + ] + table.append(0) + table.pop() + events.clear() + + class BadDelete: + def __buffer__(self, flags): + events.append("delete failure") + with assert_raises(BufferError): + table.append(0) + raise ValueError("bad delete") + + with assert_raises(ValueError): + target.translate(Table(), delete=BadDelete()) + assert events == ["table acquire", "delete failure", "table release"] + table.append(0) + + def check_padding(): + for method in ("center", "ljust", "rjust"): + target = bytearray(b"x") + assert getattr(target, method)(3, target) == b"xxx" + + class Width: + def __index__(self): + target.__init__(b"y") + return 3 + + assert getattr(target, method)(Width(), target) == b"yyy" + + check_comparison() + check_search() + check_affixes() + check_translate() + check_padding() + + +test_bytearray_callback_exports() diff --git a/extra_tests/snippets/builtin_bytearray_callback_exports.py b/extra_tests/snippets/builtin_bytearray_callback_exports.py deleted file mode 100644 index 1e80928dc..000000000 --- a/extra_tests/snippets/builtin_bytearray_callback_exports.py +++ /dev/null @@ -1,237 +0,0 @@ -import operator - -from testutils import assert_raises - - -def check_comparison(): - for compare, expected in ( - (operator.eq, False), - (operator.ne, True), - (operator.lt, True), - (operator.le, True), - (operator.gt, False), - (operator.ge, False), - ): - target = bytearray(b"a") - events = [] - - class Other: - def __buffer__(self, flags): - events.append("acquire") - with assert_raises(BufferError): - target.__init__(object()) - target[0] = ord("A") - return memoryview(b"B") - - def __release_buffer__(self, view): - events.append("release") - target[0] = ord("Z") - target.append(99) - - assert compare(target, Other()) is expected - assert target == b"Zc" - assert events == ["acquire", "release"] - - events = [] - - class Left(bytearray): - def __buffer__(self, flags): - events.append("acquire") - return memoryview(b"L") - - def __release_buffer__(self, view): - events.append("release") - - left = Left(b"ignored") - assert left.__eq__(left) is True - assert events == ["acquire", "acquire", "release", "release"] - events.clear() - assert left.__eq__(object()) is NotImplemented - assert events == [] - - class Failing: - def __buffer__(self, flags): - raise ValueError("no buffer") - - assert left.__eq__(Failing()) is NotImplemented - assert events == ["acquire", "release"] - left.append(0) - - -def check_search(): - for method, expected in ( - ("find", 0), - ("index", 0), - ("rfind", 2), - ("rindex", 2), - ("count", 2), - ): - target = bytearray(b"xxx") - events = [] - - class Needle: - def __buffer__(self, flags): - events.append("acquire") - with assert_raises(BufferError): - target.__init__(object()) - target[:] = b"aba" - return memoryview(b"a") - - def __release_buffer__(self, view): - events.append("release") - with assert_raises(BufferError): - target.append(0) - target[:] = b"zzz" - - assert getattr(target, method)(Needle()) == expected - assert target == b"zzz" - assert events == ["acquire", "release"] - target.append(0) - - target = bytearray(b"xxx") - - class IndexNeedle: - def __index__(self): - with assert_raises(BufferError): - target.__init__(object()) - target[:] = b"aba" - return ord("a") - - assert getattr(target, method)(IndexNeedle()) == expected - - target = bytearray(b"ab") - - class Start: - def __index__(self): - target.extend(b"ab") - return 1 - - assert target.count(ord("a"), Start()) == 1 - assert target == b"abab" - - -def check_affixes(): - for method in ("startswith", "endswith"): - target = bytearray(b"aaa") - events = [] - - class Affix: - def __init__(self, value, after): - self.value = value - self.after = after - - def __buffer__(self, flags): - events.append(self.value) - with assert_raises(BufferError): - target.__init__(object()) - target[0] = target[0] - return memoryview(self.value) - - def __release_buffer__(self, view): - events.append("release") - with assert_raises(BufferError): - target.append(0) - target[:] = self.after - - class Unvisited: - def __buffer__(self, flags): - raise AssertionError("tuple must short-circuit") - - assert getattr(target, method)( - (Affix(b"x", b"yyy"), Affix(b"y", b"zzz"), Unvisited()) - ) - assert target == b"zzz" - assert events == [b"x", "release", b"y", "release"] - target.append(0) - - target = bytearray(b"aaa") - events.clear() - assert getattr(target, method)(Affix(b"a", b"zzz")) - assert target == b"zzz" - assert events == [b"a", "release"] - - events.clear() - assert not getattr(target, method)(Affix(b"z", b"aaa"), 100) - assert events == [b"z", "release"] - with assert_raises(TypeError): - getattr(target, method)((Affix(b"x", b"aaa"), object())) - target.append(0) - - class BytesAffix(bytes): - def __buffer__(self, flags): - raise AssertionError("bytes subclasses use their stored bytes") - - assert getattr(bytearray(b"a"), method)(BytesAffix(b"a")) - - -def check_translate(): - target = bytearray(b"x") - table = bytearray(range(256)) - events = [] - - class Table: - def __buffer__(self, flags): - events.append("table acquire") - target.__init__(b"a") - return memoryview(table) - - def __release_buffer__(self, view): - events.append("table release") - target.__init__(b"t") - - class Delete: - def __buffer__(self, flags): - events.append("delete acquire") - with assert_raises(BufferError): - table.append(0) - table[ord("a")] = ord("b") - target.__init__(b"aa") - return memoryview(b"") - - def __release_buffer__(self, view): - events.append("delete release") - target.__init__(b"d") - - assert target.translate(Table(), delete=Delete()) == b"bb" - assert target == b"d" - assert events == [ - "table acquire", - "delete acquire", - "table release", - "delete release", - ] - table.append(0) - table.pop() - events.clear() - - class BadDelete: - def __buffer__(self, flags): - events.append("delete failure") - with assert_raises(BufferError): - table.append(0) - raise ValueError("bad delete") - - with assert_raises(ValueError): - target.translate(Table(), delete=BadDelete()) - assert events == ["table acquire", "delete failure", "table release"] - table.append(0) - - -def check_padding(): - for method in ("center", "ljust", "rjust"): - target = bytearray(b"x") - assert getattr(target, method)(3, target) == b"xxx" - - class Width: - def __index__(self): - target.__init__(b"y") - return 3 - - assert getattr(target, method)(Width(), target) == b"yyy" - - -check_comparison() -check_search() -check_affixes() -check_translate() -check_padding() diff --git a/extra_tests/snippets/builtin_bytearray_concat_exports.py b/extra_tests/snippets/builtin_bytearray_concat_exports.py deleted file mode 100644 index b40889989..000000000 --- a/extra_tests/snippets/builtin_bytearray_concat_exports.py +++ /dev/null @@ -1,79 +0,0 @@ -import operator - -from testutils import assert_raises - -for concat in (operator.add, operator.concat, bytearray.__add__): - target = bytearray(b"a") - events = [] - - class ReentrantBuffer: - def __buffer__(self, flags): - events.append("acquire") - with assert_raises(BufferError): - target.__init__(object()) - with assert_raises(BufferError): - target.append(120) - target[0] = ord("A") - return memoryview(b"b") - - def __release_buffer__(self, view): - events.append("release") - # The left operand's export must already have been released. - target.append(99) - - assert concat(target, ReentrantBuffer()) == bytearray(b"Ab") - assert events == ["acquire", "release"] - assert target == bytearray(b"Ac") - target.append(100) - - class FailingBuffer: - def __buffer__(self, flags): - raise ValueError("buffer acquisition failed") - - with assert_raises(TypeError) as error: - concat(target, FailingBuffer()) - assert str(error.exception) == "can't concat FailingBuffer to bytearray" - target.append(101) - - events = [] - - class Left(bytearray): - def __buffer__(self, flags): - events.append("left acquire") - return memoryview(b"L") - - def __release_buffer__(self, view): - events.append("left release") - - class Right: - def __buffer__(self, flags): - events.append("right acquire") - return memoryview(b"R") - - def __release_buffer__(self, view): - events.append("right release") - - assert concat(Left(b"ignored"), Right()) == bytearray(b"LR") - assert events == [ - "left acquire", - "right acquire", - "left release", - "right release", - ] - - events = [] - with assert_raises(TypeError) as error: - concat(Left(), FailingBuffer()) - assert str(error.exception) == "can't concat FailingBuffer to Left" - assert events == ["left acquire", "left release"] - - class FailingLeft(bytearray): - def __buffer__(self, flags): - events.append("left acquire") - raise ValueError("left acquisition failed") - - events = [] - with assert_raises(TypeError) as error: - concat(FailingLeft(), Right()) - assert str(error.exception) == "can't concat Right to FailingLeft" - assert events == ["left acquire"] diff --git a/extra_tests/snippets/builtin_bytearray_exports.py b/extra_tests/snippets/builtin_bytearray_exports.py deleted file mode 100644 index 06c102c32..000000000 --- a/extra_tests/snippets/builtin_bytearray_exports.py +++ /dev/null @@ -1,136 +0,0 @@ -"""Resizing paths must respect outstanding bytearray exports.""" - -import io - -from testutils import assert_raises - -for args in ((), (b"abc",), (b"x",), (b"",), (object(),)): - value = bytearray(b"abc") - with memoryview(value) as view: - with assert_raises(BufferError): - value.__init__(*args) - assert value == b"abc" - assert view.tobytes() == b"abc" - value.__init__(b"released") - assert value == b"released" - -value = bytearray(b"abc") -with assert_raises(TypeError): - value.__init__(object()) -assert value == b"" -value = bytearray(b"abc") -value.__init__(value) -assert value == b"" - - -class ExportDuringInit: - def __init__(self, count, payload=b""): - self.count = count - self.payload = payload - self.calls = 0 - - def __index__(self): - self.calls += 1 - value.extend(self.payload) - self.view = memoryview(value) - return self.count - - -value = bytearray(b"abc") -source = ExportDuringInit(2) -with assert_raises(BufferError): - value.__init__(source) -assert value == b"" -assert source.view.tobytes() == b"" -assert source.calls == 1 -source.view.release() -value.extend(b"released") -assert value == b"released" - -value = bytearray(b"abc") -source = ExportDuringInit(0) -value.__init__(source) -assert value == b"" -assert source.view.tobytes() == b"" -source.view.release() - -value = bytearray(b"abc") -source = ExportDuringInit(0, b"xy") -value.__init__(source) -assert value == b"xy" -assert source.view.tobytes() == b"xy" -assert source.calls == 1 -source.view.release() - - -class FailingIndex: - def __index__(self): - assert value == b"" - raise ValueError("conversion failed") - - -value = bytearray(b"abc") -with assert_raises(ValueError): - value.__init__(FailingIndex()) -assert value == b"" - -for key in (0, slice(0, 1)): - value = bytearray(b"abc") - with memoryview(value) as view: - with assert_raises(BufferError): - del value[key] - assert view.tobytes() == b"abc" - del value[key] - assert value == b"bc" - - -class Reader(io.RawIOBase): - def __init__(self, count, retain, payload=b"abcd"): - self.count = count - self.retain = retain - self.payload = payload - - def readinto(self, buffer): - buffer[:] = self.payload - self.buffer = buffer - if self.retain: - self.view = memoryview(buffer) - return self.count - - -for count in (0, 2, 4): - reader = Reader(count, False) - assert reader.read(4) == b"abcd"[:count] - assert reader.buffer == b"" - - reader = Reader(count, True) - with assert_raises(BufferError): - reader.read(4) - assert reader.view.tobytes() == b"abcd" - reader.view.release() - reader.buffer.clear() - -for count in (-1, 5): - reader = Reader(count, True) - with assert_raises(ValueError): - reader.read(4) - assert reader.view.tobytes() == b"abcd" - reader.view.release() - -reader = Reader(None, True) -assert reader.read(4) is None -assert reader.view.tobytes() == b"abcd" -reader.view.release() - -# The callback can shrink the temporary buffer, so slicing by the original -# requested size without checking the current buffer is not safe. -reader = Reader(2, False, b"xy") -assert reader.read(4) == b"xy" -assert reader.buffer == b"" - -reader = Reader(4, False, b"xy") -result = reader.read(4) -assert len(result) == 4 -assert result[:2] == b"xy" -# The contents of the newly grown region are unspecified by CPython. -assert reader.buffer == b"" diff --git a/extra_tests/snippets/builtin_bytearray_init_exports.py b/extra_tests/snippets/builtin_bytearray_init_exports.py deleted file mode 100644 index 7587462c3..000000000 --- a/extra_tests/snippets/builtin_bytearray_init_exports.py +++ /dev/null @@ -1,70 +0,0 @@ -"""Initializer storage and integer-deletion ordering with buffer exports.""" - -from testutils import assert_raises - - -class Zero: - def __init__(self, value): - self.value = value - self.observations = [] - - def __index__(self): - self.observations.append((len(self.value), self.value.__alloc__())) - return 0 - - -# Final CPython rejects even an empty exported initializer before conversion. -for kind in ("missing", "integer", "callback"): - value = bytearray() - source = Zero(value) - with memoryview(value) as view: - with assert_raises(BufferError): - if kind == "missing": - value.__init__() - elif kind == "integer": - value.__init__(0) - else: - value.__init__(source) - assert value == b"" - assert view.tobytes() == b"" - assert source.observations == [] - -# Clearing an initializer must release its old storage before callbacks and -# before the successful zero-count early return. -for kind in ("missing", "integer", "callback"): - value = bytearray(b"x" * 64) - source = Zero(value) - assert value.__alloc__() > 0 - if kind == "missing": - value.__init__() - elif kind == "integer": - value.__init__(0) - else: - value.__init__(source) - assert source.observations == [(0, 0)] - assert value == b"" - assert value.__alloc__() == 0 - -# An invalid integer index is rejected before checking whether resizing is -# blocked, and neither error path may alter the exported bytes. -for initial, indices in ((b"abc", (3, -4)), (b"", (0, -1))): - for index in indices: - value = bytearray(initial) - with memoryview(value) as view: - try: - del value[index] - except IndexError as exc: - assert str(exc) == "bytearray index out of range" - else: - raise AssertionError("invalid index was accepted") - assert value == initial - assert view.tobytes() == initial - -for index in (0, -1): - value = bytearray(b"abc") - with memoryview(value) as view: - with assert_raises(BufferError): - del value[index] - assert view.tobytes() == b"abc" - del value[index] - assert value == (b"bc" if index == 0 else b"ab") diff --git a/extra_tests/snippets/builtin_bytearray_threading.py b/extra_tests/snippets/builtin_bytearray_threading.py deleted file mode 100644 index c28bcb7b3..000000000 --- a/extra_tests/snippets/builtin_bytearray_threading.py +++ /dev/null @@ -1,55 +0,0 @@ -"""Acquiring a bytearray export must be atomic with respect to resizing.""" - -import threading - - -def exercise_exports(): - payload = b"x" * 65536 - value = bytearray(payload) - barrier = threading.Barrier(3, timeout=5) - errors = [] - - def worker(operation): - try: - for _ in range(256): - barrier.wait() - operation() - barrier.wait() - except BaseException as exc: - errors.append(exc) - barrier.abort() - - def resize(): - try: - value.clear() - value.extend(payload) - except BufferError: - pass - - def decode(): - result = value.decode("ascii") - assert result in ("", "x" * len(payload)) - - def export(): - with memoryview(value) as view: - size = len(view) - assert size in (0, len(payload)) - # The acquired export keeps its original length and contents even - # if the resizing thread runs between these two reads. - assert view.tobytes() == payload[:size] - assert len(view) == size - - threads = [ - threading.Thread(target=worker, args=(operation,), daemon=True) - for operation in (resize, decode, export) - ] - for thread in threads: - thread.start() - for thread in threads: - thread.join(timeout=10) - assert not any(thread.is_alive() for thread in threads), "worker did not finish" - if errors: - raise errors[0] - - -exercise_exports() From 108171daa2657a6c88798a719c76aac68b8d983b Mon Sep 17 00:00:00 2001 From: dot Date: Mon, 12 Oct 2026 14:48:48 +0900 Subject: [PATCH 7/7] Finish source-less bytearray initialization after clearing Preserve option validation and initial export rejection, then avoid a redundant resize guard and storage swap when no source was provided. Keep zero-count callback behavior and all existing regressions. Assisted-by: OpenAI Codex:model identifier unavailable --- crates/vm/src/builtins/bytearray.rs | 10 +++-- extra_tests/snippets/builtin_bytearray.py | 49 +++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/crates/vm/src/builtins/bytearray.rs b/crates/vm/src/builtins/bytearray.rs index c463489df..d6d424799 100644 --- a/crates/vm/src/builtins/bytearray.rs +++ b/crates/vm/src/builtins/bytearray.rs @@ -23,7 +23,7 @@ use crate::{ }, }, convert::{ToPyObject, ToPyResult}, - function::{ArgBytesLike, FuncArgs, PyComparisonValue, PySsize}, + function::{ArgBytesLike, FuncArgs, OptionalArg, PyComparisonValue, PySsize}, protocol::{ BufferDescriptor, BufferFlags, BufferMethods, BufferResizeGuard, PyBuffer, PyIterReturn, PyMappingMethods, PyNumberMethods, PySequenceMethods, @@ -924,15 +924,17 @@ impl Initializer for PyByteArray { type Args = ByteInnerNewOptions; fn init(zelf: &Py, options: Self::Args, vm: &VirtualMachine) -> PyResult<()> { + let source_is_missing = matches!(&options.source, OptionalArg::Missing); // Reject exports and clear existing contents before converting source. zelf.try_resizable(vm)?.elements = Vec::new(); // Conversion can call Python, so do not hold the lock across it and // check again for exports acquired by a callback or another thread. let (mut inner, zero_count) = options.get_inner_with_zero_count(bytearray_from_object, vm)?; - // A zero integer count performs no further writes, including when its - // __index__ callback changed this bytearray or acquired an export. - if zero_count { + // Missing sources and zero integer counts perform no further writes. + // Argument validation above still rejects encoding/errors without a + // source; a zero __index__ callback may have changed or exported self. + if source_is_missing || zero_count { return Ok(()); } core::mem::swap(&mut *zelf.try_resizable(vm)?, &mut inner); diff --git a/extra_tests/snippets/builtin_bytearray.py b/extra_tests/snippets/builtin_bytearray.py index 2f2793a76..6b397b12f 100644 --- a/extra_tests/snippets/builtin_bytearray.py +++ b/extra_tests/snippets/builtin_bytearray.py @@ -1504,3 +1504,52 @@ def test_bytearray_callback_exports(): test_bytearray_callback_exports() + + +def test_bytearray_missing_source_init(): + from testutils import assert_raises + + for initial in (b"", b"previous contents"): + value = bytearray(initial) + assert value.__init__() is None + assert value == b"" + assert value.__alloc__() == 0 + + for kwargs, message in ( + ({"encoding": "utf-8"}, "encoding without a string argument"), + ({"errors": "strict"}, "errors without a string argument"), + ( + {"encoding": "utf-8", "errors": "strict"}, + "encoding without a string argument", + ), + ): + value = bytearray(initial) + with assert_raises(TypeError) as caught: + value.__init__(**kwargs) + assert str(caught.exception) == message + assert value == b"" + assert value.__alloc__() == 0 + + for kwargs in ( + {}, + {"encoding": "utf-8"}, + {"errors": "strict"}, + {"encoding": "utf-8", "errors": "strict"}, + ): + value = bytearray(initial) + with memoryview(value) as view: + with assert_raises(BufferError): + value.__init__(**kwargs) + assert value == initial + assert view.tobytes() == initial + assert value.__init__() is None + assert value == b"" + + # Explicit None is a source and must still go through conversion. + value = bytearray(initial) + with assert_raises(TypeError): + value.__init__(None) + assert value == b"" + + +test_bytearray_missing_source_init()