Skip to content

gh-155742: Use PyBytesWriter in marshal - #155748

Open
vstinner wants to merge 3 commits into
python:mainfrom
vstinner:marshal
Open

gh-155742: Use PyBytesWriter in marshal#155748
vstinner wants to merge 3 commits into
python:mainfrom
vstinner:marshal

Conversation

@vstinner

@vstinner vstinner commented Aug 13, 2026

Copy link
Copy Markdown
Member

Replace soft deprecated PyBytes_FromStringAndSize() and _PyBytes_Resize() with PyBytesWriter.

Replace soft deprecated PyBytes_FromStringAndSize() and
_PyBytes_Resize() with PyBytesWriter.
No PyBytesWriter is needed.

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do not think the current code is broken.

Comment thread Python/marshal.c
Comment thread Python/marshal.c
@vstinner

Copy link
Copy Markdown
Member Author

I do not think the current code is broken.

I didn't say that the current code is broken. The PR only just avoids the soft deprecated PyBytes_FromStringAndSize() function.

I reworked the error handling.

@serhiy-storchaka: Please review the updated PR.

@vstinner

Copy link
Copy Markdown
Member Author

I wrote a script to test manually this PR by injecting MemoryError at different places:

import marshal
import io
import _testcapi
obj = b'x' * (1024 * 1024)
file = io.BytesIO()
for i in range(10):
    try:
        try:
            _testcapi.set_nomemory(i)
            res = marshal.dump(obj, file)
        finally:
            _testcapi.remove_mem_hooks()
    except Exception as exc:
        print(f"marshal.dump failed: {exc!r}")
    else:
        print(f"{res=}")

Before, the code failed with an assertion error. With my latest change, the code works is all cases (always raise MemoryError as expected).

@vstinner

vstinner commented Sep 3, 2026

Copy link
Copy Markdown
Member Author

Ah, I noticed that the PyMarshal C API is not tested by test_capi currently. So I wrote PR gh-156890 to add tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review interpreter-core (Objects, Python, Grammar, and Parser dirs) skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants