gh-160303: Keep ob_start instead of allocating in del b[:n]

Move ob_start and the size before the shrinking resize, so a failed
shrink leaves the bytes removed and the larger block in place, as the
non-front path already does.  Split the checks out of
bytearray_resize_lock_held() so the slice code can resize directly.
Move the tests into test_resize_error.

Author: gpshead

Lib/test/test_bytes.py

@@ -1663,6 +1663,7 @@ def test_resize_error(self):
         # the bytearray must be left unchanged.
 
         offset = 3
+        front = slice(None, -2)
         for logical_offset in (False, True):
             with self.subTest(logical_offset=logical_offset):
                 # grow bytearray
@@ -1687,25 +1688,28 @@ def test_resize_error(self):
                     ba.resize(1)
                 self.assertEqual(ba, expected)
 
-    @support.nomemtest
-    def test_remove_prefix_error(self):
-        # If the storage cannot be shrunk after removing a prefix
-        # (MemoryError), the bytearray must be left unchanged.
-        for logical_offset in (False, True):
-            with self.subTest(logical_offset=logical_offset):
-                ba = bytearray(b'0123456789' * 4)
+                # gh-160303: When bytes are removed at the front, they are
+                # removed even if shrinking the memory block fails.
+                ba = bytearray(b'0123456789')
                 if logical_offset:
-                    del ba[:3]
-                expected = ba.copy()
+                    del ba[:offset]
                 with inject_memory_error(self):
-                    del ba[:30]
-                self.assertEqual(ba, expected)
+                    del ba[front]
+                self.assertEqual(ba, b'89')
+                ba.append(ord('a'))
+                del ba[:1]
+                self.assertEqual(ba, b'9a')
+
+                ba = bytearray(b'0123456789')
+                if logical_offset:
+                    del ba[:offset]
+                replacement = bytearray(b'xy')
                 with inject_memory_error(self):
-                    ba[:30] = b'xy'
-                self.assertEqual(ba, expected)
-                # The bytearray is still usable
-                del ba[:30]
-                self.assertEqual(ba, expected[30:])
+                    ba[front] = replacement
+                self.assertEqual(ba, b'xy89')
+                ba.append(ord('a'))
+                del ba[:1]
+                self.assertEqual(ba, b'y89a')
 
     def test_take_bytes(self):
         ba = bytearray(b'ab')

Misc/NEWS.d/next/Core_and_Builtins/2026-10-22-15-40-00.gh-issue-160303.q7Zb2K.rst

@@ -1,3 +1,4 @@
 Fix :class:`bytearray` reading and writing memory outside of its buffer
 after ``del b[:n]`` or ``b[:n] = shorter`` failed with :exc:`MemoryError`.
-The bytearray is now left unchanged.
+The bytes are now removed even if shrinking the memory block fails;
+:exc:`MemoryError` is still raised.

Objects/bytearrayobject.c

