Skip to content

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

Closed
Closed
Segfaults when accessing module state in tp_dealloc (itertools teedataobject clear)#115874
Assignees
brandtbucher
Labels
3.12only security fixes3.13only security fixestype-crashA hard crash of the interpreter, possibly with a core dump

Description

@em9797b
em9797b
opened on Feb 24, 2024

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

Activity

  1. em9797b
    added
    type-crashA hard crash of the interpreter, possibly with a core dump
    on Feb 24, 2024

  2. JelleZijlstra commented on Feb 24, 2024

    @JelleZijlstra
    JelleZijlstra
    on Feb 24, 2024
    Member

    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).

  3. JelleZijlstra
    added
    3.12only security fixes
    3.13only security fixes
    on Feb 24, 2024

  4. JelleZijlstra commented on Feb 24, 2024

    @JelleZijlstra
    JelleZijlstra
    on Feb 24, 2024
    Member

    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)

  5. erlend-aasland commented on Feb 24, 2024

    @erlend-aasland
    erlend-aasland
    on Feb 24, 2024
    Contributor

    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.

  6. brandtbucher commented on Feb 27, 2024

    @brandtbucher
    brandtbucher
    on Feb 27, 2024
    Member

    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!

  7. brandtbucher commented on Feb 27, 2024

    @brandtbucher
    brandtbucher
    on Feb 27, 2024
    Member

    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?

  8. brandtbucher commented on Feb 27, 2024

    @brandtbucher
    brandtbucher
    on Feb 27, 2024
    Member

    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;

  9. brandtbucher commented on Feb 27, 2024

    @brandtbucher
    brandtbucher
    on Feb 27, 2024
    Member

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

  10. brandtbucher
    self-assigned this
    on Feb 27, 2024

  11. em9797b commented on Feb 27, 2024

    @em9797b
    em9797b
    on Feb 27, 2024
    Author

    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

  12. cjwatson commented on Feb 27, 2024

    @cjwatson
    cjwatson
    on Feb 27, 2024
    Contributor

    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!

  13. brandtbucher commented on Feb 27, 2024

    @brandtbucher
    brandtbucher
    on Feb 27, 2024
    Member

    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

  14. brandtbucher commented on Feb 27, 2024

    @brandtbucher
    brandtbucher
    on Feb 27, 2024
    Member

    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.

  15. 10 remaining items

  16. vstinner
    changed the title [-]Segfaults when accessing module state in `tp_dealloc`[/-] [+]Segfaults when accessing module state in `tp_dealloc` (itertools teedataobject clear)[/+] on Mar 27, 2024

  17. vstinner commented on Mar 27, 2024

    @vstinner
    vstinner
    on Mar 27, 2024
    Member

    I found a very similar bug in the tmt project: teemtee/tmt#2501 (comment). It's good to know that it has already been fixed :-)

    tmt crash: the Python crash occurs in 3 steps:

    1. itertoolsmodule_clear() is called: itertools_state is cleared, it stores strong references to itertools types
    2. type_clear() is called on itertools._tee_dataobject type => set ht_module to NULL.
    3. teedataobject_clear() is called on an itertools._tee_dataobject instance => assertion fails since ht_module is NULL.

  18. erlend-aasland commented on Mar 27, 2024

    @erlend-aasland
    erlend-aasland
    on Mar 27, 2024
    Contributor

    I have somebody who’s interested in working on the FutureIter fix.

    Do you think it is possible to get the fix merged in time for the next 3.12 release?

  19. savannahostrowski commented on Apr 8, 2024

    @savannahostrowski
    savannahostrowski
    on Apr 8, 2024
    Member

    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.

    AFACT, this no longer repros on main but I'm not aware of any fixes that landed addressing the segfault.

  20. erlend-aasland commented on Apr 9, 2024

    @erlend-aasland
    erlend-aasland
    on Apr 9, 2024
    Contributor

    AFACT, this no longer repros on main but I'm not aware of any fixes that landed addressing the segfault.

    I bisected it down to commit 8bef34f, which modified the garbage collector. Also, 3.12 still crashes, so I still think the bug should be fixed both in main and 3.12.

  21. diegorusso
    added a commit that references this issue on Apr 17, 2024

    pythongh-115874: Don't use module state in teedataobject tp_dealloc (p…

    231a673
  22. brandtbucher
    added a commit that references this issue on Apr 19, 2024

    GH-115874: Fix segfault in FutureIter_dealloc (GH-117741)

    d8f3503
  23. brandtbucher
    closed this as completedon Apr 19, 2024
  24. miss-islington
    added a commit that references this issue on Apr 19, 2024

    pythonGH-115874: Fix segfault in FutureIter_dealloc (pythonGH-117741)

    2f6fc1a
  25. brandtbucher
    added a commit that references this issue on Apr 19, 2024

    [3.12] GH-115874: Fix segfault in FutureIter_dealloc (GH-118114)

    23192ab

  26. vstinner commented on Apr 29, 2024

    @vstinner
    vstinner
    on Apr 29, 2024
    Member

    I still think the bug should be fixed both in main and 3.12.

    I suggest to open a new issue since this one is closed.

  27. erlend-aasland commented on Apr 29, 2024

    @erlend-aasland
    erlend-aasland
    on Apr 29, 2024
    Contributor

    I suggest to open a new issue since this one is closed.

    It was fixed by #117741 and #118114. No need to open a new issue.

  28. FakeShell
    added a commit that references this issue on Aug 1, 2024

    Don't rely on module state in teedataobject_clear (from Brandt Bucher…

    c2e29e5

  29. vstinner commented on Aug 6, 2024

    @vstinner
    vstinner
    on Aug 6, 2024
    Member

    See also the similar issue gh-122695.

  30. mhy-pexip
    added a commit that references this issue on Apr 10, 2025

    Import Debian changes 3.13.2-3

    aada8d6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

3.12only security fixes3.13only security fixestype-crashA hard crash of the interpreter, possibly with a core dump

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions