GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-157242: Fix PyBytesWriter_Resize() on MemoryError by vstinner · Pull Request #157243 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Sep 10, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@vstinner
pythongh-157242: Leave bytearray unchanged if resize() fails
d0441c4
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.
@vstinner vstinner added the needs backport to 3.15 bugs and security fixes label Sep 10, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Sep 10, 2026
bytearray.resize() must not clear the object on error (memory allocation failure) #157242
Closed
@bedevere-app bedevere-app Bot added the awaiting core review label Sep 10, 2026
vstinner
vstinner commented Sep 10, 2026
View reviewed changes
Comment thread Objects/bytesobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner commented Sep 10, 2026
View reviewed changes
Comment thread Objects/bytearrayobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@vstinner
Add NEWS entries
5f1ef32
@cmaloney

cmaloney commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka
serhiy-storchaka reviewed Sep 10, 2026
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
maurycy
maurycy reviewed Sep 10, 2026
View reviewed changes
Comment thread Objects/bytearrayobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

vstinner and others added 2 commits September 10, 2026 10:39
@vstinner @maurycy
Fix resize() with non-zero start
a1e64d3
Co-authored-by: Maurycy Pawłowski-Wieroński <[email protected]>
@vstinner
Reimplement _PyBytes_Resize() via _PyBytes_ResizeKeepOnError()
4b4bcfb
For the in-place resize code path, no longer call
_Py_ForgetReference() and _PyReftracerTrack() before
PyObject_Realloc().
@vstinner

vstinner commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

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().

maurycy reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

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.

maurycy reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner
Merge branch 'main' into writer_resize2
ba27fe0
@vstinner

vstinner commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

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.

cmaloney reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

maurycy
maurycy reviewed Sep 10, 2026
View reviewed changes
Comment thread Objects/bytearrayobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

vstinner added 2 commits September 10, 2026 12:03
@vstinner
Fix resize() when shrinking the bytearray
4d581fa
@vstinner
Fix take_bytes()
f51cba5
@vstinner

vstinner commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

@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 reacted with eyes emoji
All reactions
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@maurycy

maurycy commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

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

vstinner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

maurycy
maurycy reviewed Sep 10, 2026
View reviewed changes
Comment thread Lib/test/test_bytes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Objects/bytesobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Objects/bytearrayobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@maurycy

maurycy commented Sep 10, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

@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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cmaloney
cmaloney reviewed Sep 10, 2026
View reviewed changes
Comment thread Lib/test/test_bytes.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Objects/bytearrayobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

vstinner added 6 commits September 11, 2026 18:29
@vstinner
Fix typo in test_take_bytes_error(): pass to_take argument
7116757
@vstinner
Add inject_memory_error()
0ca5c9c
@vstinner
Remove alloc parameter of bytearray_reinit_from_bytes()
ba1e2c8
alloc is just the ob_bytes_object size.
@vstinner
Add bytearray_realign_data_lock_held()
e9e1f02
@vstinner
Don't call _PyReftracerTrack() on dangling pointer
a0a3753
@vstinner
Add test_bytes_resize_tracer()
53c3b51
vstinner added 4 commits September 12, 2026 00:38
@vstinner
Merge branch 'main' into writer_resize2
306f132
@vstinner
Revert bytearray changes
a9a1423
@vstinner
Resize: set pv earlier
9fa1305
@vstinner
Move PyBytesWriter test
9ce6a99
@vstinner
vstinner enabled auto-merge (squash) September 11, 2026 22:49
@vstinner

vstinner commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

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.

maurycy reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner vstinner removed the needs backport to 3.15 bugs and security fixes label Sep 11, 2026
@vstinner
vstinner merged commit f5dd52d into python:main Sep 11, 2026
56 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@vstinner
vstinner deleted the writer_resize2 branch September 11, 2026 23:22
@bedevere-app bedevere-app Bot removed the awaiting core review label Sep 11, 2026
@vstinner vstinner changed the title gh-157242: Leave bytearray unchanged if resize() fails gh-157242: Fix PyBytesWriter_Resize() on MemoryError Sep 11, 2026
@vstinner

vstinner commented Sep 11, 2026

Copy link
Copy Markdown
Member Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot

bedevere-bot commented Sep 12, 2026

Copy link
Copy Markdown

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Arch Linux TraceRefs 3.x (tierless) has failed when building commit f5dd52d.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/484/builds/7361) and take a look at the build logs.
  4. Check if the failure is related to this commit (f5dd52d) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/484/builds/7361

Failed tests:

  • test_capi

Summary of the results of the build (if available):

==

