gh-157176: Fix GC tracking in PyStructSequence_New - #157179
Conversation
|
@sergey-miryanov, could you please review this PR? |
| for (i = 0; i < size; i++) | ||
| obj->ob_item[i] = NULL; | ||
|
|
||
| _PyObject_GC_TRACK(obj); |
There was a problem hiding this comment.
This looks safe, but we usually try to avoid exposing partially constructed objects to the GC. I'm not sure I get why this change fixes your repro. Could you explain a bit?
There was a problem hiding this comment.
-
Why it fixes the repro:
time.gmtime()is not tracked by the GC. Because of that, the GC cannot see the cycletype(t) -> t -> type(t), so the entire heap type leaks at shutdown. Trackingtlets GC traverset -> Py_TYPE(t)and collect the cycle. -
Partially constructed object: All items are pre-initialized to
NULL. Bothstructseq_traverse(Py_VISIT) andstructseq_dealloc(Py_XDECREF) safely ignoreNULL. This followsPyTuple_New(), which also tracks the object withNULLitems since C API callers have no separate "finish" step.
There was a problem hiding this comment.
So, this change affects all call sites where PyStructSequence_New is called without a subsequent call to _PyObject_GC_TRACK. But at first glance, many of them don't seem to need tracking for PyStructSequence_New instances.
@vstinner Do we need frozen version of PyStructSequence?
structseqtype and its instance #157176