GH-155728: Untrack tuples while unmarshalling - #156423
Conversation
| } | ||
| PyTuple_SET_ITEM(v, i, v2); | ||
| } | ||
| _PyTuple_MaybeUntrack(v); |
There was a problem hiding this comment.
Can we allocate this untracked, then conditionally track it after construction. That way we're not exposing partially constructed tuples to the GC.
Also, it would be a bit more efficient to record whether tracking is needed in the loop, so we don't need to loop over the tuple again
There was a problem hiding this comment.
Ok, got it.
sergey-miryanov
added
needs backport to 3.14
|
I'm wondering if it makes sense to reuse this set of payloads for testing cpython/Lib/test/test_marshal.py Line 411 in 9036982 My hunch is to check for recursion, self-referentials etc. |
| break; | ||
| } | ||
| PyTuple_SET_ITEM(v, i, v2); | ||
| if (!track_tuple && PyObject_GC_IsTracked(v2)) { |
There was a problem hiding this comment.
Why not _PyObject_GC_MAY_BE_TRACKED, as in _PyObject_GC_TRACK?
Is there a significant difference between these functions?
There was a problem hiding this comment.
PyObject_GC_IsTracked wider than _PyObject_GC_MAY_BE_TRACKED, I'm not sure we should narrow our checks only to tuples as _PyObject_GC_MAY_BE_TRACKED do.
| self._not_tracked_instantly((1, 2) * 5) | ||
|
|
||
| self._not_tracked(((1, x), y, (2, 3))) | ||
| self._not_tracked((1, 2, (True, False, ()))) |
There was a problem hiding this comment.
Maybe, something like this?
t = (True, False, ())
self._not_tracked((1, 2, t))
Because for now self._not_tracked_instantly((1, 2, (True, False, ()))) is also correct.
| @@ -1,3 +1,5 @@ | |||
| import gc | |||
There was a problem hiding this comment.
Maybe we should move this import below?
4 participants
Uh oh!
There was an error while loading. Please reload this page.