Repository navigation
Conversation
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
left a comment
cmaloney
left a comment
There was a problem hiding this comment.
@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
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.
|
Reworked to keep ... we could have a failed shrink not raise at all, since the larger block is valid? I still like the error myself. |
…hecked resize Only the front path of bytearray_setslice_linear() calls bytearray_resize_unchecked(): it has already advanced ob_start and reduced the size, so bytearray_resize_lock_held() would return early without releasing memory. Other slices go through bytearray_resize_lock_held() again and keep the failure fixup to themselves. test_resize_error now also covers slices in the middle and at the end, and checks the null byte after the data with float(). Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
gpshead
added
the
🔨 test-with-buildbots
|
🤖 New build scheduled with the buildbot fleet by @gpshead for commit 8601525 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F160304%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
bedevere-bot
removed
the
🔨 test-with-buildbots
After a failed
del b[:n],bytearray_setslice_linear()movedob_startback, butbytearray_resize_storage()had already realigned the data and resetob_start, so the bytearray then startednbytes before its buffer. The front-delete path now movesob_startand 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, raisingMemoryErroras the non-front path already does. The checks inbytearray_resize_lock_held()are split from a newbytearray_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
bytearrayreads and writes outside its buffer afterdel b[:n]fails withMemoryError#160303