Skip to content

Commit 59fd8de

Browse files
committed
Fix the race condition in the PyMem_RawFree() hook
* Hold the table lock while calling _PyTraceMalloc_Stop(). * Add tracemalloc_raw_free().
1 parent 1e3eac3 commit 59fd8de

File tree

1 file changed

+34
-7
lines changed

1 file changed

+34
-7
lines changed

Python/tracemalloc.c

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -603,18 +603,39 @@ tracemalloc_realloc(void *ctx, void *ptr, size_t new_size)
603603
static void
604604
tracemalloc_free(void *ctx, void *ptr)
605605
{
606+
if (ptr == NULL)
607+
return;
608+
606609
PyMemAllocatorEx *alloc = (PyMemAllocatorEx *)ctx;
607610

611+
alloc->free(alloc->ctx, ptr);
612+
613+
TABLES_LOCK();
614+
REMOVE_TRACE(ptr);
615+
TABLES_UNLOCK();
616+
}
617+
618+
619+
static void
620+
tracemalloc_raw_free(void *ctx, void *ptr)
621+
{
608622
if (ptr == NULL)
609623
return;
610624

625+
PyMemAllocatorEx *alloc = (PyMemAllocatorEx *)ctx;
626+
611627
/* GIL cannot be locked in PyMem_RawFree() because it would introduce
612628
a deadlock in _PyThreadState_DeleteCurrent(). */
613629

614630
alloc->free(alloc->ctx, ptr);
615631

616632
TABLES_LOCK();
617-
REMOVE_TRACE(ptr);
633+
if (tracemalloc_config.tracing) {
634+
REMOVE_TRACE(ptr);
635+
}
636+
else {
637+
// gh-128679: tracemalloc.stop() was called by another thread
638+
}
618639
TABLES_UNLOCK();
619640
}
620641

@@ -790,17 +811,15 @@ tracemalloc_clear_filename(void *value)
790811

791812
/* reentrant flag must be set to call this function and GIL must be held */
792813
static void
793-
tracemalloc_clear_traces(void)
814+
tracemalloc_clear_traces_unlocked(void)
794815
{
795816
/* The GIL protects variables against concurrent access */
796817
assert(PyGILState_Check());
797818

798-
TABLES_LOCK();
799819
_Py_hashtable_clear(tracemalloc_traces);
800820
_Py_hashtable_clear(tracemalloc_domains);
801821
tracemalloc_traced_memory = 0;
802822
tracemalloc_peak_traced_memory = 0;
803-
TABLES_UNLOCK();
804823

805824
_Py_hashtable_clear(tracemalloc_tracebacks);
806825

@@ -941,7 +960,7 @@ _PyTraceMalloc_Start(int max_nframe)
941960
alloc.malloc = tracemalloc_raw_malloc;
942961
alloc.calloc = tracemalloc_raw_calloc;
943962
alloc.realloc = tracemalloc_raw_realloc;
944-
alloc.free = tracemalloc_free;
963+
alloc.free = tracemalloc_raw_free;
945964

946965
alloc.ctx = &allocators.raw;
947966
PyMem_GetAllocator(PYMEM_DOMAIN_RAW, &allocators.raw);
@@ -974,6 +993,10 @@ _PyTraceMalloc_Stop(void)
974993
if (!tracemalloc_config.tracing)
975994
return;
976995

996+
// Lock to synchronize with tracemalloc_raw_free() which checks
997+
// 'tracing' while holding the lock.
998+
TABLES_LOCK();
999+
9771000
/* stop tracing Python memory allocations */
9781001
tracemalloc_config.tracing = 0;
9791002

@@ -984,11 +1007,13 @@ _PyTraceMalloc_Stop(void)
9841007
PyMem_SetAllocator(PYMEM_DOMAIN_MEM, &allocators.mem);
9851008
PyMem_SetAllocator(PYMEM_DOMAIN_OBJ, &allocators.obj);
9861009

987-
tracemalloc_clear_traces();
1010+
tracemalloc_clear_traces_unlocked();
9881011

9891012
/* release memory */
9901013
raw_free(tracemalloc_traceback);
9911014
tracemalloc_traceback = NULL;
1015+
1016+
TABLES_UNLOCK();
9921017
}
9931018

9941019

@@ -1436,7 +1461,9 @@ _PyTraceMalloc_ClearTraces(void)
14361461
return;
14371462
}
14381463
set_reentrant(1);
1439-
tracemalloc_clear_traces();
1464+
TABLES_LOCK();
1465+
tracemalloc_clear_traces_unlocked();
1466+
TABLES_UNLOCK();
14401467
set_reentrant(0);
14411468
}
14421469

0 commit comments

Comments
 (0)