Skip to content

gh-160303: Fix bytearray state after a failed del b[:n] - #160304

Open
gpshead wants to merge 3 commits into
python:mainfrom
gpshead:scanfix-bytearrayobjec-qrz6idtc
Open

gpshead wants to merge 3 commits into
python:mainfrom
gpshead:scanfix-bytearrayobjec-qrz6idtc

Conversation

@gpshead

@gpshead gpshead commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

After a failed del b[:n], bytearray_setslice_linear() moved ob_start back, but bytearray_resize_storage() had already realigned the data and reset ob_start, so the bytearray then started n bytes before its buffer. The front-delete path now moves ob_start and the size before the shrinking resize and no longer restores anything, so a failed shrink leaves the bytes removed and the larger block in place, raising MemoryError as the non-front path already does. The checks in bytearray_resize_lock_held() are split from a new bytearray_resize_unchecked() so the slice code can call it directly without changing other callers, and the non-front failure path now writes the trailing null byte it was missing.

Fixes #160303

ANT-2026-QRZ6IDTC

When shrinking the storage of a bytearray which has a logical offset,
allocate the new buffer before modifying the object, so that
bytearray_setslice_linear() can restore ob_start if the allocation fails.

@cmaloney cmaloney left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vstinner recently made it so that failed allocations keep the pre-existing bytes (#157242). That means we should always have a valid bytes. Backported to 3.15 (3.14 has a different internal bytearray structure).

bytearray implements a fast delete at front by having an ob_start member which can be incremented. Also fast delete at end: Leave ob_alloc as is and call Py_SET_SIZE to set the new size. The "effective" bytes being smaller than the allocation is okay.

The bug report sounds like we're getting that pointer manipulation off? I would strongly prefer not allocating a new bytes here with PyBytes_FromStringAndSize

Comment thread Lib/test/test_bytes.py Resolved
@gpshead gpshead added the sprint label Oct 23, 2026
@github-project-automation github-project-automation Bot moved this to Todo in Sprint Oct 23, 2026
@gpshead gpshead moved this from Todo to In Progress in Sprint Oct 25, 2026
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.
@gpshead

gpshead commented Nov 1, 2026 •

Copy link
Copy Markdown
Member Author

Reworked to keep ob_start and avoid the extra allocation: the delete now moves ob_start and the size before the resize. A failed shrink leaves the bytes removed and the larger block in place, raising MemoryError as the non-front path already does.

... we could have a failed shrink not raise at all, since the larger block is valid? I still like the error myself. del implies someone was hoping to release memory. Silently not doing so is presumably only going to happen in a high memory pressure situation? Being silent would then just mean that something else fails soon after.

@gpshead
gpshead requested a review from cmaloney November 1, 2026 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

bytearray reads and writes outside its buffer after del b[:n] fails with MemoryError

2 participants