From 50cf12b2e9d73d20723ef7fbc0d5aad27edbbfb0 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Thu, 9 May 2024 12:17:15 -0700 Subject: [PATCH 1/3] Fix data races reported by TSAN on `interp->threads.main` Use relaxed loads/stores when reading/writing to this field. This fixes races like https://gist.github.com/mpage/e07497ad8dd444a789ff306cb7996acc --- Python/pystate.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/Python/pystate.c b/Python/pystate.c index b1e085bb806915..de6a768a50f997 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -1038,6 +1038,17 @@ _PyInterpreterState_DeleteExceptMain(_PyRuntimeState *runtime) } #endif +static inline void +set_main_thread(PyInterpreterState *interp, PyThreadState *tstate) +{ + _Py_atomic_store_ptr_relaxed(&interp->threads.main, tstate); +} + +static inline PyThreadState * +get_main_thread(PyInterpreterState *interp) +{ + return _Py_atomic_load_ptr_relaxed(&interp->threads.main); +} int _PyInterpreterState_SetRunningMain(PyInterpreterState *interp) @@ -1052,21 +1063,22 @@ _PyInterpreterState_SetRunningMain(PyInterpreterState *interp) "current tstate has wrong interpreter"); return -1; } - interp->threads.main = tstate; + set_main_thread(interp, tstate); + return 0; } void _PyInterpreterState_SetNotRunningMain(PyInterpreterState *interp) { - assert(interp->threads.main == current_fast_get()); - interp->threads.main = NULL; + assert(get_main_thread(interp) == current_fast_get()); + set_main_thread(interp, NULL); } int _PyInterpreterState_IsRunningMain(PyInterpreterState *interp) { - if (interp->threads.main != NULL) { + if (get_main_thread(interp) != NULL) { return 1; } // Embedders might not know to call _PyInterpreterState_SetRunningMain(), @@ -1082,18 +1094,15 @@ int _PyThreadState_IsRunningMain(PyThreadState *tstate) { PyInterpreterState *interp = tstate->interp; - if (interp->threads.main != NULL) { - return tstate == interp->threads.main; - } // See the note in _PyInterpreterState_IsRunningMain() about // possible false negatives here for embedders. - return 0; + return get_main_thread(interp) == tstate; } int _PyInterpreterState_FailIfRunningMain(PyInterpreterState *interp) { - if (interp->threads.main != NULL) { + if (get_main_thread(interp) != NULL) { PyErr_SetString(PyExc_InterpreterError, "interpreter already running"); return -1; @@ -1105,8 +1114,8 @@ void _PyInterpreterState_ReinitRunningMain(PyThreadState *tstate) { PyInterpreterState *interp = tstate->interp; - if (interp->threads.main != tstate) { - interp->threads.main = NULL; + if (get_main_thread(interp) != tstate) { + set_main_thread(interp, NULL); } } From f412a0cdd96bdca224168ee902098aa837495355 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Thu, 9 May 2024 19:47:14 +0000 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2024-05-09-19-47-12.gh-issue-117657.Vn0Yey.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-05-09-19-47-12.gh-issue-117657.Vn0Yey.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-05-09-19-47-12.gh-issue-117657.Vn0Yey.rst b/Misc/NEWS.d/next/Core and Builtins/2024-05-09-19-47-12.gh-issue-117657.Vn0Yey.rst new file mode 100644 index 00000000000000..db4c5813ca610c --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-05-09-19-47-12.gh-issue-117657.Vn0Yey.rst @@ -0,0 +1 @@ +Fix data races on the field that stores a pointer to the interpreter's main thread that occur in free-threaded builds. From 17d86222d81259cb903776869640c83d3d3eeed9 Mon Sep 17 00:00:00 2001 From: Matt Page Date: Thu, 9 May 2024 15:03:46 -0700 Subject: [PATCH 3/3] Remove corresponding suppressions --- Tools/tsan/suppressions_free_threading.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/Tools/tsan/suppressions_free_threading.txt b/Tools/tsan/suppressions_free_threading.txt index 7f91a9113c4e1a..d5f4cd7acd36b7 100644 --- a/Tools/tsan/suppressions_free_threading.txt +++ b/Tools/tsan/suppressions_free_threading.txt @@ -33,8 +33,6 @@ race_top:_mi_heap_delayed_free_partial race_top:_PyEval_EvalFrameDefault race_top:_PyImport_AcquireLock race_top:_PyImport_ReleaseLock -race_top:_PyInterpreterState_SetNotRunningMain -race_top:_PyInterpreterState_IsRunningMain # https://gist.github.com/mpage/0a24eb2dd458441ededb498e9b0e5de8 race_top:_PyParkingLot_Park race_top:_PyType_HasFeature