gh-157242: Leave bytearray unchanged if resize() fails

[vstinner]

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.

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

[cmaloney]

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.

[vstinner]

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]

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]

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]

@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]

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

[maurycy]

@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

[vstinner]

@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]

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

[vstinner]

@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]

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]

@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. */