Fix a leak in RawJSON() when the argument is rejected - #239
Merged
Merged
Conversation
RawJSON_new allocated the object before parsing its argument and returned NULL without releasing it when the "U" check failed, so every RawJSON(<non-str>) leaked one object. Parse first and allocate second, as the other constructors in this file do, and check tp_alloc's result. Add a tracemalloc test in the shape of the existing ones: 1000 rejected calls leave 1001 extra allocations before this change and 0 after. Also fix the stub: RawJSON.value is the str that was passed, not a RawJSON. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Contributor
|
Thank you, will merge before next release. |
Contributor
|
Merged in just released v1.24. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
RawJSON(value)leaks one object every time the argument is rejected:RawJSON_newcallstp_allocbeforePyArg_ParseTupleAndKeywords, and the failure branch returnsNULLwithout releasing it. The result oftp_allocwas also never checked.This reorders the constructor to parse first and allocate second, matching
decoder_new/encoder_new/validator_new, and adds a tracemalloc test next to the existing ones intest_memory_leaks.py. On CPython 3.12.13, 100,000 rejected calls increasedsys.getallocatedblocks()by 100,001 before this change and by 1 after; the 1.23 source I measured has the sameRawJSON_newas master.Also fixes the stub, where
RawJSON.valuewas typed asRawJSONinstead ofstr. That hunk is independent; drop it if you would rather not have it here.Assisted-by: Claude Code:claude-fable-5-1