Skip to content

[3.12] gh-128679: Fix tracemalloc.stop() race conditions (#128897) #129022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Include/tracemalloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ PyAPI_FUNC(PyObject *) _PyTraceMalloc_GetTraces(void);
PyAPI_FUNC(PyObject *) _PyTraceMalloc_GetObjectTraceback(PyObject *obj);

/* Initialize tracemalloc */
PyAPI_FUNC(int) _PyTraceMalloc_Init(void);
PyAPI_FUNC(PyStatus) _PyTraceMalloc_Init(void);

/* Start tracemalloc */
PyAPI_FUNC(int) _PyTraceMalloc_Start(int max_nframe);
Expand Down
10 changes: 9 additions & 1 deletion Lib/test/test_tracemalloc.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
interpreter_requires_environment)
from test import support
from test.support import os_helper
from test.support import threading_helper

try:
import _testcapi
Expand Down Expand Up @@ -946,7 +947,6 @@ def check_env_var_invalid(self, nframe):
return
self.fail(f"unexpected output: {stderr!a}")


def test_env_var_invalid(self):
for nframe in INVALID_NFRAME:
with self.subTest(nframe=nframe):
Expand Down Expand Up @@ -1095,6 +1095,14 @@ def test_stop_untrack(self):
with self.assertRaises(RuntimeError):
self.untrack()

@unittest.skipIf(_testcapi is None, 'need _testcapi')
@threading_helper.requires_working_threading()
# gh-128679: Test crash on a debug build (especially on FreeBSD).
@unittest.skipIf(support.Py_DEBUG, 'need release build')
def test_tracemalloc_track_race(self):
# gh-128679: Test fix for tracemalloc.stop() race condition
_testcapi.tracemalloc_track_race()


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix :func:`tracemalloc.stop` race condition. Fix :mod:`tracemalloc` to
support calling :func:`tracemalloc.stop` in one thread, while another thread
is tracing memory allocations. Patch by Victor Stinner.
100 changes: 100 additions & 0 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -3238,6 +3238,105 @@ test_atexit(PyObject *self, PyObject *Py_UNUSED(args))

static PyObject *test_buildvalue_issue38913(PyObject *, PyObject *);


static void
tracemalloc_track_race_thread(void *data)
{
PyTraceMalloc_Track(123, 10, 1);

PyThread_type_lock lock = (PyThread_type_lock)data;
PyThread_release_lock(lock);
}

// gh-128679: Test fix for tracemalloc.stop() race condition
static PyObject *
tracemalloc_track_race(PyObject *self, PyObject *args)
{
#define NTHREAD 50
PyObject *tracemalloc = NULL;
PyObject *stop = NULL;
PyThread_type_lock locks[NTHREAD];
memset(locks, 0, sizeof(locks));

// Call tracemalloc.start()
tracemalloc = PyImport_ImportModule("tracemalloc");
if (tracemalloc == NULL) {
goto error;
}
PyObject *start = PyObject_GetAttrString(tracemalloc, "start");
if (start == NULL) {
goto error;
}
PyObject *res = PyObject_CallNoArgs(start);
Py_DECREF(start);
if (res == NULL) {
goto error;
}
Py_DECREF(res);

stop = PyObject_GetAttrString(tracemalloc, "stop");
Py_CLEAR(tracemalloc);
if (stop == NULL) {
goto error;
}

// Start threads
for (size_t i = 0; i < NTHREAD; i++) {
PyThread_type_lock lock = PyThread_allocate_lock();
if (!lock) {
PyErr_NoMemory();
goto error;
}
locks[i] = lock;
PyThread_acquire_lock(lock, 1);

unsigned long thread;
thread = PyThread_start_new_thread(tracemalloc_track_race_thread,
(void*)lock);
if (thread == (unsigned long)-1) {
PyErr_SetString(PyExc_RuntimeError, "can't start new thread");
goto error;
}
}

// Call tracemalloc.stop() while threads are running
res = PyObject_CallNoArgs(stop);
Py_CLEAR(stop);
if (res == NULL) {
goto error;
}
Py_DECREF(res);

// Wait until threads complete with the GIL released
Py_BEGIN_ALLOW_THREADS
for (size_t i = 0; i < NTHREAD; i++) {
PyThread_type_lock lock = locks[i];
PyThread_acquire_lock(lock, 1);
PyThread_release_lock(lock);
}
Py_END_ALLOW_THREADS

// Free threads locks
for (size_t i=0; i < NTHREAD; i++) {
PyThread_type_lock lock = locks[i];
PyThread_free_lock(lock);
}
Py_RETURN_NONE;

error:
Py_CLEAR(tracemalloc);
Py_CLEAR(stop);
for (size_t i=0; i < NTHREAD; i++) {
PyThread_type_lock lock = locks[i];
if (lock) {
PyThread_free_lock(lock);
}
}
return NULL;
#undef NTHREAD
}


static PyMethodDef TestMethods[] = {
{"set_errno", set_errno, METH_VARARGS},
{"test_config", test_config, METH_NOARGS},
Expand Down Expand Up @@ -3378,6 +3477,7 @@ static PyMethodDef TestMethods[] = {
{"function_get_kw_defaults", function_get_kw_defaults, METH_O, NULL},
{"function_set_kw_defaults", function_set_kw_defaults, METH_VARARGS, NULL},
{"test_atexit", test_atexit, METH_NOARGS},
{"tracemalloc_track_race", tracemalloc_track_race, METH_NOARGS},
{NULL, NULL} /* sentinel */
};

Expand Down
5 changes: 0 additions & 5 deletions Modules/_tracemalloc.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,5 @@ PyInit__tracemalloc(void)
if (m == NULL)
return NULL;

if (_PyTraceMalloc_Init() < 0) {
Py_DECREF(m);
return NULL;
}

return m;
}
5 changes: 5 additions & 0 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,11 @@ pycore_create_interpreter(_PyRuntimeState *runtime,
// didn't depend on interp->feature_flags being set already.
_PyObject_InitState(interp);

status = _PyTraceMalloc_Init();
if (_PyStatus_EXCEPTION(status)) {
return status;
}

PyThreadState *tstate = _PyThreadState_New(interp);
if (tstate == NULL) {
return _PyStatus_ERR("can't make first thread");
Expand Down
Loading
Loading