Assertion failure when func_repr is called on an already tp_clear-ed object

[hvenev]

Crash report

It is possible to resurrect a tp_clear-ed object using pure python. The following piece of code illustrates how:

import gc, weakref


class LateFin:
    __slots__ = ('ref',)

    def __del__(self):
        # 8. Now `latefin`'s finalizer is called. Here we obtain a reference to
        # `func`, which is currently undergoing `tp_clear`.
        global func
        func = self.ref()


class Cyclic(tuple):
    __slots__ = ()

    # 4. The finalizers of all garbage objects are called. In this case this is
    # only us as `func` doesn't have a finalizer.
    def __del__(self):
        # 5. Create a weakref to `func` now. If we had created it earlier, it
        # would have been cleared by the garbage collector before calling the
        # finalizers.
        self[1].ref = weakref.ref(self[0])
        # 6. Drop the global reference to `latefin`. The only remaining
        # reference is the one we have.
        global latefin
        del latefin
    # 7. Now `func` is `tp_clear`-ed. This drops the last reference to `Cyclic`,
    # which gets `tp_dealloc`-ed. This drops the last reference to `latefin`.


latefin = LateFin()
func = lambda: None
cyc = tuple.__new__(Cyclic, (func, latefin))

# 1. Create a reference cycle of `cyc` and `func`.
func.__module__ = cyc

# 2. Make the cycle unreachable, but keep the global reference to `latefin` so
# that it isn't detected as garbage. This way its finalizer will not be called
# immediately.
del func, cyc

# 3. Invoke garbage collection, which will find `cyc` and `func` as garbage.
gc.set_debug(gc.DEBUG_COLLECTABLE)
gc.collect()
gc.set_debug(0)

# 9. Call `repr()`, which will try to use the NULL `func_qualname`.
repr(func)

#5  0x00007ffff7d0a596 in __GI___assert_fail (assertion=assertion@entry=0x697e93 "obj && _PyUnicode_CHECK(obj)", file=file@entry=0x696a92 "Objects/unicodeobject.c", line=line@entry=3000, 
    function=function@entry=0x69e110 <__PRETTY_FUNCTION__.299> "unicode_fromformat_arg") at assert.c:101
#6  0x0000000000538d8e in unicode_fromformat_arg (writer=writer@entry=0x7fffffffd9f0, f=0x682bf1 "U at %p>", f@entry=0x682bf0 "%U at %p>", vargs=vargs@entry=0x7fffffffda28)
    at Objects/unicodeobject.c:3000
#7  0x0000000000539098 in PyUnicode_FromFormatV (format=<optimized out>, vargs=vargs@entry=0x7fffffffda68) at Objects/unicodeobject.c:3109
#8  0x00000000005391ab in PyUnicode_FromFormat (format=format@entry=0x682be6 "<function %U at %p>") at Objects/unicodeobject.c:3161
#9  0x00000000004c2c7b in func_repr (op=<optimized out>) at Objects/funcobject.c:710
#10 0x00000000004ea437 in PyObject_Repr (v=0x91d650) at Objects/object.c:434
#11 0x0000000000571932 in builtin_repr (module=<optimized out>, obj=<optimized out>) at Python/bltinmodule.c:2252
#12 0x00000000004e6c82 in cfunction_vectorcall_O (func=0x9a10f0, args=<optimized out>, nargsf=<optimized out>, kwnames=<optimized out>) at Objects/methodobject.c:514
#13 0x00000000004a5700 in _PyObject_VectorcallTstate (tstate=0x915430 <_PyRuntime+166000>, callable=callable@entry=0x9a10f0, args=args@entry=0x9a4f98, nargsf=9223372036854775809, 
    kwnames=kwnames@entry=0x0) at ./Include/internal/pycore_call.h:92
#14 0x00000000004a57cb in PyObject_Vectorcall (callable=callable@entry=0x9a10f0, args=args@entry=0x9a4f98, nargsf=<optimized out>, kwnames=kwnames@entry=0x0) at Objects/call.c:299
#15 0x0000000000586b4b in _PyEval_EvalFrameDefault (tstate=0x915430 <_PyRuntime+166000>, frame=0x9a4f40, throwflag=<optimized out>) at Python/ceval.c:4765
#16 0x000000000058af91 in _PyEval_EvalFrame (tstate=tstate@entry=0x915430 <_PyRuntime+166000>, frame=frame@entry=0x9a4f40, throwflag=throwflag@entry=0)
    at ./Include/internal/pycore_ceval.h:66
#17 0x000000000058b09c in _PyEval_Vector (tstate=tstate@entry=0x915430 <_PyRuntime+166000>, func=func@entry=0xa41dd0, locals=locals@entry=0xa5ca80, args=args@entry=0x0, 
    argcount=argcount@entry=0, kwnames=kwnames@entry=0x0) at Python/ceval.c:6387
