Skip to content

Four bugs from allocation-failure fuzzing (get_handle, reduce, __setitem__, concurrent resize) #149

Description

@devdanzin

I've run fusil in allocation-failure mode on cereggii and four bugs surfaced. Full write-ups (root cause + inline reproducer + proposed fix for each) are in this gist:

https://gist.github.com/devdanzin/10536845b9ec874be0b9471258879b55

All reproduce on a free-threaded CPython 3.14 debug + ASan build (Py_GIL_DISABLED), on current main; run the reproducers with PYTHON_GIL=0. One is a reference-counting bug, one is a cyclic-GC accounting bug, and two are NULL-dereferences.

As discussed, here are PRs for review/merge. Each fix was built and verified on a debug free-threaded ASan build (reproducer flips from crash to clean, full test suite green), and each PR adds a regression test that crashes/fails on the unfixed build and passes with the fix. I also grepped for the same mistake elsewhere in the tree; the audit notes are in each PR.

# Bug PR Status
1 AtomicDict double-DECREFs a key when a colliding key's __eq__ raises #145 ready
3 AtomicDict.reduce/reduce_count NULL-deref when a key's __eq__ raises #146 ready
4 get_handle() dereferences an unchecked allocation under memory pressure (3 sites) #147 ready
2 Cyclic GC double-counts keys in pages shared across a resize (surfaces as the gist's "deferred-refcount key" abort) #148 ready

On #2 (the one you wanted to think about): the gist framed it as a deferred-refcount corruption, but after pinning it under rr I found the reference count is actually correct — the bug is a cyclic-GC double-count of shared pages. On a resize, meta_copy_pages shares page objects between the old and new meta, and a stale per-accessor storage->meta keeps the old meta alive. AtomicDictMeta_traverse walks its pages inline (AtomicDictPage_traverse(...)), so a shared page's keys are reported to the GC once per meta; the collector then subtracts more internal references than the object's refcount, drives gc_get_refs negative, and aborts (validate_gc_objects: "refcount is too small"). It needs enough concurrent inserters to force a resize (8 threads reproduce, 2 don't), and it's fatal with deferred-refcount keys because their gc_refs seed (Py_REFCNT − _Py_REF_DEFERRED) is small. rr made this concrete: two different meta objects traversing the same page object.

The fix makes AtomicDictPage a first-class GC node (Py_TPFLAGS_HAVE_GC + tp_traverse, PyObject_GC_New/Track/UnTrack) and has the meta Py_VISIT its pages instead of recursing, so a shared page is traversed once regardless of how many metas reference it — 11 lines, no reference-counting changes, no vendored internals. The gist's proposed _Py_SetWeakrefAndIncref guard is therefore not needed. This supersedes it; I've dropped that draft.

Separately, while running the suite I noticed test_racy_deletes crashes intermittently (~1/20) on pristine main as well — a lock-free-read use-after-free in the concurrent delete+insert path (SEGV reading entry.key in expected_update_entry during a compare). It's unrelated to these four fixes; flagging it in case it isn't already on your radar.

Investigation, fixes and issue draft by Claude Code (Opus 4.8).

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions