Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 3193cb5

Browse filesBrowse files
authored
gh-128679: Fix tracemalloc.stop() race conditions (#128893)
tracemalloc_alloc(), tracemalloc_realloc(), tracemalloc_free(), _PyTraceMalloc_TraceRef() and _PyTraceMalloc_GetMemory() now check 'tracemalloc_config.tracing' after calling TABLES_LOCK(). _PyTraceMalloc_TraceRef() now always returns 0.
1 parent 313b96e commit 3193cb5
Copy full SHA for 3193cb5

File tree

Expand file treeCollapse file tree

4 files changed

+146
-17
lines changed
Filter options
Expand file treeCollapse file tree

4 files changed

+146
-17
lines changed

‎Lib/test/test_tracemalloc.py

Copy file name to clipboardExpand all lines: Lib/test/test_tracemalloc.py
+8-2Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,9 @@
77
from test.support.script_helper import (assert_python_ok, assert_python_failure,
88
interpreter_requires_environment)
99
from test import support
10-
from test.support import os_helper
1110
from test.support import force_not_colorized
11+
from test.support import os_helper
12+
from test.support import threading_helper
1213

1314
try:
1415
import _testcapi
@@ -952,7 +953,6 @@ def check_env_var_invalid(self, nframe):
952953
return
953954
self.fail(f"unexpected output: {stderr!a}")
954955

955-
956956
def test_env_var_invalid(self):
957957
for nframe in INVALID_NFRAME:
958958
with self.subTest(nframe=nframe):
@@ -1101,6 +1101,12 @@ def test_stop_untrack(self):
11011101
with self.assertRaises(RuntimeError):
11021102
self.untrack()
11031103

1104+
@unittest.skipIf(_testcapi is None, 'need _testcapi')
1105+
@threading_helper.requires_working_threading()
1106+
def test_tracemalloc_track_race(self):
1107+
# gh-128679: Test fix for tracemalloc.stop() race condition
1108+
_testcapi.tracemalloc_track_race()
1109+
11041110

11051111
if __name__ == "__main__":
11061112
unittest.main()
+3Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
:mod:`tracemalloc`: Fix race conditions when :func:`tracemalloc.stop` is
2+
called by a thread, while other threads are tracing memory allocations.
3+
Patch by Victor Stinner.

‎Modules/_testcapimodule.c

Copy file name to clipboardExpand all lines: Modules/_testcapimodule.c
+99Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3435,6 +3435,104 @@ code_offset_to_line(PyObject* self, PyObject* const* args, Py_ssize_t nargsf)
34353435
return PyLong_FromInt32(PyCode_Addr2Line(code, offset));
34363436
}
34373437

