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")