Skip to content

Commit 72986b8

Browse files
committed
gh-123241: Make gc_free_threading.c more robust to increfs/decrefs in tp_traverse
The GC for the free threading build reuses `ob_ref_local` when determining if any objects in the unreachable set were resurrected. This is generally okay because the refcount fields are already merged. However, if the tp_traverse calls `Py_INCREF`/`Py_DECREF`, it may see an `ob_ref_local` of `UINT32_MAX` that looks like an immortal object. Refactor `handle_resurrected_objects` so that `ob_ref_local` doesn't reach `UINT32_MAX`.
1 parent b3bf212 commit 72986b8

2 files changed

Lines changed: 34 additions & 11 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Make the GC for the :term:`free threading` build more robust to
2+
:c:member:`~PyTypeObject.tp_traverse` implementations that call
3+
:c:func:`Py_INCCREF` and :c:func:`Py_DECREF`.

Python/gc_free_threading.c

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1788,12 +1788,36 @@ show_stats_each_generations(GCState *gcstate)
17881788
// TODO
17891789
}
17901790

1791+
1792+
// Initialize the `ob_ref_local` field to the refcount if it's not already
1793+
// initialized. This is for detecting resurrected objects in the unreachable
1794+
// set. Returns true if initialization was performed and false if the refcount
1795+
// doesn't fit in `ob_ref_local`.
1796+
static bool
1797+
gc_maybe_init_unreachable_refs(PyObject *op)
1798+
{
1799+
Py_ssize_t refcount = (op->ob_ref_shared >> _Py_REF_SHARED_SHIFT);
1800+
assert(refcount >= 1);
1801+
if (refcount > INT32_MAX) {
1802+
// The refcount is too big to fit in `ob_ref_local`
1803+
return false;
1804+
}
1805+
1806+
if (op->ob_ref_local == 0) {
1807+
op->ob_ref_local = (uint32_t)refcount;
1808+
}
1809+
return true;
1810+
}
1811+
17911812
// Traversal callback for handle_resurrected_objects.
17921813
static int
17931814
visit_decref_unreachable(PyObject *op, void *data)
17941815
{
17951816
if (gc_is_unreachable(op) && _PyObject_GC_IS_TRACKED(op)) {
1796-
op->ob_ref_local -= 1;
1817+
if (gc_maybe_init_unreachable_refs(op)) {
1818+
assert(op->ob_ref_local > 1);
1819+
op->ob_ref_local -= 1;
1820+
}
17971821
}
17981822
return 0;
17991823
}
@@ -1846,8 +1870,7 @@ handle_resurrected_objects(struct collection_state *state)
18461870
continue;
18471871
}
18481872

1849-
Py_ssize_t refcount = (op->ob_ref_shared >> _Py_REF_SHARED_SHIFT);
1850-
if (refcount > INT32_MAX) {
1873+
if (!gc_maybe_init_unreachable_refs(op)) {
18511874
// The refcount is too big to fit in `ob_ref_local`. Mark the
18521875
// object as immortal and bail out.
18531876
gc_clear_unreachable(op);
@@ -1856,24 +1879,21 @@ handle_resurrected_objects(struct collection_state *state)
18561879
continue;
18571880
}
18581881

1859-
op->ob_ref_local += (uint32_t)refcount;
1860-
1861-
// Subtract one to account for the reference from the worklist.
1862-
op->ob_ref_local -= 1;
1863-
18641882
traverseproc traverse = Py_TYPE(op)->tp_traverse;
18651883
(void)traverse(op, visit_decref_unreachable, NULL);
18661884
}
18671885

18681886
// Find resurrected objects
18691887
bool any_resurrected = false;
18701888
WORKSTACK_FOR_EACH(&state->unreachable, op) {
1871-
int32_t gc_refs = (int32_t)op->ob_ref_local;
1889+
uint32_t gc_refs = op->ob_ref_local;
18721890
op->ob_ref_local = 0; // restore ob_ref_local
18731891

1874-
_PyObject_ASSERT(op, gc_refs >= 0);
1892+
// Subtract one for the worklist reference
1893+
_PyObject_ASSERT(op, gc_refs > 0);
1894+
gc_refs -= 1;
18751895

1876-
if (gc_is_unreachable(op) && gc_refs > 0) {
1896+
if (gc_is_unreachable(op) && gc_refs != 0) {
18771897
// Clear the unreachable flag on any transitively reachable objects
18781898
// from this one.
18791899
any_resurrected = true;

0 commit comments

Comments
 (0)