Segfaults when accessing module state in `tp_dealloc` (itertools teedataobject clear)

[em9797b]

Crash report

What happened?

from dataclasses import dataclass
from itertools import tee
from typing import Optional


# if we remove @dataclass, then no segfault
@dataclass
class SomeDataClass:
    pass


class SomeClass:

    # if we remove Optional, then no segfault
    _value: Optional[SomeDataClass]

    def __init__(self, it):
        self._it = it

    def prepare_segfault(self):
        (lhs, _) = tee(self._it)
        # if we don't assign lhs to self._it, then no segfault
        self._it = lhs


# if some_object isn't bound at the top-level scope, then no segfault
some_object = SomeClass(iter("testing"))
some_object.prepare_segfault()

Running the file from the terminal with python3.12 minimal.py is sufficient. When the interpreter exits, it segfaults.
Crash does not occur in python 3.8-3.11, but does occur in 3.12 and 3.13. Crash not observed on Windows with 3.12.

Backtrace:

#0  0x00007fc2c1a72e01 in teedataobject_clear (tdo=tdo@entry=0x7fc2b3ed2040) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Modules/itertoolsmodule.c:836
#1  0x00007fc2c1a72d49 in teedataobject_dealloc (tdo=0x7fc2b3ed2040) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Modules/itertoolsmodule.c:845
#2  0x00007fc2c1b06a0e in Py_DECREF (op=<optimized out>) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Include/object.h:706
#3  tee_clear (to=to@entry=0x7fc2b3c13b00) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Modules/itertoolsmodule.c:1050
#4  0x00007fc2c1b069ac in tee_dealloc (to=0x7fc2b3c13b00) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Modules/itertoolsmodule.c:1059
#5  0x00007fc2c1a390d8 in _Py_Dealloc (op=<optimized out>) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Objects/object.c:2608
#6  Py_DECREF (op=<optimized out>) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Include/object.h:706
#7  Py_XDECREF (op=<optimized out>) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Include/object.h:799
#8  _PyObject_FreeInstanceAttributes (self=0x7fc2b3daf1d0) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Objects/dictobject.c:5571
#9  subtype_dealloc (self=0x7fc2b3daf1d0) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Objects/typeobject.c:2017
#10 0x00007fc2c19f5440 in _Py_Dealloc (op=<optimized out>) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Objects/object.c:2625
#11 Py_DECREF (op=<optimized out>) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Include/object.h:706
#12 Py_XDECREF (op=<optimized out>) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Include/object.h:799
#13 free_keys_object (interp=0x7fc2c1df0d48 <_PyRuntime+76392>, keys=0x7fc2b3d7a100) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Objects/dictobject.c:673
#14 0x00007fc2c1aa72bd in dict_tp_clear (op=<optimized out>) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Objects/dictobject.c:3564
#15 0x00007fc2c1a02cfa in delete_garbage (old=0x7fc2c1df0e00 <_PyRuntime+76576>, collectable=0x7ffcdfc26270, gcstate=0x7fc2c1df0db8 <_PyRuntime+76504>, tstate=0x7fc2c1e4e668 <_PyRuntime+459656>)
    at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Modules/gcmodule.c:1029
#16 gc_collect_main (tstate=0x7fc2c1e4e668 <_PyRuntime+459656>, generation=generation@entry=2, n_collected=n_collected@entry=0x0, n_uncollectable=n_uncollectable@entry=0x0, nofail=nofail@entry=1)
    at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Modules/gcmodule.c:1303
#17 0x00007fc2c1abe201 in _PyGC_CollectNoFail (tstate=<optimized out>) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Modules/gcmodule.c:2135
#18 0x00007fc2c1aaacda in Py_FinalizeEx () at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Python/pylifecycle.c:1889
#19 0x00007fc2c1ab96c9 in Py_RunMain () at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Modules/main.c:711
#20 0x00007fc2c1a74f5c in Py_BytesMain (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/python3.12-3.12.1-2.fc39.x86_64/Modules/main.c:763
#21 0x00007fc2c164614a in __libc_start_call_main (main=main@entry=0x56033de41160 <main>, argc=argc@entry=2, argv=argv@entry=0x7ffcdfc26698) at ../sysdeps/nptl/libc_start_call_main.h:58
#22 0x00007fc2c164620b in __libc_start_main_impl (main=0x56033de41160 <main>, argc=2, argv=0x7ffcdfc26698, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7ffcdfc26688)
    at ../csu/libc-start.c:360
