gh-157242: Fix PyBytesWriter_Resize() on MemoryError by vstinner · Pull Request #157243 · python/cpython

vstinner · 2026-09-10T00:40:58Z

If PyBytesWriter_Resize() fails, leave the writer unchanged.

Add a new internal _PyBytes_ResizeKeepOnError() function similar to _PyBytes_Resize() but leaves the bytes object unchanged on error.

Issue: bytearray.resize() must not clear the object on error (memory allocation failure) #157242

If bytearray.resize() or bytearray.take_bytes() fails, leave the bytearray unchanged. If PyBytesWriter_Resize() fails, leave the writer unchanged. Add a new internal _PyBytes_ResizeKeepOnError() function similar to _PyBytes_Resize() but leaves the bytes object unchanged on error.

cmaloney · 2026-09-10T02:31:20Z

It would be nice if the "guaranteed no global" case was also used for the PyBytes_FromStringAndSize(NULL, size); call. That the first parameter there must be NULL to avoid a global is unintuitive.

serhiy-storchaka

No need to add a new parameter, it saves nothing. _PyBytes_Resize can be implemented via _PyBytes_ResizeKeepOnError.

I wonder if we can simply change the behavior of _PyBytes_Resize.

Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com>

For the in-place resize code path, no longer call _Py_ForgetReference() and _PyReftracerTrack() before PyObject_Realloc().

vstinner · 2026-09-10T08:52:04Z

Please review the updated PR. I addressed reviews.

@maurycy found a fix for the memmove() code path which worried me. I applied his suggestion and added a test.

@serhiy-storchaka:

No need to add a new parameter, it saves nothing. _PyBytes_Resize can be implemented via _PyBytes_ResizeKeepOnError. I wonder if we can simply change the behavior of _PyBytes_Resize.

Thanks for the advice. I reworked _PyBytes_Resize(): for the in-place resize code path, no longer call _Py_ForgetReference() and _PyReftracerTrack() before PyObject_Realloc(). Only call them on success. With this change, I was able to easy implement _PyBytes_Resize() with _PyBytes_ResizeKeepOnError().

vstinner · 2026-09-10T08:55:50Z

It would be nice if the "guaranteed no global" case was also used for the PyBytes_FromStringAndSize(NULL, size); call. That the first parameter there must be NULL to avoid a global is unintuitive.

Maybe PyBytes_FromStringAndSize() documentation should be elaborated to explain that PyBytes_FromStringAndSize(NULL, size) can be mutated, whereas PyBytes_FromStringAndSize(str, size) must not be mutated?

Note: PR gh-156996 does fix PyBytes_FromStringAndSize() usage in bytearray. I don't try to replace this fix.

vstinner · 2026-09-10T09:21:11Z

bytearray.resize() and bytearray.take_bytes() have been fixed to no longer use a singleton: I merged main in my PR to get the PR gh-156996 fix.

vstinner · 2026-09-10T10:28:45Z

@maurycy: I modified resize() and take_bytes() to avoid memmove() usage if we would be unable to revert the bytesarray to its previous state on MemoryError. Does it look correct to you?

I also added more tests injecting MemoryError.

maurycy · 2026-09-10T11:29:36Z

@vstinner: Thank you. I will take a look more carefully later today.

maurycy · 2026-09-10T15:52:01Z

@vstinner Two more findings. That's such a nice PR!

Good news: the PR fixes this regression:

