https://github.com/python/cpython/commit/46c7e13c055c218e18b0424efc60965e6a5... commit: 46c7e13c055c218e18b0424efc60965e6a5fe6ea branch: main author: Victor Stinner <vstinner@python.org> committer: vstinner <vstinner@python.org> date: 2025-01-23T12:07:34+01:00 summary: gh-129185: Fix PyTraceMalloc_Untrack() at Python exit (#129191) Support calling PyTraceMalloc_Track() and PyTraceMalloc_Untrack() during late Python finalization. * Call _PyTraceMalloc_Fini() later in Python finalization. * Test also PyTraceMalloc_Untrack() without the GIL * PyTraceMalloc_Untrack() now gets the GIL. * Test also PyTraceMalloc_Untrack() in test_tracemalloc_track_race(). files: M Lib/test/test_tracemalloc.py M Modules/_testcapi/mem.c M Modules/_testcapimodule.c M Python/pylifecycle.c M Python/tracemalloc.c diff --git a/Lib/test/test_tracemalloc.py b/Lib/test/test_tracemalloc.py index 238ae14b388c76..0220a83d24b428 100644 --- a/Lib/test/test_tracemalloc.py +++ b/Lib/test/test_tracemalloc.py @@ -1,6 +1,7 @@ import contextlib import os import sys +import textwrap import tracemalloc import unittest from unittest.mock import patch @@ -19,6 +20,7 @@ _testinternalcapi = None +DEFAULT_DOMAIN = 0 EMPTY_STRING_SIZE = sys.getsizeof(b'') INVALID_NFRAME = (-1, 2**30) @@ -1027,8 +1029,8 @@ def track(self, release_gil=False, nframe=1): release_gil) return frames - def untrack(self): - _testcapi.tracemalloc_untrack(self.domain, self.ptr) + def untrack(self, release_gil=False): + _testcapi.tracemalloc_untrack(self.domain, self.ptr, release_gil) def get_traced_memory(self): # Get the traced size in the domain @@ -1070,7 +1072,7 @@ def test_track_already_tracked(self): self.assertEqual(self.get_traceback(), tracemalloc.Traceback(frames)) - def test_untrack(self): + def check_untrack(self, release_gil): tracemalloc.start() self.track() @@ -1078,13 +1080,19 @@ def test_untrack(self): self.assertEqual(self.get_traced_memory(), self.size) # untrack must remove the trace - self.untrack() + self.untrack(release_gil) self.assertIsNone(self.get_traceback()) self.assertEqual(self.get_traced_memory(), 0) # calling _PyTraceMalloc_Untrack() multiple times must not crash - self.untrack() - self.untrack() + self.untrack(release_gil) + self.untrack(release_gil) + + def test_untrack(self): + self.check_untrack(False) + + def test_untrack_without_gil(self): + self.check_untrack(True) def test_stop_track(self): tracemalloc.start() @@ -1110,6 +1118,29 @@ def test_tracemalloc_track_race(self): # gh-128679: Test fix for tracemalloc.stop() race condition _testcapi.tracemalloc_track_race() + def test_late_untrack(self): + code = textwrap.dedent(f""" + from test import support + import tracemalloc + import _testcapi + + class Tracked: + def __init__(self, domain, size): + self.domain = domain + self.ptr = id(self) + self.size = size + _testcapi.tracemalloc_track(self.domain, self.ptr, self.size) + + def __del__(self, untrack=_testcapi.tracemalloc_untrack): + untrack(self.domain, self.ptr, 1) + + domain = {DEFAULT_DOMAIN} + tracemalloc.start() + obj = Tracked(domain, 1024 * 1024) + support.late_deletion(obj) + """) + assert_python_ok("-c", code) + if __name__ == "__main__": unittest.main() diff --git a/Modules/_testcapi/mem.c b/Modules/_testcapi/mem.c index ab4ad934644c38..ecae5ba26226a6 100644 --- a/Modules/_testcapi/mem.c +++ b/Modules/_testcapi/mem.c @@ -557,8 +557,9 @@ tracemalloc_untrack(PyObject *self, PyObject *args) { unsigned int domain; PyObject *ptr_obj; + int release_gil = 0; - if (!PyArg_ParseTuple(args, "IO", &domain, &ptr_obj)) { + if (!PyArg_ParseTuple(args, "IO|i", &domain, &ptr_obj, &release_gil)) { return NULL; } void *ptr = PyLong_AsVoidPtr(ptr_obj); @@ -566,7 +567,15 @@ tracemalloc_untrack(PyObject *self, PyObject *args) return NULL; } - int res = PyTraceMalloc_Untrack(domain, (uintptr_t)ptr); + int res; + if (release_gil) { + Py_BEGIN_ALLOW_THREADS + res = PyTraceMalloc_Untrack(domain, (uintptr_t)ptr); + Py_END_ALLOW_THREADS + } + else { + res = PyTraceMalloc_Untrack(domain, (uintptr_t)ptr); + } if (res < 0) { PyErr_SetString(PyExc_RuntimeError, "PyTraceMalloc_Untrack error"); return NULL; diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 996b96bc000450..c405a352ed74a1 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3394,6 +3394,7 @@ static void tracemalloc_track_race_thread(void *data) { PyTraceMalloc_Track(123, 10, 1); + PyTraceMalloc_Untrack(123, 10); PyThread_type_lock lock = (PyThread_type_lock)data; PyThread_release_lock(lock); diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index f6526725d5dccc..52890cfc5df829 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -2113,7 +2113,7 @@ _Py_Finalize(_PyRuntimeState *runtime) /* Disable tracemalloc after all Python objects have been destroyed, so it is possible to use tracemalloc in objects destructor. */ - _PyTraceMalloc_Fini(); + _PyTraceMalloc_Stop(); /* Finalize any remaining import state */ // XXX Move these up to where finalize_modules() is currently. @@ -2166,6 +2166,8 @@ _Py_Finalize(_PyRuntimeState *runtime) finalize_interp_clear(tstate); + _PyTraceMalloc_Fini(); + #ifdef Py_TRACE_REFS /* Display addresses (& refcnts) of all objects still alive. * An address can be used to find the repr of the object, printed diff --git a/Python/tracemalloc.c b/Python/tracemalloc.c index b398ea379e0607..62065e85ac8350 100644 --- a/Python/tracemalloc.c +++ b/Python/tracemalloc.c @@ -1256,9 +1256,17 @@ PyTraceMalloc_Track(unsigned int domain, uintptr_t ptr, size_t size) { PyGILState_STATE gil_state = PyGILState_Ensure(); + int result; + + // gh-129185: Check before TABLES_LOCK() to support calls after + // _PyTraceMalloc_Fini(). + if (!tracemalloc_config.tracing) { + result = -2; + goto done; + } + TABLES_LOCK(); - int result; if (tracemalloc_config.tracing) { result = tracemalloc_add_trace_unlocked(domain, ptr, size); } @@ -1268,6 +1276,7 @@ PyTraceMalloc_Track(unsigned int domain, uintptr_t ptr, } TABLES_UNLOCK(); +done: PyGILState_Release(gil_state); return result; @@ -1277,9 +1286,19 @@ PyTraceMalloc_Track(unsigned int domain, uintptr_t ptr, int PyTraceMalloc_Untrack(unsigned int domain, uintptr_t ptr) { + // Need the GIL to prevent races on the first 'tracing' test + PyGILState_STATE gil_state = PyGILState_Ensure(); + int result; + + // gh-129185: Check before TABLES_LOCK() to support calls after + // _PyTraceMalloc_Fini() + if (!tracemalloc_config.tracing) { + result = -2; + goto done; + } + TABLES_LOCK(); - int result; if (tracemalloc_config.tracing) { tracemalloc_remove_trace_unlocked(domain, ptr); result = 0; @@ -1290,6 +1309,8 @@ PyTraceMalloc_Untrack(unsigned int domain, uintptr_t ptr) } TABLES_UNLOCK(); +done: + PyGILState_Release(gil_state); return result; }