#23 0x000056033de41095 in _start ()

CPython versions tested on:

3.12

Operating systems tested on:

Linux

Output from running 'python -VV' on the command line:

Python 3.12.1 (main, Dec 18 2023, 00:00:00) [GCC 13.2.1 20231205 (Red Hat 13.2.1-6)]

Linked PRs

• gh-115874: Don't use module state in teedataobject tp_dealloc #116204
• [3.12] gh-115874: Don't use module state in teedataobject tp_dealloc (GH-116204) #116955
• gh-115874: Fix segfault in FutureIter_dealloc #117741
• [3.12] GH-115874: Fix segfault in FutureIter_dealloc (GH-117741) #118114

[JelleZijlstra]

Slightly different crash on current main on macOS. It fails an assertion Assertion failed: (et->ht_module), function _PyType_GetModuleState, file pycore_typeobject.h, line 110. with the following stack trace:

(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = hit program assert
    frame #0: 0x00000001962e8724 libsystem_kernel.dylib`__pthread_kill + 8
    frame #1: 0x000000019631fc28 libsystem_pthread.dylib`pthread_kill + 288
    frame #2: 0x000000019622dae8 libsystem_c.dylib`abort + 180
    frame #3: 0x000000019622ce44 libsystem_c.dylib`__assert_rtn + 272
  * frame #4: 0x00000001002a168c python.exe`_PyType_GetModuleState.cold.3 at pycore_typeobject.h:110:5 [opt]
    frame #5: 0x0000000100244230 python.exe`_PyType_GetModuleState(type=0x0000000101886030) at pycore_typeobject.h:110:5 [opt]
    frame #6: 0x00000001002441c8 python.exe`get_module_state_by_cls(cls=<unavailable>) at itertoolsmodule.c:52:19 [opt]
    frame #7: 0x0000000100247138 python.exe`teedataobject_clear(tdo=0x0000000100a43130) at itertoolsmodule.c:840:30 [opt]
    frame #8: 0x0000000100246fa0 python.exe`teedataobject_dealloc(tdo=0x0000000100a43130) at itertoolsmodule.c:850:5 [opt]
    frame #9: 0x00000001000d7fb4 python.exe`_Py_Dealloc(op=0x0000000100a43130) at object.c:2889:5 [opt]
    frame #10: 0x000000010023f2e8 python.exe`Py_DECREF(filename=<unavailable>, lineno=<unavailable>, op=0x0000000100a43130) at object.h:922:9 [opt]
    frame #11: 0x0000000100246bb4 python.exe`tee_clear(to=0x0000000103159850) at itertoolsmodule.c:1055:5 [opt]
    frame #12: 0x0000000100246adc python.exe`tee_dealloc(to=0x0000000103159850) at itertoolsmodule.c:1064:5 [opt]
    frame #13: 0x00000001000d7fb4 python.exe`_Py_Dealloc(op=0x0000000103159850) at object.c:2889:5 [opt]
    frame #14: 0x00000001000bf880 python.exe`Py_DECREF(filename=<unavailable>, lineno=<unavailable>, op=0x0000000103159850) at object.h:922:9 [opt]
    frame #15: 0x00000001000c44a8 python.exe`Py_XDECREF(op=<unavailable>) at object.h:1030:9 [opt]
    frame #16: 0x00000001000c445c python.exe`_PyObject_FreeInstanceAttributes(self=<unavailable>) at dictobject.c:6830:9 [opt]
    frame #17: 0x00000001000fc204 python.exe`subtype_dealloc(self=0x000000010314fee0) at typeobject.c:2151:13 [opt]
    frame #18: 0x00000001000d7fb4 python.exe`_Py_Dealloc(op=0x000000010314fee0) at object.c:2889:5 [opt]
    frame #19: 0x00000001000bf880 python.exe`Py_DECREF(filename=<unavailable>, lineno=<unavailable>, op=0x000000010314fee0) at object.h:922:9 [opt]
    frame #20: 0x00000001000c44a8 python.exe`Py_XDECREF(op=<unavailable>) at object.h:1030:9 [opt]
    frame #21: 0x00000001000c4954 python.exe`dictkeys_decref(interp=<unavailable>, dk=0x0000000100fc12d0, use_qsbr=false) at dictobject.c:459:17 [opt]
    frame #22: 0x00000001000c0830 python.exe`clear_lock_held(op=0x0000000100c72ed0) at dictobject.c:0 [opt]
    frame #23: 0x00000001000c0748 python.exe`PyDict_Clear(op=<unavailable>) at dictobject.c:2692:5 [opt]
    frame #24: 0x00000001000c288c python.exe`dict_tp_clear(op=<unavailable>) at dictobject.c:4438:5 [opt]
    frame #25: 0x00000001001b18e8 python.exe`delete_garbage(tstate=0x000000010048fe60, gcstate=0x000000010046ab08, collectable=0x000000016fdff258, old=0x000000010046ab50) at gc.c:1016:24 [opt]
    frame #26: 0x00000001001afc5c python.exe`gc_collect_main(tstate=0x000000010048fe60, generation=2, reason=_Py_GC_REASON_SHUTDOWN) at gc.c:1421:5 [opt]
    frame #27: 0x00000001001afe28 python.exe`_PyGC_CollectNoFail(tstate=<unavailable>) at gc.c:1655:12 [opt]
    frame #28: 0x00000001001db9e8 python.exe`finalize_modules(tstate=0x000000010048fe60) at pylifecycle.c:1711:5 [opt]
    frame #29: 0x00000001001db694 python.exe`Py_FinalizeEx at pylifecycle.c:1973:5 [opt]
    frame #30: 0x0000000100208334 python.exe`Py_RunMain at main.c:709:9 [opt]
    frame #31: 0x0000000100208638 python.exe`pymain_main(args=0x000000016fdff3e0) at main.c:737:12 [opt]
    frame #32: 0x000000010020867c python.exe`Py_BytesMain(argc=<unavailable>, argv=<unavailable>) at main.c:761:12 [opt]
    frame #33: 0x0000000100001668 python.exe`main(argc=<unavailable>, argv=<unavailable>) at python.c:15:12 [opt]
    frame #34: 0x0000000195fc7f28 dyld`start + 2236

The crash happens during finalization. Apparently the module has already been finalized but the tee type still exists.

cc @erlend-aasland who worked on itertools module state (e.g., #101477).

[JelleZijlstra]

The crash goes away if I remove both of the copyreg.pickle calls here:

cpython/Lib/typing.py

Line 1797 in 200271c

copyreg.pickle(ParamSpecArgs, _pickle_psargs)

[erlend-aasland]

Apparently the module has already been finalized but the tee type still exists.

That sounds really strange. The (heap) type keeps a strong reference to the module.

[brandtbucher]

Tiny reproducer:

import typing, copyreg, itertools
copyreg.buggy_tee = itertools.tee(())
# Crash!

Here's essentially the same thing, but also clearing every reference I can think of.

import sys, gc, typing, copyreg, itertools

# Clear typing:
[f() for f in typing._cleanups]
typing.__dict__.clear()
del typing

# Clear copyreg...
copyreg.dispatch_table.clear()
copyreg._extension_registry.clear()
copyreg._inverted_registry.clear()
copyreg._extension_cache.clear()
copyreg.__dict__.clear()

# ...*except* for a cycle with itself and a reference to a tee:
copyreg.buggy_cycle = copyreg
copyreg.buggy_tee = itertools.tee(())[0]

# Clear itertools:
itertools.__dict__.clear()

# Clear all modules except __main__:
for module in list(sys.modules):
    if module != "__main__":
        del sys.modules[module]

# The only remaining non-cyclic references to copyreg and itertools are in this
# module's globals. copyreg is referenced by __main__ and itself:
assert gc.get_referrers(copyreg) == [globals(), copyreg.__dict__]
assert sys.getrefcount(copyreg) - 1 == len(gc.get_referrers(copyreg))

# itertools is referenced by __main__ and all of the itertools types:
assert gc.get_referrers(itertools)[0] == globals()
assert all(isinstance(t, type) and t.__module__ == "itertools" for t in gc.get_referrers(itertools)[1:])
assert sys.getrefcount(itertools) - 1 == len(gc.get_referrers(itertools))

# Crash!

[brandtbucher]

The (heap) type keeps a strong reference to the module.

One thing I'm curious about (maybe others with GC or multi-phase init expertise can shed some light): how is the "type -> module -> type" cycle handled for these types?

More concretely, it seems like there could be a few cycles here at shutdown:

• tee type -> itertools module -> tee type
• teedataobject type -> itertools module -> teedataobject type
• copyreg module -> copyreg module dict -> copyreg module
  • this cycle keeps a tee (and, in turn, a teedataobject) alive

Faced with these three cycles, it seems like it's only "correct" to clear the third, before moving on of the other two. But there's nothing stopping the GC from "picking" one of the other two to clear instead, which leads to this nasty situation. So it seems like relying on module state in tp_clear, as the teedataobject type does, is incorrect.

Am I missing something?

[brandtbucher]

Confirming my hunch, the following patch fixes this for me:

diff --git a/Modules/itertoolsmodule.c b/Modules/itertoolsmodule.c
index 164741495c..ab0f440bee 100644
--- a/Modules/itertoolsmodule.c
+++ b/Modules/itertoolsmodule.c
@@ -837,8 +837,7 @@ teedataobject_clear(teedataobject *tdo)
         Py_CLEAR(tdo->values[i]);
     tmp = tdo->nextlink;
     tdo->nextlink = NULL;
-    itertools_state *state = get_module_state_by_cls(Py_TYPE(tdo));
-    teedataobject_safe_decref(tmp, state->teedataobject_type);
+    teedataobject_safe_decref(tmp, Py_TYPE(tdo));
     return 0;
 }

I also think I found an unrelated refleak while chasing this down:

diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index fe3b7b87c8..181d032328 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -6549,6 +6549,7 @@ reduce_newobj(PyObject *obj)
     }
     else {
         /* args == NULL */
+        Py_DECREF(copyreg);
         Py_DECREF(kwargs);
         PyErr_BadInternalCall();
         return NULL;

[brandtbucher]

I can open a PR for this tomorrow if others confirm that my understanding is correct.

[em9797b]

I don't know enough to comment on the GC, but I can confirm that, for me:

• the tiny reproducer crashes the same way the original sample does in Fedora 39's 3.12, repo 3.12, and main
• applying the patches to either repo 3.12 or main eliminates the crash

[cjwatson]

I ran into this while trying to track down https://bugs.debian.org/1063345 (a test failure in Debian's celery package) and confirmed that applying @brandtbucher's first patch from #115874 (comment) to Debian's python3.12 package is enough to fix the crash. Thanks!

[brandtbucher]

I scanned through the other modules. FutureIter_dealloc has a similar issue, where the module state is used to add the current futureiterobject to a free list. That one seems less easy to fix:

https://github.com/python/cpython/blob/main/Modules/_asynciomodule.c#L1600-L1617

[brandtbucher]

Here's a tiny reproducer for the FutureIter case:

import sys, copyreg, _asyncio
copyreg.buggy_cycle = copyreg
copyreg.buggy_futureiter = iter(_asyncio.Future())
del sys.modules["copyreg"]
# Crash!

% ./python.exe bug.py
/Users/brandtbucher/cpython/bug.py:3: DeprecationWarning: There is no current event loop
  copyreg.buggy_futureiter = iter(_asyncio.Future())
Assertion failed: (mro != NULL), function PyType_GetModuleByDef, file typeobject.c, line 4770.
zsh: abort      ./python.exe bug.py

Commenting out the state stuff in FutureIter_dealloc fixes the crash.