Skip to content

Commit e69d068

Browse files
authored
gh-117657: Fix race involving GC and heap initialization (#119923)
The `_PyThreadState_Bind()` function is called before the first `PyEval_AcquireThread()` so it's not synchronized with the stop the world GC. We had a race where `gc_visit_heaps()` might visit a thread's heap while it's being initialized. Use a simple atomic int to avoid visiting heaps for threads that are not yet fully initialized (i.e., before `tstate_mimalloc_bind()` is called). The race was reproducible by running: `python Lib/test/test_importlib/partial/pool_in_threads.py`.
1 parent bd8c1f9 commit e69d068

File tree

4 files changed

+7
-3
lines changed

4 files changed

+7
-3
lines changed

Include/internal/pycore_mimalloc.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ struct _mimalloc_thread_state {
5252
mi_heap_t *current_object_heap;
5353
mi_heap_t heaps[_Py_MIMALLOC_HEAP_COUNT];
5454
mi_tld_t tld;
55+
int initialized;
5556
struct llist_node page_list;
5657
};
5758
#endif

Python/gc_free_threading.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,10 @@ gc_visit_heaps_lock_held(PyInterpreterState *interp, mi_block_visit_fun *visitor
252252
// visit each thread's heaps for GC objects
253253
for (PyThreadState *p = interp->threads.head; p != NULL; p = p->next) {
254254
struct _mimalloc_thread_state *m = &((_PyThreadStateImpl *)p)->mimalloc;
255+
if (!_Py_atomic_load_int(&m->initialized)) {
256+
// The thread may not have called tstate_mimalloc_bind() yet.
257+
continue;
258+
}
255259

256260
arg->offset = offset_base;
257261
if (!mi_heap_visit_blocks(&m->heaps[_Py_MIMALLOC_HEAP_GC], true,

Python/pystate.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3074,6 +3074,8 @@ tstate_mimalloc_bind(PyThreadState *tstate)
30743074
// _PyObject_GC_New() and similar functions temporarily override this to
30753075
// use one of the GC heaps.
30763076
mts->current_object_heap = &mts->heaps[_Py_MIMALLOC_HEAP_OBJECT];
3077+
3078+
_Py_atomic_store_int(&mts->initialized, 1);
30773079
#endif
30783080
}
30793081

Tools/tsan/suppressions_free_threading.txt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,13 @@ race:free_threadstate
2525

2626
race_top:_add_to_weak_set
2727
race_top:_in_weak_set
28-
race_top:_mi_heap_delayed_free_partial
2928
race_top:_PyEval_EvalFrameDefault
3029
race_top:_PyImport_AcquireLock
3130
race_top:_PyImport_ReleaseLock
3231
race_top:_PyType_HasFeature
3332
race_top:assign_version_tag
3433
race_top:insertdict
3534
race_top:lookup_tp_dict
36-
race_top:mi_heap_visit_pages
3735
race_top:PyMember_GetOne
3836
race_top:PyMember_SetOne
3937
race_top:new_reference
@@ -58,7 +56,6 @@ race_top:_Py_slot_tp_getattr_hook
5856
race_top:add_threadstate
5957
race_top:dump_traceback
6058
race_top:fatal_error
61-
race_top:mi_page_decode_padding
6259
race_top:_multiprocessing_SemLock_release_impl
6360
race_top:_PyFrame_GetCode
6461
race_top:_PyFrame_Initialize

0 commit comments

Comments
 (0)