From d0819a2fabf2017c5603e089acc7c3a22020f241 Mon Sep 17 00:00:00 2001 From: dot Date: Mon, 12 Oct 2026 05:33:55 +0900 Subject: [PATCH 1/5] Serialize bytearray exports with resizing Keep descriptor acquisition and export retention under the bytearray read lock, and recheck exports after acquiring a resize lock. Guard initializer and sequence deletion writes, preserve zero-index initialization callbacks, and match RawIOBase.read buffer ownership without bypassing export checks. Add bounded Python export-lifetime and concurrent decode/memoryview regression coverage. Assisted-by: OpenAI Codex:model identifier unavailable --- crates/vm/src/builtins/bytearray.rs | 33 +++-- crates/vm/src/bytes_inner.rs | 19 ++- crates/vm/src/stdlib/_io.rs | 11 +- .../snippets/builtin_bytearray_exports.py | 136 ++++++++++++++++++ .../snippets/builtin_bytearray_threading.py | 55 +++++++ 5 files changed, 236 insertions(+), 18 deletions(-) create mode 100644 extra_tests/snippets/builtin_bytearray_exports.py create mode 100644 extra_tests/snippets/builtin_bytearray_threading.py diff --git a/crates/vm/src/builtins/bytearray.rs b/crates/vm/src/builtins/bytearray.rs index 5afeec62833..4904f96fbce 100644 --- a/crates/vm/src/builtins/bytearray.rs +++ b/crates/vm/src/builtins/bytearray.rs @@ -173,11 +173,6 @@ impl PyByteArray { fn inner(&self) -> PyDetachingRwLockReadGuard<'_, PyBytesInner> { self.inner.read() } - #[inline] - fn inner_mut(&self) -> PyDetachingRwLockWriteGuard<'_, PyBytesInner> { - self.inner.write() - } - fn __len__(&self) -> usize { self.borrow_buf().len() } @@ -781,9 +776,18 @@ impl Initializer for PyByteArray { type Args = ByteInnerNewOptions; fn init(zelf: &Py, options: Self::Args, vm: &VirtualMachine) -> PyResult<()> { - // First unpack bytearray and *then* get a lock to set it. - let mut inner = options.get_inner(bytearray_from_object, vm)?; - core::mem::swap(&mut *zelf.inner_mut(), &mut inner); + // Reject exports and clear existing contents before converting source. + zelf.try_resizable(vm)?.elements.clear(); + // 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 { + return Ok(()); + } + core::mem::swap(&mut *zelf.try_resizable(vm)?, &mut inner); Ok(()) } } @@ -841,9 +845,11 @@ impl AsBuffer for PyByteArray { } fn as_buffer(zelf: &Py, _vm: &VirtualMachine) -> PyResult { + // Keep the length stable until PyBuffer::new retains the export. + let inner = zelf.inner(); Ok(PyBuffer::new( zelf.to_owned().into(), - BufferDescriptor::simple(zelf.__len__(), false), + BufferDescriptor::simple(inner.elements.len(), false), &BUFFER_METHODS, )) } @@ -855,7 +861,12 @@ impl BufferResizeGuard for PyByteArray { fn try_resizable_opt(&self) -> Option> { // 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()) + if self.exports.load(Ordering::SeqCst) != 0 { + return None; + } + let inner = self.inner.write(); + // An export may have been acquired while we waited for the lock. + (self.exports.load(Ordering::SeqCst) == 0).then_some(inner) } } @@ -907,7 +918,7 @@ impl AsSequence for PyByteArray { if let Some(value) = value { zelf._setitem_by_index(i, &value, vm) } else { - zelf.borrow_buf_mut().delitem_by_index(vm, i) + zelf.try_resizable(vm)?.elements.delitem_by_index(vm, i) } }), contains: atomic_func!(|seq, other, vm| { diff --git a/crates/vm/src/bytes_inner.rs b/crates/vm/src/bytes_inner.rs index f577e302085..48f9bd1d445 100644 --- a/crates/vm/src/bytes_inner.rs +++ b/crates/vm/src/bytes_inner.rs @@ -225,12 +225,24 @@ impl ByteInnerNewOptions { /// read: `bytes()` and `bytearray()` differ in whether they ask it how long /// it is. pub fn get_inner(self, from_object: FromObject, vm: &VirtualMachine) -> PyResult { - match (self.source, self.encoding, self.errors) { + self.get_inner_with_zero_count(from_object, vm) + .map(|(inner, _)| inner) + } + + pub(crate) fn get_inner_with_zero_count( + self, + from_object: FromObject, + vm: &VirtualMachine, + ) -> PyResult<(PyBytesInner, bool)> { + let mut zero_count = false; + let inner = match (self.source, self.encoding, self.errors) { (OptionalArg::Present(obj), OptionalArg::Missing, OptionalArg::Missing) => { // Try __index__ first to handle int-like objects that might raise custom exceptions if let Some(index_result) = obj.try_index_opt(vm) { match index_result { - Ok(index) => Self::get_value_from_size(&index, vm), + Ok(index) => Self::get_value_from_size(&index, vm).inspect(|inner| { + zero_count = inner.elements.is_empty(); + }), Err(e) => { // Only propagate non-TypeError exceptions // TypeError means the object doesn't support __index__, so fall back @@ -266,7 +278,8 @@ impl ByteInnerNewOptions { (OptionalArg::Present(_), OptionalArg::Missing, OptionalArg::Present(_)) => { Err(vm.new_type_error(STRING_WITHOUT_ENCODING.to_owned())) } - } + }?; + Ok((inner, zero_count)) } } diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index 1d86717a211..86b68322aa3 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -762,10 +762,13 @@ mod _io { ))); } let n = n as usize; - let mut bytes = b.borrow_buf_mut(); - bytes.truncate(n); - // FIXME: try to use Arc::unwrap on the bytearray to get at the inner buffer - bytes.clone().to_pyobject(vm) + let mut inner = b.try_resizable(vm)?; + let bytes = &mut inner.elements; + bytes + .try_reserve_exact(n.saturating_sub(bytes.len())) + .map_err(|_| vm.no_memory_error())?; + bytes.resize(n, 0); + core::mem::take(bytes).to_pyobject(vm) } }) } else { diff --git a/extra_tests/snippets/builtin_bytearray_exports.py b/extra_tests/snippets/builtin_bytearray_exports.py new file mode 100644 index 00000000000..06c102c320d --- /dev/null +++ b/extra_tests/snippets/builtin_bytearray_exports.py @@ -0,0 +1,136 @@ +"""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_threading.py b/extra_tests/snippets/builtin_bytearray_threading.py new file mode 100644 index 00000000000..c28bcb7b34e --- /dev/null +++ b/extra_tests/snippets/builtin_bytearray_threading.py @@ -0,0 +1,55 @@ +"""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 fcd4f4c8ae18b220168147b8d2c3fd5946636255 Mon Sep 17 00:00:00 2001 From: dot Date: Mon, 12 Oct 2026 10:39:35 +0900 Subject: [PATCH 2/5] 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 4904f96fbce..3186df3c013 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 00000000000..7587462c317 --- /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 3/5] 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 3186df3c013..81939961266 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 00000000000..b40889989c3 --- /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 4/5] 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 b9ce612500b..1b7b20641e1 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 81939961266..c463489df61 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 48f9bd1d445..8c52948c831 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 00000000000..1e80928dcf1 --- /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 f163e1371018275e40b4651108818d722b02368c Mon Sep 17 00:00:00 2001 From: dot Date: Mon, 12 Oct 2026 14:24:38 +0900 Subject: [PATCH 5/5] 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 d897ec94782..2f2793a76cf 100644 --- a/extra_tests/snippets/builtin_bytearray.py +++ b/extra_tests/snippets/builtin_bytearray.py @@ -913,3 +913,594 @@ class B(bytearray): 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 1e80928dcf1..00000000000 --- 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 b40889989c3..00000000000 --- 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 06c102c320d..00000000000 --- 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 7587462c317..00000000000 --- 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 c28bcb7b34e..00000000000 --- 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()