Click to see traceback logs
Note: switching to 'f5dd52df16e1b3f3f8cc2762f1de783a41d69f4f'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at f5dd52df16e gh-157242: Fix PyBytesWriter_Resize() on MemoryError (#157243)
Switched to and reset branch 'main'

make: *** [Makefile:2481: buildbottest] Error 2
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner vstinner mentioned this pull request Sep 12, 2026
gh-157242: Leave bytearray unchanged if resize() fails #157340
Merged
@vstinner

vstinner commented Sep 12, 2026

Copy link
Copy Markdown
Member Author

The buildbot AMD64 Arch Linux TraceRefs 3.x (tierless) has failed when building commit f5dd52d.

Ah, Py_TRACE_REFS builds are special, they cannot test set_nomemory(). I wrote PR gh-157350 to fix test_capi on Py_TRACE_REFS build.

maurycy reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

clin1234 pushed a commit to clin1234/cpython that referenced this pull request Sep 12, 2026
@vstinner @maurycy @clin1234
pythongh-157242: Fix PyBytesWriter_Resize() on MemoryError (python#15…
b73337d
…7243)

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.

Co-authored-by: Maurycy Pawłowski-Wieroński <[email protected]>
johnslavik pushed a commit to johnslavik/cpython that referenced this pull request Sep 14, 2026
@vstinner @maurycy @johnslavik
pythongh-157242: Fix PyBytesWriter_Resize() on MemoryError (python#15…
6a68fda
…7243)

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.

Co-authored-by: Maurycy Pawłowski-Wieroński <[email protected]>
@vstinner vstinner added needs backport to 3.14 bugs and security fixes needs backport to 3.15 bugs and security fixes labels Oct 12, 2026
@miss-islington-app

miss-islington-app Bot commented Oct 12, 2026

Copy link
Copy Markdown

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington-app

miss-islington-app Bot commented Oct 12, 2026

Copy link
Copy Markdown

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington-app

miss-islington-app Bot commented Oct 12, 2026

Copy link
Copy Markdown

Sorry, @vstinner, I could not cleanly backport this to 3.14 due to a conflict.

Please backport manually with cherry_picker, see the devguide for more information.

cherry_picker f5dd52df16e1b3f3f8cc2762f1de783a41d69f4f 3.14
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington-app miss-islington-app Bot assigned vstinner Oct 12, 2026
@miss-islington-app

miss-islington-app Bot commented Oct 12, 2026

Copy link
Copy Markdown

Sorry, @vstinner, I could not cleanly backport this to 3.15 due to a conflict.

Please backport manually with cherry_picker, see the devguide for more information.

cherry_picker f5dd52df16e1b3f3f8cc2762f1de783a41d69f4f 3.15
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner added a commit to vstinner/cpython that referenced this pull request Oct 12, 2026
@vstinner @maurycy
pythongh-157242: Fix PyBytesWriter_Resize() on MemoryError (python#15…
2f471c9
…7243)

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.

Add BaseWriterTest for BytesWriterTest and ByteArrayWriterTest.

Add _testcapi.PyBytesWriter_small_buffer constant.

Co-authored-by: Maurycy Pawłowski-Wieroński <[email protected]>
(cherry picked from commit f5dd52d)
@bedevere-app

bedevere-app Bot commented Oct 12, 2026

Copy link
Copy Markdown

GH-159260 is a backport of this pull request to the 3.15 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 bugs and security fixes label Oct 12, 2026
vstinner added a commit that referenced this pull request Oct 12, 2026
@vstinner @maurycy
[3.15] gh-157242: Fix PyBytesWriter_Resize() on MemoryError (#157243) (…
73e6301
…#159260)

gh-157242: Fix PyBytesWriter_Resize() on MemoryError (#157243)

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.

Add BaseWriterTest for BytesWriterTest and ByteArrayWriterTest.

Add _testcapi.PyBytesWriter_small_buffer constant.


(cherry picked from commit f5dd52d)

Co-authored-by: Maurycy Pawłowski-Wieroński <[email protected]>
vstinner added a commit that referenced this pull request Oct 16, 2026
@vstinner @maurycy
[3.14] gh-157242: Do not close io.BytesIO on MemoryError (#159529)
2e9acb4
* gh-157242: Add _PyBytes_ResizeKeepOnError() function (#157243)

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

Co-authored-by: Maurycy Pawłowski-Wieroński <[email protected]>
(cherry picked from commit f5dd52d)

* gh-157242: Fix undefined behavior in _PyBytes_ResizeKeepOnError() (#157352)

Fix undefined behavior in _PyBytes_ResizeKeepOnError(): raise
OverflowError on integer overflow.

(cherry picked from commit 63d4398)

* gh-157242: Do not close io.BytesIO on MemoryError (#157344)

Replace _PyBytes_Resize() with _PyBytes_ResizeKeepOnError().

Fix truncate(): on error, restore string_size to its previous value.

(cherry picked from commit 23eda00)

---------

Co-authored-by: Maurycy Pawłowski-Wieroński <[email protected]>
@StanFromIreland StanFromIreland removed the needs backport to 3.14 bugs and security fixes label Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmaloney cmaloney cmaloney left review comments
@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments
+1 more reviewer
@maurycy maurycy maurycy left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@vstinner vstinner

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@vstinner @cmaloney @maurycy @bedevere-bot @serhiy-storchaka @StanFromIreland
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.