gh-152107: Fix crash when creating a dict item iterator under MemoryError#152110
gh-152107: Fix crash when creating a dict item iterator under MemoryError#152110zangjiucheng wants to merge 3 commits into
Conversation
| @support.cpython_only | ||
| def test_item_iterator_no_memory(self): | ||
| # gh-152107: dictiter_new() for a dict item-iterator allocates | ||
| # di_result via _PyTuple_FromPairSteal() before _PyObject_GC_TRACK(). |
There was a problem hiding this comment.
I would suggest to remove all the technical details from the comments, they will change at some point.
There was a problem hiding this comment.
Thank you. yep. I wasn't sure earlier whether I should keep the behaviour explanation.
Will address in next commit
| dictiterobject *di = (dictiterobject *)self; | ||
| /* bpo-31095: UnTrack is needed before calling any callbacks */ | ||
| _PyObject_GC_UNTRACK(di); | ||
| /* bpo-31095: UnTrack is needed before calling any callbacks. |
There was a problem hiding this comment.
Please, do not change the existing comment, add a new one.
| for start in range(60): | ||
| set_nomemory(start) | ||
| try: | ||
| _strptime._strptime("", "") |
There was a problem hiding this comment.
I am not sure that this test is correct: _strptime can switch from dict api to something else, test will pass, but it won't check the same problem.
There was a problem hiding this comment.
Good catch. I will rewrite the test to drive the failing path directly via iter(d.items()) instead of relying on _strptime's internals, which prevent _strtime implement changes.
| import _strptime | ||
| from _testcapi import set_nomemory | ||
| for start in range(60): | ||
| set_nomemory(start) |
There was a problem hiding this comment.
shouldn't we clear the memory hooks here on each iteration?
There was a problem hiding this comment.
I will add remove_mem_hooks() in a finally on every iteration, and move the whole thing into a subprocess via assert_python_ok so the OOM injection can't leak into the rest of the suite.
|
Since having |
That's a good point. The fix is identical, but we can make it early to ensure the |
|
Hello everyone, thank you for your suggestions!! I’ve addressed all the issues based on your feedback.
|
Fix: guard the untrack with
_PyObject_GC_IS_TRACKED()so a never-trackediterator is freed cleanly.
Reproducer (debug build,
_strptimedrains the size-2 tuple freelist so thedi_resultallocation actually fails):Found via OOM-injection fuzzing (OOM-0006 in #151763).
🤖 Generated with Claude Code
dictitem-iterator:_PyObject_GC_UNTRACKon never-tracked iterator under OOM (dictobject.c) #152107