#18 0x000000000058b190 in PyEval_EvalCode (co=co@entry=0xaa0e70, globals=globals@entry=0xa5ca80, locals=locals@entry=0xa5ca80) at Python/ceval.c:1157
#19 0x00000000005cb444 in run_eval_code_obj (tstate=tstate@entry=0x915430 <_PyRuntime+166000>, co=co@entry=0xaa0e70, globals=globals@entry=0xa5ca80, locals=locals@entry=0xa5ca80)
    at Python/pythonrun.c:1713
#20 0x00000000005cb4fb in run_mod (mod=mod@entry=0xaafda0, filename=filename@entry=0xa8dad0, globals=globals@entry=0xa5ca80, locals=locals@entry=0xa5ca80, flags=flags@entry=0x7fffffffdf08, 
    arena=arena@entry=0xa8dc20) at Python/pythonrun.c:1734
#21 0x00000000005cb5bf in pyrun_file (fp=fp@entry=0x9c8d20, filename=filename@entry=0xa8dad0, start=start@entry=257, globals=globals@entry=0xa5ca80, locals=locals@entry=0xa5ca80, 
    closeit=closeit@entry=1, flags=0x7fffffffdf08) at Python/pythonrun.c:1629
#22 0x00000000005cded0 in _PyRun_SimpleFileObject (fp=fp@entry=0x9c8d20, filename=filename@entry=0xa8dad0, closeit=closeit@entry=1, flags=flags@entry=0x7fffffffdf08)
    at Python/pythonrun.c:439
#23 0x00000000005ce078 in _PyRun_AnyFileObject (fp=fp@entry=0x9c8d20, filename=filename@entry=0xa8dad0, closeit=closeit@entry=1, flags=flags@entry=0x7fffffffdf08) at Python/pythonrun.c:78
#24 0x00000000005e9d4b in pymain_run_file_obj (program_name=program_name@entry=0xa02210, filename=filename@entry=0xa8dad0, skip_source_first_line=0) at Modules/main.c:353
#25 0x00000000005e9e49 in pymain_run_file (config=config@entry=0x8fb490 <_PyRuntime+59600>) at Modules/main.c:372
#26 0x00000000005ea4e8 in pymain_run_python (exitcode=exitcode@entry=0x7fffffffe05c) at Modules/main.c:592
#27 0x00000000005ea735 in Py_RunMain () at Modules/main.c:671
#28 0x00000000005ea78a in pymain_main (args=args@entry=0x7fffffffe0a0) at Modules/main.c:701
#29 0x00000000005ea80f in Py_BytesMain (argc=<optimized out>, argv=<optimized out>) at Modules/main.c:725
#30 0x000000000041d70f in main (argc=<optimized out>, argv=<optimized out>) at ./Programs/python.c:15

Your environment

• CPython versions tested on: 3.10.4, 3.11.0a7+
• Operating system and architecture: x86_64

Linked PRs

• GH-91636: Clear weakrefs created by finalizers. #136401
• [3.14] GH-91636: Clear weakrefs created by finalizers. (GH-136401) #141993
• [3.14] Revert "GH-91636: Clear weakrefs created by finalizers. (GH-136401) (#141993)" #142152
• [3.13] GH-91636: Clear weakrefs created by finalizers. (GH-136401) #144214
• [3.14] GH-91636: Clear weakrefs created by finalizers. (GH-136401) #144444

[sweeneyde]

@pitrou and @pablogsal are listed at https://devguide.python.org/experts/ under "gc".

[sweeneyde]

Bisected to here @methane:

3c45240 is the first bad commit
commit 3c45240
Author: INADA Naoki methane@users.noreply.github.com
Date: Wed Jul 4 11:15:50 2018 +0900

bpo-33418: Add tp_clear for function object (GH-8058)

Without tp_clear, GC can't break cyclic reference.
It will cause memory leak when cyclic reference is
created intentionally.

[sweeneyde]

I opened #91651 to attempt to fix at least this one specific case.

[methane]

Hm. this is wider problem. I think many more types are not safe after tp_clear...

[pablogsal]

This may be quite a widespread problem given how often tp_clear is used as a proxy to tp_dealloc :(

[methane]

Yes. This would be widespread problem. I quickly find two issues:

• module_getattro accesses m->md_dict while it would be cleared.
• cm_repr accesses cm->cm_callable.

[pitrou]

It's probably worthwhile fixing those tp_clear methods to not break invariants, then. Perhaps at least set the cleared fields to None so that we don't get crashes when another method such as tp_repr is invoked?

[pablogsal]

set the cleared fields to None

Then we need to also modify tp_dealloc to also drop the reference to None

[nascheme]

I think changing tp_clear methods so that objects are usable after calling it is the not correct approach.

The issue with the code above is that the weakref created in the __del__ method got excluded from weakrefs being cleared before delete_garbage() is called (which calls tp_clear). No Python code is supposed to have access to an object that has tp_clear called on it.

GH-136189 should make it so the example code doesn't crash, even with the old function tp_clear logic. I didn't actually test that but it should be so. For the test_function_tp_clear_leaves_consistent_state() case in test_gc.py, the func global is now None. That means that you can't get access to the function after it was cleared, which is how it should be.

This flaw in the GC existed for a long time, I think. Probably since weakrefs were first introduced actually.