GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||
GeezerVison is showing github.com — gh-157242: Fix PyBytesWriter_Resize() on MemoryError by vstinner · Pull Request #157243 · python/cpython Conversation
|
|
It would be nice if the "guaranteed no global" case was also used for the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Maurycy Pawłowski-Wieroński <[email protected]>
For the in-place resize code path, no longer call _Py_ForgetReference() and _PyReftracerTrack() before PyObject_Realloc().
|
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.
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(). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Maybe Note: PR gh-156996 does fix PyBytes_FromStringAndSize() usage in bytearray. I don't try to replace this fix. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@vstinner: Thank you. I will take a look more carefully later today. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
alloc is just the ob_bytes_object size.
|
The PR became quite big, so I modified it to focus strictly on |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Ok, I merged the first part. I created the follow-up PR for bytearray: PR gh-157340. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…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]>
…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]>
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.15. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry, @vstinner, I could not cleanly backport this to Please backport manually with cherry_picker, see the devguide for more information. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Sorry, @vstinner, I could not cleanly backport this to Please backport manually with cherry_picker, see the devguide for more information. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…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)
|
GH-159260 is a backport of this pull request to the 3.15 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…#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]>
* 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]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.