Skip to content

Commit 780c497

Browse files
committed
update_refs(): assert that incoming refcounts aren't 0. The comment
for this function has always claimed that was true, but it wasn't verified before. For the latest batch of "double deallocation" bugs (stemming from weakref callbacks invoked by way of subtype_dealloc), this assert would have triggered (instead of waiting for _Py_ForgetReference to die with a segfault later).
1 parent 0bd743c commit 780c497

File tree

1 file changed

+19
-0
lines changed

1 file changed

+19
-0
lines changed

Modules/gcmodule.c

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,25 @@ update_refs(PyGC_Head *containers)
214214
for (; gc != containers; gc = gc->gc.gc_next) {
215215
assert(gc->gc.gc_refs == GC_REACHABLE);
216216
gc->gc.gc_refs = FROM_GC(gc)->ob_refcnt;
217+
/* Python's cyclic gc should never see an incoming refcount
218+
* of 0: if something decref'ed to 0, it should have been
219+
* deallocated immediately at that time.
220+
* Possible cause (if the assert triggers): a tp_dealloc
221+
* routine left a gc-aware object tracked during its teardown
222+
* phase, and did something-- or allowed something to happen --
223+
* that called back into Python. gc can trigger then, and may
224+
* see the still-tracked dying object. Before this assert
225+
* was added, such mistakes went on to allow gc to try to
226+
* delete the object again. In a debug build, that caused
227+
* a mysterious segfault, when _Py_ForgetReference tried
228+
* to remove the object from the doubly-linked list of all
229+
* objects a second time. In a release build, an actual
230+
* double deallocation occurred, which leads to corruption
231+
* of the allocator's internal bookkeeping pointers. That's
232+
* so serious that maybe this should be a release-build
233+
* check instead of an assert?
234+
*/
235+
assert(gc->gc.gc_refs != 0);
217236
}
218237
}
219238

0 commit comments

Comments
 (0)