@@ -238,69 +238,55 @@ bytearray_resize_storage(PyByteArrayObject *self,
 
     Py_ssize_t size = Py_SIZE(self);
 
-    if (self->ob_start != self->ob_bytes && new_size < size) {
-        /* Shrink with a logical offset: copy the bytes to keep into a new
-           allocation. Allocate first, so that the bytearray (including
-           ob_start) is left unchanged on error: bytearray_setslice_linear()
-           relies on it to restore the bytearray after a failed "del b[:n]".
+    /* Re-align data to the start of the allocation. */
+    char *old_start = self->ob_start;
+    if (self->ob_start != self->ob_bytes) {
+        /* optimization tradeoff: This is faster than a new allocation when
+           the number of bytes being removed in a resize is small; for
+           large size changes it may be better to just make a new bytes
+           object as _PyBytes_Resize will do a malloc + memcpy internally.
            */
-        PyObject *bytes = PyBytes_FromStringAndSize(NULL, alloc);
-        if (bytes == NULL) {
-            return -1;
-        }
-        memcpy(PyBytes_AS_STRING(bytes), self->ob_start, new_size);
-        Py_SETREF(self->ob_bytes_object, bytes);
+        Py_ssize_t move = Py_MIN(new_size, size);
+        memmove(self->ob_bytes, self->ob_start, move);
+        self->ob_start = self->ob_bytes;
     }
-    else {
-        /* Re-align data to the start of the allocation. */
-        if (self->ob_start != self->ob_bytes) {
-            /* optimization tradeoff: This is faster than a new allocation
-               when the number of bytes being added in a resize is small. */
-            memmove(self->ob_bytes, self->ob_start, size);
-            self->ob_start = self->ob_bytes;
-        }
 
-        if (_PyBytes_ResizeKeepOnError(&self->ob_bytes_object, alloc) < 0) {
-            bytearray_write_trailing_null_byte(self);
-            return -1;
+    if (_PyBytes_ResizeKeepOnError(&self->ob_bytes_object, alloc) < 0) {
+        if (old_start != self->ob_bytes && new_size < size) {
+            // Move remaining bytes
+            Py_ssize_t moved = new_size;
+            Py_ssize_t remaining = size - moved;
+            memmove(self->ob_bytes + moved, old_start + moved, remaining);
         }
+        bytearray_write_trailing_null_byte(self);
+        return -1;
     }
     assert(_PyBytes_IsMutable(self->ob_bytes_object));
     return 0;
 }
 
 
+/* Resize the storage for requested_size bytes, even if it is already the
+   size of the bytearray.  The caller must have checked that the bytearray
+   can be resized. */
 static int
-bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size)
+bytearray_resize_unchecked(PyByteArrayObject *obj, Py_ssize_t requested_size)
 {
-    _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self);
-    PyByteArrayObject *obj = ((PyByteArrayObject *)self);
+    _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(obj);
+    PyObject *self = (PyObject *)obj;
 
     assert(obj->ob_bytes_object != NULL);
+    assert(requested_size >= 0);
+    assert(obj->ob_exports == 0);
 
     /* All computations are done unsigned to avoid integer overflows
        (see issue #22335). */
     size_t alloc = (size_t) obj->ob_alloc;
     size_t logical_offset = (size_t) (obj->ob_start - obj->ob_bytes);
     size_t size = (size_t) requested_size;
 
-    assert(self != NULL);
-    assert(PyByteArray_Check(self));
     assert(logical_offset <= alloc);
 
-    if (requested_size < 0) {
-        PyErr_Format(PyExc_ValueError,
-            "Can only resize to positive sizes, got %zd", requested_size);
-        return -1;
-    }
-
-    if (requested_size == Py_SIZE(self)) {
-        return 0;
-    }
-    if (!_canresize(obj)) {
-        return -1;
-    }
-
     /* Resize to 0 resets to empty bytes (see issue #153419). */
     if (requested_size == 0) {
         Py_SETREF(obj->ob_bytes_object,
@@ -352,6 +338,30 @@ bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size)
     return 0;
 }
 
+static int
+bytearray_resize_lock_held(PyObject *self, Py_ssize_t requested_size)
+{
+    _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self);
+    PyByteArrayObject *obj = ((PyByteArrayObject *)self);
+
+    assert(self != NULL);
+    assert(PyByteArray_Check(self));
+
+    if (requested_size < 0) {
+        PyErr_Format(PyExc_ValueError,
+            "Can only resize to positive sizes, got %zd", requested_size);
+        return -1;
+    }
+
+    if (requested_size == Py_SIZE(self)) {
+        return 0;
+    }
+    if (!_canresize(obj)) {
+        return -1;
+    }
+    return bytearray_resize_unchecked(obj, requested_size);
+}
+
 int
 PyByteArray_Resize(PyObject *self, Py_ssize_t requested_size)
 {
@@ -616,8 +626,12 @@ bytearray_setslice_linear(PyByteArrayObject *self,
             return -1;
 
         if (lo == 0) {
-            /* Shrink the buffer by advancing its logical start */
+            /* Shrink the buffer by advancing its logical start.  The bytes
+               are removed at this point; the resize below only releases
+               memory. */
             self->ob_start -= growth;
+            Py_SET_SIZE(self, Py_SIZE(self) + growth);
+            growth = 0;
             /*
               0   lo               hi             old_size
               |   |<----avail----->|<-----tail------>|
@@ -635,23 +649,14 @@ bytearray_setslice_linear(PyByteArrayObject *self,
             memmove(buf + lo + bytes_len, buf + hi,
                     Py_SIZE(self) - hi);
         }
-        if (bytearray_resize_lock_held((PyObject *)self,
-                               Py_SIZE(self) + growth) < 0) {
+        if (bytearray_resize_unchecked(self, Py_SIZE(self) + growth) < 0) {
             /* Issue #19578: Handling the memory allocation failure here is
                tricky here because the bytearray object has already been
-               modified. Depending on growth and lo, the behaviour is
-               different.
-
-               If growth < 0 and lo != 0, the operation is completed, but a
-               MemoryError is still raised and the memory block is not
-               shrunk. Otherwise, the bytearray is restored in its previous
-               state and a MemoryError is raised. */
-            if (lo == 0) {
-                self->ob_start += growth;
-                return -1;
-            }
-            /* memmove() removed bytes, the bytearray object cannot be
-               restored in its previous state. */
+               modified: the bytes are removed and the bytearray object
+               cannot be restored in its previous state.
+
+               The operation is completed, but a MemoryError is still
+               raised and the memory block is not shrunk. */
             Py_SET_SIZE(self, Py_SIZE(self) + growth);
             bytearray_write_trailing_null_byte(self);
             res = -1;