gh-151895: Fix marshal.loads() crash on dict reference-tracking failure#151896
Open
tonghuaroot wants to merge 3 commits into
Open
gh-151895: Fix marshal.loads() crash on dict reference-tracking failure#151896tonghuaroot wants to merge 3 commits into
tonghuaroot wants to merge 3 commits into
Conversation
… failure Loading a reference-tracked dictionary dereferenced a NULL pointer when the allocation that registers it for back-references failed under low memory. It now raises MemoryError, matching the tuple and list paths.
Member
|
We usually don't add tests for cases where the return value of malloc is not checked, so I don't think this test case is necessary. Otherwise, the change looks good to me. |
Per review: CPython does not usually add tests for unchecked-malloc failure paths. Keep just the r_object() TYPE_DICT guard.
Contributor
Author
|
Good point — dropped the test in a178dac; the |
aisk
approved these changes
Jun 22, 2026
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
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.
r_object()inPython/marshal.ccould dereference aNULLdictionary. When loading a back-referenced dict (FLAG_REF), theTYPE_DICTbranch callsR_REF(v), which expands tov = r_ref(v, flag, p)and can returnNULLwhenr_ref()'s internalPyList_Append()fails underMemoryError. The branch then fell through toPyDict_SetItem(v, ...)withv == NULL, crashing the interpreter (SIGSEGV on a release build, an assertion abort on a debug build) instead of raising a cleanMemoryError.Every sibling container branch —
TYPE_TUPLE,TYPE_LIST,TYPE_FROZENDICT, and the set/frozenset branches — already followsR_REF(v)with anif (v == NULL) break;(orif (idx < 0)) guard;TYPE_DICTwas the only one missing it. This adds the guard.r_ref()has already decref'd the dict on its failure path, so the earlybreakpropagates the exception cleanly andmarshal.loads()raisesMemoryError.A regression test is added in
Lib/test/test_marshal.pythat uses_testcapi.set_nomemory()to fail the reference-tracking allocation while loading a back-referenced dict: it crashes the interpreter without this change and raisesMemoryErrorwith it.test_marshalpasses.This is a regression introduced in 2e37d83 (gh-148653), which refactored the
TYPE_DICT/TYPE_FROZENDICTbranches to share code and dropped the dict path'sif (v == NULL) break;. It first shipped in v3.15.0b1, so onlymainand3.15are affected (3.13 and 3.14 still have the guard); no released version is exposed. Needs backport to 3.15.