diff --git a/Include/internal/pycore_interp_structs.h b/Include/internal/pycore_interp_structs.h index f1f427d99dea69..a3357d65e7b754 100644 --- a/Include/internal/pycore_interp_structs.h +++ b/Include/internal/pycore_interp_structs.h @@ -701,6 +701,7 @@ struct _Py_interp_static_objects { _PyGC_Head_UNUSED _hamt_empty_gc_not_used; PyHamtObject hamt_empty; PyBaseExceptionObject last_resort_memory_error; + PyObject *subclasses_weakref_sentinel; } singletons; }; diff --git a/Include/internal/pycore_weakref.h b/Include/internal/pycore_weakref.h index 4ed8928c0b92a8..fb2fd419f0ba78 100644 --- a/Include/internal/pycore_weakref.h +++ b/Include/internal/pycore_weakref.h @@ -9,6 +9,7 @@ extern "C" { #endif #include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION() +#include "pycore_interp_structs.h" // PyInterpreterState #include "pycore_lock.h" // PyMutex_LockFlags() #include "pycore_object.h" // _Py_REF_IS_MERGED() #include "pycore_pyatomic_ft_wrappers.h" @@ -127,6 +128,18 @@ extern void _PyWeakref_ClearWeakRefsNoCallbacks(PyObject *obj); PyAPI_FUNC(int) _PyWeakref_IsDead(PyObject *weakref); +// Create sentinel callback object for subclasses weakrefs +int _PyWeakref_InitSubclassSentinel(PyInterpreterState *interp); + +// Clear sentinel callback object +void _PyWeakref_ClearSubclassSentinel(PyInterpreterState *interp); + +// Create new PyWeakReference object with sentinel callback +PyObject * _PyWeakref_NewSubclassRef(PyObject *ob); + +// Check if weakref has sentinel callback +int _PyWeakref_IsSubclassRef(PyWeakReference *weakref); + #ifdef __cplusplus } #endif diff --git a/Lib/test/test_gc.py b/Lib/test/test_gc.py index b4cbfb6d774080..a6d9ce6f91ff2d 100644 --- a/Lib/test/test_gc.py +++ b/Lib/test/test_gc.py @@ -1125,6 +1125,30 @@ def test_something(self): """) assert_python_ok("-c", source) + def test_do_not_cleanup_type_subclasses_before_finalization(self): + # https://github.com/python/cpython/issues/135552 + code = """ + class BaseNode: + def __del__(self): + BaseNode.next = BaseNode.next.next + BaseNode.next.next + + class Node(BaseNode): + pass + + BaseNode.next = Node() + BaseNode.next.next = Node() + """ + assert_python_ok("-c", textwrap.dedent(code)) + + code_inside_function = textwrap.dedent(F""" + def test(): + {textwrap.indent(code, ' ')} + + test() + """) + assert_python_ok("-c", code_inside_function) + class IncrementalGCTests(unittest.TestCase): @unittest.skipIf(_testinternalcapi is None, "requires _testinternalcapi") @@ -1518,6 +1542,7 @@ def test_ast_fini(self): assert_python_ok("-c", code) + def setUpModule(): global enabled, debug enabled = gc.isenabled() diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-06-23-01-07-09.gh-issue-135552.eG9vzP.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-23-01-07-09.gh-issue-135552.eG9vzP.rst new file mode 100644 index 00000000000000..3d8e2e906475d1 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-06-23-01-07-09.gh-issue-135552.eG9vzP.rst @@ -0,0 +1,4 @@ +Do not clear type's subclasses weak references in the GC to prevent the clearing +of the type's subclasses list too early. This fix a crash that occurs when +:meth:`~object.__del__` modifies the base's attributes and tries to access them +from self. diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 6e7471cb5941a7..55a11a74ecacc8 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -9259,7 +9259,7 @@ add_subclass(PyTypeObject *base, PyTypeObject *type) if (key == NULL) return -1; - PyObject *ref = PyWeakref_NewRef((PyObject *)type, NULL); + PyObject *ref = _PyWeakref_NewSubclassRef((PyObject *)type); if (ref == NULL) { Py_DECREF(key); return -1; diff --git a/Objects/weakrefobject.c b/Objects/weakrefobject.c index bd4c4ac9b3475a..e3689a63afe9b1 100644 --- a/Objects/weakrefobject.c +++ b/Objects/weakrefobject.c @@ -1,12 +1,17 @@ #include "Python.h" #include "pycore_critical_section.h" +#include "pycore_global_objects.h" // _Py_INTERP_STATIC_OBJECT() +#include "pycore_interp_structs.h" // _PyInterpreterState_GET() #include "pycore_lock.h" #include "pycore_modsupport.h" // _PyArg_NoKwnames() #include "pycore_object.h" // _PyObject_GET_WEAKREFS_LISTPTR() +#include "pycore_opcode_metadata.h" // RESUME #include "pycore_pyerrors.h" // _PyErr_ChainExceptions1() #include "pycore_pystate.h" #include "pycore_weakref.h" // _PyWeakref_GET_REF() + + #ifdef Py_GIL_DISABLED /* * Thread-safety for free-threaded builds @@ -1132,3 +1137,179 @@ _PyWeakref_IsDead(PyObject *weakref) { return _PyWeakref_IS_DEAD(weakref); } + +static const uint8_t noop_func_bytecode[6] = { + RESUME, RESUME_AT_FUNC_START, + LOAD_CONST, 0, + RETURN_VALUE, 0 +}; + +static const uint8_t noop_func_linetable[2] = { + (1 << 7) // New entry. + | (PY_CODE_LOCATION_INFO_NO_COLUMNS << 3) + | (3 - 1), // Three code units. + 0, // Offset from co_firstlineno. +}; + +PyCodeObject * +create_new_noop_code(const char *filename, const char *funcname, int firstlineno) +{ + PyObject *nulltuple = NULL; + PyObject *filename_str = NULL; + PyObject *funcname_str = NULL; + PyObject *code_bytes = NULL; + PyObject *linetable_bytes = NULL; + PyObject *wr_str = NULL; + PyObject *consts = NULL; + PyObject *varnames = NULL; + PyCodeObject *result = NULL; + + nulltuple = PyTuple_New(0); + if (nulltuple == NULL) { + goto failed; + } + funcname_str = PyUnicode_FromString(funcname); + if (funcname_str == NULL) { + goto failed; + } + filename_str = PyUnicode_FromString(filename); + if (filename_str == NULL) { + goto failed; + } + code_bytes = PyBytes_FromStringAndSize((const char *)noop_func_bytecode, 6); + if (code_bytes == NULL) { + goto failed; + } + linetable_bytes = PyBytes_FromStringAndSize((const char *)noop_func_linetable, 2); + if (linetable_bytes == NULL) { + goto failed; + } + + consts = PyTuple_Pack(1, Py_None); + if (consts == NULL) { + goto failed; + } + + wr_str = PyUnicode_FromString("wr"); + if (wr_str == NULL) { + goto failed; + } + + varnames = PyTuple_Pack(1, wr_str); + if (varnames == NULL) { + goto failed; + } + +#define emptybytes (PyObject *)&_Py_SINGLETON(bytes_empty) + + int argcount = 1; + int posonlyargcount = 0; + int kwonlyargcount = 0; + int nlocals = 1; + int stacksize = 1; + int flags = CO_OPTIMIZED | CO_NEWLOCALS; + + result = PyUnstable_Code_NewWithPosOnlyArgs( + argcount, posonlyargcount, kwonlyargcount, nlocals, stacksize, flags, + code_bytes, consts, nulltuple, varnames, nulltuple, nulltuple, + filename_str, funcname_str, funcname_str, firstlineno, + linetable_bytes, emptybytes); + +#undef emptybytes + +failed: + Py_XDECREF(nulltuple); + Py_XDECREF(funcname_str); + Py_XDECREF(filename_str); + Py_XDECREF(code_bytes); + Py_XDECREF(linetable_bytes); + Py_XDECREF(consts); + Py_XDECREF(varnames); + Py_XDECREF(wr_str); + + return result; +} + +int +_PyWeakref_InitSubclassSentinel(PyInterpreterState *interp) +{ + int status = 0; + + PyObject *globals = NULL; + PyObject *func = NULL; + PyObject *code = (PyObject *)create_new_noop_code( + "", + "", + 1); + if (code == NULL) { + return -1; + } + + globals = PyDict_New(); + if (globals == NULL) { + status = -1; + goto failed; + } + + func = PyFunction_New(code, globals); + if (func == NULL) { + status = -1; + goto failed; + } + +#ifdef Py_DEBUG + PyObject *result = PyObject_CallOneArg(func, Py_None); + if (!result) { + status = -1; + goto failed; + } + Py_DECREF(result); +#endif + + _Py_SetImmortal(func); + _Py_INTERP_SINGLETON(interp, subclasses_weakref_sentinel) = func; + +failed: + Py_XDECREF(code); + Py_XDECREF(globals); + + return status; +} + +void +_PyWeakref_ClearSubclassSentinel(PyInterpreterState *interp) +{ + if (interp == interp->runtime->interpreters.main) { + PyObject *func = _Py_INTERP_SINGLETON(interp, subclasses_weakref_sentinel); + + assert(PyObject_GC_IsTracked(func) == 0); + _Py_SetMortal(_Py_INTERP_SINGLETON(interp, subclasses_weakref_sentinel), 1); + PyObject_GC_Track(func); + + Py_CLEAR(_Py_INTERP_SINGLETON(interp, subclasses_weakref_sentinel)); + } +} + +PyObject * +_PyWeakref_NewSubclassRef(PyObject *ob) +{ + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (interp != interp->runtime->interpreters.main) { + interp = interp->runtime->interpreters.main; + } + PyObject *func = _Py_INTERP_SINGLETON(interp, subclasses_weakref_sentinel); + return PyWeakref_NewRef(ob, func); +} + +int +_PyWeakref_IsSubclassRef(PyWeakReference *weakref) +{ + assert(weakref != NULL); + + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (interp != interp->runtime->interpreters.main) { + interp = interp->runtime->interpreters.main; + } + PyObject *func = _Py_INTERP_SINGLETON(interp, subclasses_weakref_sentinel); + return func != NULL && weakref->wr_callback == func; +} diff --git a/Python/gc.c b/Python/gc.c index 7b0e6d6e803504..da162d4c3fff91 100644 --- a/Python/gc.c +++ b/Python/gc.c @@ -906,8 +906,15 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old) * reference cycle. If we don't clear the weakref, the callback * will run and potentially cause a crash. See bpo-38006 for * one example. + * + * If this is a subclass weakref we can safely ignore it's cleanup. */ - _PyWeakref_ClearRef((PyWeakReference *)op); + if (!_PyWeakref_IsSubclassRef((PyWeakReference *)op)) { + _PyWeakref_ClearRef((PyWeakReference *)op); + } + else { + continue; + } } if (! _PyType_SUPPORTS_WEAKREFS(Py_TYPE(op))) { @@ -925,9 +932,20 @@ handle_weakrefs(PyGC_Head *unreachable, PyGC_Head *old) * all the weakrefs, and move the weakrefs with callbacks * that must be called into wrcb_to_call. */ - for (wr = *wrlist; wr != NULL; wr = *wrlist) { + PyWeakReference *wr_next = *wrlist; + for (wr = wr_next; wr != NULL; wr = wr_next) { PyGC_Head *wrasgc; /* AS_GC(wr) */ + wr_next = wr->wr_next; + + /* If this is a subclass weakref we can safely ignore it's cleanup. + * It has only sentinel callback (no-op) and we also can safely + * not invoke them. + */ + if (_PyWeakref_IsSubclassRef(wr) == 1) { + continue; + } + /* _PyWeakref_ClearRef clears the weakref but leaves * the callback pointer intact. Obscure: it also * changes *wrlist. diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 724fda63511282..aa80c057dcfbc0 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1376,6 +1376,12 @@ init_interp_main(PyThreadState *tstate) return _PyStatus_ERR("failed to set builtin dict watcher"); } + if (is_main_interp) { + if (_PyWeakref_InitSubclassSentinel(interp) < 0) { + return _PyStatus_ERR("failed to create subclasses weakref sentinel"); + } + } + assert(!_PyErr_Occurred(tstate)); return _PyStatus_OK(); @@ -1870,6 +1876,10 @@ finalize_interp_types(PyInterpreterState *interp) _PyObject_FinalizeUniqueIdPool(interp); #endif + // We clear subclass sentinel here because some weakrefs may + // hold reference to it (for example, type's subclasses) + _PyWeakref_ClearSubclassSentinel(interp); + _PyCode_Fini(interp); // Call _PyUnicode_ClearInterned() before _PyDict_Fini() since it uses diff --git a/Python/pystate.c b/Python/pystate.c index 0d4c26f92cec90..d95831f8ae6773 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -24,6 +24,7 @@ #include "pycore_stackref.h" // Py_STACKREF_DEBUG #include "pycore_time.h" // _PyTime_Init() #include "pycore_uniqueid.h" // _PyObject_FinalizePerThreadRefcounts() +#include "pycore_weakref.h" // _PyWeakref_ClearSubclassSentinel() /* --------------------------------------------------------------------------