2026-09-10T17:44:57.890346000+0200 maurycy@gimel /Users/maurycy/work/cpython (main 9bd670c?) % ./python.exe 
Python 3.16.0a0 (heads/main:9bd670cba21, Sep 10 2026, 17:37:43) [Clang 21.0.0 (clang-2100.1.1.101)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import _testcapi
... import sys
... 
... ba = bytearray(b"abc123")
... try:
...     _testcapi.set_nomemory(0)
...     del ba[1:-1]
... except MemoryError: pass
... finally: _testcapi.remove_mem_hooks()
... 
>>> len(ba)
Traceback (most recent call last):
  File "<python-input-1>", line 1, in <module>
    len(ba)
    ~~~^^^^
SystemError: <built-in function len> returned NULL without setting an exception
>>> bytes(ba)
Traceback (most recent call last):
  File "<python-input-2>", line 1, in <module>
    bytes(ba)
    ~~~~~^^^^
ValueError: size must be >= 0
>>> 
2026-09-10T17:45:16.126365000+0200 maurycy@gimel /Users/maurycy/work/cpython-pr157243 (pr157243 f51cba5?) % ./python.exe 
Python 3.16.0a0 (heads/pr157243:f51cba57d3e, Sep 10 2026, 14:31:04) [Clang 21.0.0 (clang-2100.1.1.101)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import _testcapi
... import sys
... 
... ba = bytearray(b"abc123")
... try:
...     _testcapi.set_nomemory(0)
...     del ba[1:-1]
... except MemoryError: pass
... finally: _testcapi.remove_mem_hooks()
... 
>>> len(ba)
2
>>> bytes(ba)
b'a3'
>>>

Perhaps we cover this with test, too?

Bad news:

2026-09-10T17:47:19.394053000+0200 maurycy@gimel /Users/maurycy/work/cpython-pr157243 (pr157243 f51cba5?) % ./python.exe
Python 3.16.0a0 (heads/pr157243:f51cba57d3e, Sep 10 2026, 14:31:04) [Clang 21.0.0 (clang-2100.1.1.101)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import _testcapi
... import sys
... 
... ba = bytearray(b"abc123456")
... ba += b"7"
... 
... try:
...     _testcapi.set_nomemory(0)
...     del ba[::2]
... except MemoryError: pass
... finally: _testcapi.remove_mem_hooks()
... 
... ba
... 
bytearray(b'b135734567')
>>> 
2026-09-10T17:48:34.225959000+0200 maurycy@gimel /Users/maurycy/work/cpython (main 9bd670c?) % ./python.exe 
Python 3.16.0a0 (heads/main:9bd670cba21, Sep 10 2026, 17:37:43) [Clang 21.0.0 (clang-2100.1.1.101)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import _testcapi
... import sys
... 
... ba = bytearray(b"abc123456")
... ba += b"7"
... 
... try:
...     _testcapi.set_nomemory(0)
...     del ba[::2]
... except MemoryError: pass
... finally: _testcapi.remove_mem_hooks()
... 
... ba
... 
bytearray(b'')

Unfortunately, I don't have bandwitch to investigate what's the exact fix here

alloc is just the ob_bytes_object size.

vstinner · 2026-09-11T19:07:39Z

@maurycy @cmaloney: Oh, you made multiple comments. Let me push a first batch of fixes:

Fix typo in test_take_bytes_error(): pass to_take argument
Add inject_memory_error() helper function: share code in test_bytes
Remove alloc parameter of bytearray_reinit_from_bytes()
Add bytearray_realign_data_lock_held(): share code in Objects/bytearrayobject.c
Don't call _PyReftracerTrack() on dangling pointer
Add test_bytes_resize_tracer(): check that PyRefTracer_DESTROY and PyRefTracer_CREATE events are emitted

@maurycy:

_PyReftracerTrack(v, PyRefTracer_DESTROY);
But v is now freed by PyObject_Realloc()? But ASan should've caught this?

I checked tracemalloc "ref tracer" callback: it does read the object memory, so the pointer must not be a dangling pointer or Python will crash.

I reverted my change: _PyReftracerTrack(v, PyRefTracer_DESTROY) is now called before PyObject_Realloc() again (as done currently).

maurycy · 2026-09-11T19:54:51Z

@vstinner Thanks, I appreciate it! Could you re-request a review once you've pushed the remaining fixes?

vstinner · 2026-09-11T20:20:08Z

@maurycy @cmaloney: I finished to update the PR, you can now review it.

I pushed a fix to write the null byte on bytearray_realign_data_lock_held() error path.

I'm not sure that test_bytes_resize_tracer() is needed: it's a functional test on PyRefTracer_DESTROY and PyRefTracer_CREATE events. I wrote it to make sure that we don't mess up with these events if the code is modified later.

I also wrote a large change locally to check bytearray consistency in all methods modifying bytearray, but I prefer to propose a separated PR later for this change. For example, my check makes sure that the bytearray has trailing null byte. See also PR gh-156943 which uses a "canary byte" to detect buffer overflow in the PyBytesWriter C API.

vstinner · 2026-09-11T20:25:18Z

 static void
-bytearray_reinit_from_bytes(PyByteArrayObject *self, Py_ssize_t size,
-                            Py_ssize_t alloc)
+bytearray_reinit_from_bytes(PyByteArrayObject *self, Py_ssize_t size)


Removing the alloc parameter is not strictly needed by this PR, but I removed it to help me checking that the bytearray remains consistent with my changes.

vstinner · 2026-09-11T20:28:39Z

@maurycy:

@vstinner Two more findings. That's such a nice PR! (...) del ba[1:-1] (...) del ba[::2]

I'm not sure that Python 3.14 and older leaves the bytearray unchanged on MemoryError.

For example, bytearray_setslice_linear() contains the following comment in Python 3.14 on bytearray_resize_lock_held() error path:

            /* memmove() removed bytes, the bytearray object cannot be
               restored in its previous state. */

Always use _PyBytes_ResizeKeepOnError(), but move remaining bytes on error.

Rename bytearray_realign_data_lock_held() to bytearray_resize_storage(). On error, only move data if ob_start was different than ob_bytes. Add one more test: resize() shrinks without logical offset.

maurycy · 2026-09-11T21:09:20Z

@vstinner You're right. Both del ba[1:-1] and del ba[::2] behave the same on 3.14. The del ba[1:-1] is since 732224e:

`git bisect log` (`v3.14.0..main`, `Objects/bytearrayobject.c`)

2026-09-11T22:58:48.213303000+0200 maurycy@gimel /Users/maurycy/src/github.com/maurycy/cpython (main 392ad7e?) % cat repro.py 
import _testcapi
ba = bytearray(b"abc123")
try:
    _testcapi.set_nomemory(0); del ba[1:-1]
except MemoryError: pass
finally: _testcapi.remove_mem_hooks()
assert len(ba) == 2, ba
2026-09-11T22:58:48.905587000+0200 maurycy@gimel /Users/maurycy/src/github.com/maurycy/cpython (main 392ad7e?) % git bisect start upstream-ro/main v3.14.0 -- Objects/bytearrayobject.c
Bisecting: a merge base must be tested
[b092705907c758d4f9742028652c9802f9f03dd3] Python 3.14.0b1
maurycy@gimel cpython  (git)-[tags/v3.14.0b1|bisect]?- % git bisect run sh -c 'make -s -j8 >/dev/null 2>&1 || exit 125; ./python.exe repro.py; test $? -eq 0'
running 'sh' '-c' 'make -s -j8 >/dev/null 2>&1 || exit 125; ./python.exe repro.py; test $? -eq 0'
Bisecting: 9 revisions left to test after this (roughly 3 steps)
[9976c2b6349a079ae39931d960b8c147e21c6c3f] gh-143195: fix UAF in `{bytearray,memoryview}.hex(sep)` via re-entrant `sep.__len__` (#143209)
running 'sh' '-c' 'make -s -j8 >/dev/null 2>&1 || exit 125; ./python.exe repro.py; test $? -eq 0'
Traceback (most recent call last):
  File "/Users/maurycy/src/github.com/maurycy/cpython/repro.py", line 7, in <module>
    assert len(ba) == 2, ba
           ~~~^^^^
SystemError: <built-in function len> returned NULL without setting an exception
Bisecting: 4 revisions left to test after this (roughly 2 steps)
[6416e6ebe5b88087ada6f4a56972053edb9c2e01] gh-129559: Remove extra dot in bytearray.resize AC (#140134)
running 'sh' '-c' 'make -s -j8 >/dev/null 2>&1 || exit 125; ./python.exe repro.py; test $? -eq 0'
Bisecting: 2 revisions left to test after this (roughly 1 step)
[e265ce8a563ba7f91c5ada0592de8cb85622b433] gh-139871: Optimize small takes in bytearray.take_bytes (GH-141741)
running 'sh' '-c' 'make -s -j8 >/dev/null 2>&1 || exit 125; ./python.exe repro.py; test $? -eq 0'
Traceback (most recent call last):
  File "/Users/maurycy/src/github.com/maurycy/cpython/repro.py", line 7, in <module>
    assert len(ba) == 2, ba
           ~~~^^^^
SystemError: <built-in function len> returned NULL without setting an exception
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[732224e1139f7ed4fe0259a2dad900f84910949e] gh-139871: Add `bytearray.take_bytes([n])` to efficiently extract `bytes` (GH-140128)
running 'sh' '-c' 'make -s -j8 >/dev/null 2>&1 || exit 125; ./python.exe repro.py; test $? -eq 0'
Traceback (most recent call last):
  File "/Users/maurycy/src/github.com/maurycy/cpython/repro.py", line 7, in <module>
    assert len(ba) == 2, ba
           ~~~^^^^
SystemError: <built-in function len> returned NULL without setting an exception
732224e1139f7ed4fe0259a2dad900f84910949e is the first 'bad' commit
commit 732224e1139f7ed4fe0259a2dad900f84910949e
Author: Cody Maloney <cmaloney@users.noreply.github.com>
Date:   2025-11-13 05:19:44 -0800

    gh-139871: Add `bytearray.take_bytes([n])` to efficiently extract `bytes` (GH-140128)
    
    Update `bytearray` to contain a `bytes` and provide a zero-copy path to
    "extract" the `bytes`. This allows making several code paths more efficient.
    
    This does not move any codepaths to make use of this new API. The documentation
    changes include common code patterns which can be made more efficient with
    this API.
    
    ---
    
    When just changing `bytearray` to contain `bytes` I ran pyperformance on a
    `--with-lto --enable-optimizations --with-static-libpython` build and don't see
    any major speedups or slowdowns with this; all seems to be in the noise of
    my machine (Generally changes under 5% or benchmarks that don't touch
    bytes/bytearray).
    
    
    Co-authored-by: Victor Stinner <vstinner@python.org>
    Co-authored-by: Maurycy Pawłowski-Wieroński <5383+maurycy@users.noreply.github.com>

 Doc/library/stdtypes.rst                           |  24 +++
 Doc/whatsnew/3.15.rst                              |  80 +++++++
 Include/cpython/bytearrayobject.h                  |  16 +-
 Include/internal/pycore_bytesobject.h              |   8 +
 Lib/test/test_bytes.py                             |  81 +++++++
 Lib/test/test_capi/test_bytearray.py               |   5 +-
 Lib/test/test_sys.py                               |   2 +-
 .../2025-10-14-18-24-16.gh-issue-139871.SWtuUz.rst |   2 +
 Objects/bytearrayobject.c                          | 238 ++++++++++++++-------
 Objects/bytesobject.c                              |   8 +-
 Objects/clinic/bytearrayobject.c.h                 |  39 +++-
 11 files changed, 407 insertions(+), 96 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-10-14-18-24-16.gh-issue-139871.SWtuUz.rst
bisect found first 'bad' commit
maurycy@gimel cpython  (git)-[bisect/bad|bisect]?- %

I cannot approve but it LGreatTM.

Test without logical offset.

vstinner · 2026-09-11T22:50:40Z

The PR became quite big, so I modified it to focus strictly on _PyBytes_ResizeKeepOnError(). Once this PR will be merged, I will open a second issue to fix bytearray methods.

vstinner · 2026-09-11T23:25:46Z

Ok, I merged the first part. I created the follow-up PR for bytearray: PR gh-157340.

vstinner added the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Sep 10, 2026

bedevere-app Bot mentioned this pull request Sep 10, 2026

bytearray.resize() must not clear the object on error (memory allocation failure) #157242

Open

bedevere-app Bot added the awaiting core review label Sep 10, 2026

vstinner commented Sep 10, 2026

View reviewed changes

Comment thread Objects/bytesobject.c Outdated

vstinner commented Sep 10, 2026

View reviewed changes

Comment thread Objects/bytearrayobject.c Outdated

Add NEWS entries

5f1ef32

serhiy-storchaka reviewed Sep 10, 2026

View reviewed changes

maurycy reviewed Sep 10, 2026

View reviewed changes

Comment thread Objects/bytearrayobject.c

vstinner and others added 2 commits September 10, 2026 10:39

Fix resize() with non-zero start

a1e64d3

Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com>

Reimplement _PyBytes_Resize() via _PyBytes_ResizeKeepOnError()

4b4bcfb

For the in-place resize code path, no longer call _Py_ForgetReference() and _PyReftracerTrack() before PyObject_Realloc().

Merge branch 'main' into writer_resize2

ba27fe0

maurycy reviewed Sep 10, 2026

View reviewed changes

Comment thread Objects/bytearrayobject.c

vstinner added 2 commits September 10, 2026 12:03

Fix resize() when shrinking the bytearray

4d581fa

Fix take_bytes()

f51cba5

maurycy reviewed Sep 10, 2026

View reviewed changes

Comment thread Lib/test/test_bytes.py Outdated

Comment thread Objects/bytesobject.c

Comment thread Objects/bytearrayobject.c Outdated

cmaloney reviewed Sep 10, 2026

View reviewed changes

Comment thread Lib/test/test_bytes.py Outdated

Comment thread Objects/bytearrayobject.c Outdated

vstinner added 6 commits September 11, 2026 18:29

Fix typo in test_take_bytes_error(): pass to_take argument

7116757

Add inject_memory_error()

0ca5c9c

Remove alloc parameter of bytearray_reinit_from_bytes()

ba1e2c8

alloc is just the ob_bytes_object size.

Add bytearray_realign_data_lock_held()

e9e1f02

Don't call _PyReftracerTrack() on dangling pointer

a0a3753

Add test_bytes_resize_tracer()

53c3b51

vstinner added 2 commits September 11, 2026 21:58

Write trailing null bytes on MemoryError

fb67d23

Add bytearray_write_trailing_null_byte()

6762d16

vstinner commented Sep 11, 2026

View reviewed changes

vstinner added 2 commits September 11, 2026 22:51

Optimize bytearray_realign_data_lock_held()

909270c

Always use _PyBytes_ResizeKeepOnError(), but move remaining bytes on error.

Fix bytearray_resize_storage()

0a33fd4

Rename bytearray_realign_data_lock_held() to bytearray_resize_storage(). On error, only move data if ob_start was different than ob_bytes. Add one more test: resize() shrinks without logical offset.

vstinner added 6 commits September 12, 2026 00:26

Add one more take_bytes() error test

dfb05bf

Test without logical offset.

no need for testing PyBytesWriter_Resize() shrinking

2f1e965

Merge branch 'main' into writer_resize2

306f132

Revert bytearray changes

a9a1423

Resize: set pv earlier

9fa1305

Move PyBytesWriter test

9ce6a99

vstinner enabled auto-merge (squash) September 11, 2026 22:49

vstinner removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Sep 11, 2026

vstinner merged commit f5dd52d into python:main Sep 11, 2026
56 checks passed

vstinner deleted the writer_resize2 branch September 11, 2026 23:22

bedevere-app Bot removed the awaiting core review label Sep 11, 2026

vstinner changed the title ~~gh-157242: Leave bytearray unchanged if resize() fails~~ gh-157242: Fix PyBytesWriter_Resize() on MemoryError Sep 11, 2026

Uh oh!

Uh oh!

Conversation

vstinner commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmaloney commented Sep 10, 2026

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vstinner commented Sep 10, 2026

Uh oh!

vstinner commented Sep 10, 2026

Uh oh!

vstinner commented Sep 10, 2026

Uh oh!

Uh oh!

vstinner commented Sep 10, 2026

Uh oh!

maurycy commented Sep 10, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maurycy commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vstinner commented Sep 11, 2026

Uh oh!

maurycy commented Sep 11, 2026

Uh oh!

vstinner commented Sep 11, 2026

Uh oh!

vstinner Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

vstinner commented Sep 11, 2026

Uh oh!

maurycy commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Sep 11, 2026

Uh oh!

Uh oh!

vstinner commented Sep 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vstinner commented Sep 10, 2026 •

edited

Loading

maurycy commented Sep 10, 2026 •

edited

Loading

maurycy commented Sep 11, 2026 •

edited

Loading