From 2d9d3a9f5319ce3f850341d116b63cc51869df3a Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Thu, 8 Aug 2024 12:48:17 -0400 Subject: [PATCH] gh-122697: Fix free-threading memory leaks at shutdown (#122703) We were not properly accounting for interpreter memory leaks at shutdown and had two sources of leaks: * Objects that use deferred reference counting and were reachable via static types outlive the final GC. We now disable deferred reference counting on all objects if we are calling the GC due to interpreter shutdown. * `_PyMem_FreeDelayed` did not properly check for interpreter shutdown so we had some memory blocks that were enqueued to be freed, but never actually freed. * `_PyType_FinalizeIdPool` wasn't called at interpreter shutdown. --- .../2024-08-05-19-28-12.gh-issue-122697.17MvYl.rst | 2 ++ Objects/obmalloc.c | 11 ++++++++--- Python/gc_free_threading.c | 12 ++++++++++++ Python/pylifecycle.c | 4 ++++ Python/pystate.c | 2 +- 5 files changed, 27 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-08-05-19-28-12.gh-issue-122697.17MvYl.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-08-05-19-28-12.gh-issue-122697.17MvYl.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-08-05-19-28-12.gh-issue-122697.17MvYl.rst new file mode 100644 index 00000000000..34ee6a916bc --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-08-05-19-28-12.gh-issue-122697.17MvYl.rst @@ -0,0 +1,2 @@ +Fixed memory leaks at interpreter shutdown in the free-threaded build, and +also reporting of leaked memory blocks via :option:`-X showrefcount <-X>`. diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index a6a71802ef8..dfeccfa4dd7 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -1109,9 +1109,12 @@ free_delayed(uintptr_t ptr) #ifndef Py_GIL_DISABLED free_work_item(ptr); #else - if (_PyRuntime.stoptheworld.world_stopped) { - // Free immediately if the world is stopped, including during - // interpreter shutdown. + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (_PyInterpreterState_GetFinalizing(interp) != NULL || + interp->stoptheworld.world_stopped) + { + // Free immediately during interpreter shutdown or if the world is + // stopped. free_work_item(ptr); return; } @@ -1474,6 +1477,8 @@ _PyInterpreterState_FinalizeAllocatedBlocks(PyInterpreterState *interp) { #ifdef WITH_MIMALLOC if (_PyMem_MimallocEnabled()) { + Py_ssize_t leaked = _PyInterpreterState_GetAllocatedBlocks(interp); + interp->runtime->obmalloc.interpreter_leaks += leaked; return; } #endif diff --git a/Python/gc_free_threading.c b/Python/gc_free_threading.c index 1e02db00649..543bee24652 100644 --- a/Python/gc_free_threading.c +++ b/Python/gc_free_threading.c @@ -55,6 +55,7 @@ struct collection_state { struct visitor_args base; PyInterpreterState *interp; GCState *gcstate; + _PyGC_Reason reason; Py_ssize_t collected; Py_ssize_t uncollectable; Py_ssize_t long_lived_total; @@ -572,6 +573,16 @@ scan_heap_visitor(const mi_heap_t *heap, const mi_heap_area_t *area, worklist_push(&state->unreachable, op); } } + else if (state->reason == _Py_GC_REASON_SHUTDOWN && + _PyObject_HasDeferredRefcount(op)) + { + // Disable deferred refcounting for reachable objects as well during + // interpreter shutdown. This ensures that these objects are collected + // immediately when their last reference is removed. + disable_deferred_refcounting(op); + merge_refcount(op, 0); + state->long_lived_total++; + } else { // object is reachable, restore `ob_tid`; we're done with these objects gc_restore_tid(op); @@ -1228,6 +1239,7 @@ gc_collect_main(PyThreadState *tstate, int generation, _PyGC_Reason reason) struct collection_state state = { .interp = interp, .gcstate = gcstate, + .reason = reason, }; gc_collect_internal(interp, &state, generation); diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index 6b641c0775f..3a41c640fd9 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -28,6 +28,7 @@ #include "pycore_sliceobject.h" // _PySlice_Fini() #include "pycore_sysmodule.h" // _PySys_ClearAuditHooks() #include "pycore_traceback.h" // _Py_DumpTracebackThreads() +#include "pycore_typeid.h" // _PyType_FinalizeIdPool() #include "pycore_typeobject.h" // _PyTypes_InitTypes() #include "pycore_typevarobject.h" // _Py_clear_generic_types() #include "pycore_unicodeobject.h" // _PyUnicode_InitTypes() @@ -1832,6 +1833,9 @@ finalize_interp_types(PyInterpreterState *interp) _PyTypes_FiniTypes(interp); _PyTypes_Fini(interp); +#ifdef Py_GIL_DISABLED + _PyType_FinalizeIdPool(interp); +#endif _PyCode_Fini(interp); diff --git a/Python/pystate.c b/Python/pystate.c index 8f4818cee00..bba88b76088 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -20,7 +20,7 @@ #include "pycore_runtime_init.h" // _PyRuntimeState_INIT #include "pycore_sysmodule.h" // _PySys_Audit() #include "pycore_obmalloc.h" // _PyMem_obmalloc_state_on_heap() -#include "pycore_typeid.h" // _PyType_FinalizeIdPool +#include "pycore_typeid.h" // _PyType_FinalizeThreadLocalRefcounts() /* -------------------------------------------------------------------------- CAUTION