3438+
3439+
static void
3440+
tracemalloc_track_race_thread(void *data)
3441+
{
3442+
PyTraceMalloc_Track(123, 10, 1);
3443+
3444+
PyThread_type_lock lock = (PyThread_type_lock)data;
3445+
PyThread_release_lock(lock);
3446+
}
3447+
3448+
// gh-128679: Test fix for tracemalloc.stop() race condition
3449+
static PyObject *
3450+
tracemalloc_track_race(PyObject *self, PyObject *args)
3451+
{
3452+
#define NTHREAD 50
3453+
PyObject *tracemalloc = NULL;
3454+
PyObject *stop = NULL;
3455+
PyThread_type_lock locks[NTHREAD];
3456+
memset(locks, 0, sizeof(locks));
3457+
3458+
// Call tracemalloc.start()
3459+
tracemalloc = PyImport_ImportModule("tracemalloc");
3460+
if (tracemalloc == NULL) {
3461+
goto error;
3462+
}
3463+
PyObject *start = PyObject_GetAttrString(tracemalloc, "start");
3464+
if (start == NULL) {
3465+
goto error;
3466+
}
3467+
PyObject *res = PyObject_CallNoArgs(start);
3468+
Py_DECREF(start);
3469+
if (res == NULL) {
3470+
goto error;
3471+
}
3472+
Py_DECREF(res);
3473+
3474+
stop = PyObject_GetAttrString(tracemalloc, "stop");
3475+
Py_CLEAR(tracemalloc);
3476+
if (stop == NULL) {
3477+
goto error;
3478+
}
3479+
3480+
// Start threads
3481+
for (size_t i = 0; i < NTHREAD; i++) {
3482+
PyThread_type_lock lock = PyThread_allocate_lock();
3483+
if (!lock) {
3484+
PyErr_NoMemory();
3485+
goto error;
3486+
}
3487+
locks[i] = lock;
3488+
PyThread_acquire_lock(lock, 1);
3489+
3490+
unsigned long thread;
3491+
thread = PyThread_start_new_thread(tracemalloc_track_race_thread,
3492+
(void*)lock);
3493+
if (thread == (unsigned long)-1) {
3494+
PyErr_SetString(PyExc_RuntimeError, "can't start new thread");
3495+
goto error;
3496+
}
3497+
}
3498+
3499+
// Call tracemalloc.stop() while threads are running
3500+
res = PyObject_CallNoArgs(stop);
3501+
Py_CLEAR(stop);
3502+
if (res == NULL) {
3503+
goto error;
3504+
}
3505+
Py_DECREF(res);
3506+
3507+
// Wait until threads complete with the GIL released
3508+
Py_BEGIN_ALLOW_THREADS
3509+
for (size_t i = 0; i < NTHREAD; i++) {
3510+
PyThread_type_lock lock = locks[i];
3511+
PyThread_acquire_lock(lock, 1);
3512+
PyThread_release_lock(lock);
3513+
}
3514+
Py_END_ALLOW_THREADS
3515+
3516+
// Free threads locks
3517+
for (size_t i=0; i < NTHREAD; i++) {
3518+
PyThread_type_lock lock = locks[i];
3519+
PyThread_free_lock(lock);
3520+
}
3521+
Py_RETURN_NONE;
3522+
3523+
error:
3524+
Py_CLEAR(tracemalloc);
3525+
Py_CLEAR(stop);
3526+
for (size_t i=0; i < NTHREAD; i++) {
3527+
PyThread_type_lock lock = locks[i];
3528+
if (lock) {
3529+
PyThread_free_lock(lock);
3530+
}
3531+
}
3532+
return NULL;
3533+
#undef NTHREAD
3534+
}
3535+
34383536
static PyMethodDef TestMethods[] = {
34393537
{"set_errno", set_errno, METH_VARARGS},
34403538
{"test_config", test_config, METH_NOARGS},
@@ -3578,6 +3676,7 @@ static PyMethodDef TestMethods[] = {
35783676
{"type_freeze", type_freeze, METH_VARARGS},
35793677
{"test_atexit", test_atexit, METH_NOARGS},
35803678
{"code_offset_to_line", _PyCFunction_CAST(code_offset_to_line), METH_FASTCALL},
3679+
{"tracemalloc_track_race", tracemalloc_track_race, METH_NOARGS},
35813680
{NULL, NULL} /* sentinel */
35823681
};
35833682

‎Python/tracemalloc.c

Copy file name to clipboardExpand all lines: Python/tracemalloc.c
+36-15Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -567,11 +567,14 @@ tracemalloc_alloc(int need_gil, int use_calloc,
567567
}
568568
TABLES_LOCK();
569569

570-
if (ADD_TRACE(ptr, nelem * elsize) < 0) {
571-
// Failed to allocate a trace for the new memory block
572-
alloc->free(alloc->ctx, ptr);
573-
ptr = NULL;
570+
if (tracemalloc_config.tracing) {
571+
if (ADD_TRACE(ptr, nelem * elsize) < 0) {
572+
// Failed to allocate a trace for the new memory block
573+
alloc->free(alloc->ctx, ptr);
574+
ptr = NULL;
575+
}
574576
}
577+
// else: gh-128679: tracemalloc.stop() was called by another thread
575578

576579
TABLES_UNLOCK();
577580
if (need_gil) {
@@ -614,6 +617,11 @@ tracemalloc_realloc(int need_gil, void *ctx, void *ptr, size_t new_size)
614617
}
615618
TABLES_LOCK();
616619

620+
if (!tracemalloc_config.tracing) {
621+
// gh-128679: tracemalloc.stop() was called by another thread
622+
goto unlock;
623+
}
624+
617625
if (ptr != NULL) {
618626
// An existing memory block has been resized
619627

@@ -646,6 +654,7 @@ tracemalloc_realloc(int need_gil, void *ctx, void *ptr, size_t new_size)
646654
}
647655
}
648656

657+
unlock:
649658
TABLES_UNLOCK();
650659
if (need_gil) {
651660
PyGILState_Release(gil_state);
@@ -674,7 +683,12 @@ tracemalloc_free(void *ctx, void *ptr)
674683
}
675684

676685
TABLES_LOCK();
677-
REMOVE_TRACE(ptr);
686+
687+
if (tracemalloc_config.tracing) {
688+
REMOVE_TRACE(ptr);
689+
}
690+
// else: gh-128679: tracemalloc.stop() was called by another thread
691+
678692
TABLES_UNLOCK();
679693
}
680694

@@ -1312,8 +1326,9 @@ _PyTraceMalloc_TraceRef(PyObject *op, PyRefTracerEvent event,
13121326
assert(PyGILState_Check());
13131327
TABLES_LOCK();
13141328

1315-
int result = -1;
1316-
assert(tracemalloc_config.tracing);
1329+
if (!tracemalloc_config.tracing) {
1330+
goto done;
1331+
}
13171332

13181333
PyTypeObject *type = Py_TYPE(op);
13191334
const size_t presize = _PyType_PreHeaderSize(type);
@@ -1325,13 +1340,13 @@ _PyTraceMalloc_TraceRef(PyObject *op, PyRefTracerEvent event,
13251340
traceback_t *traceback = traceback_new();
13261341
if (traceback != NULL) {
13271342
trace->traceback = traceback;
1328-
result = 0;
13291343
}
13301344
}
13311345
/* else: cannot track the object, its memory block size is unknown */
13321346

1347+
done:
13331348
TABLES_UNLOCK();
1334-
return result;
1349+
return 0;
13351350
}
13361351

13371352

@@ -1472,13 +1487,19 @@ int _PyTraceMalloc_GetTracebackLimit(void)
14721487
size_t
14731488
_PyTraceMalloc_GetMemory(void)
14741489
{
1475-
size_t size = _Py_hashtable_size(tracemalloc_tracebacks);
1476-
size += _Py_hashtable_size(tracemalloc_filenames);
1477-
14781490
TABLES_LOCK();
1479-
size += _Py_hashtable_size(tracemalloc_traces);
1480-
_Py_hashtable_foreach(tracemalloc_domains,
1481-
tracemalloc_get_tracemalloc_memory_cb, &size);
1491+
size_t size;
1492+
if (tracemalloc_config.tracing) {
1493+
size = _Py_hashtable_size(tracemalloc_tracebacks);
1494+
size += _Py_hashtable_size(tracemalloc_filenames);
1495+
1496+
size += _Py_hashtable_size(tracemalloc_traces);
1497+
_Py_hashtable_foreach(tracemalloc_domains,
1498+
tracemalloc_get_tracemalloc_memory_cb, &size);
1499+
}
1500+
else {
1501+
size = 0;
1502+
}
14821503
TABLES_UNLOCK();
14831504
return size;
14841505
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.