gh-137400: Fix thread-safety issues when profiling all threads #137518
Conversation
There were a few thread-safety issues when profiling or tracing all threads via PyEval_SetProfileAllThreads or PyEval_SetTraceAllThreads: * The loop over thread states could crash if a thread exits concurrently (in both the free threading and default build) * The modification of `c_profilefunc` and `c_tracefunc` wasn't thread-safe on the free threading build.
|
🤖 New build scheduled with the buildbot fleet by @colesbury for commit 09b28c8 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F137518%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again. |
| @@ -387,6 +413,38 @@ def noop(): | |||
|
|
|||
| self.observe_threads(noop, buf) | |||
|
|
|||
| def test_trace_concurrent(self): | |||
| # Test calling a function concurrently from a tracing and a non-tracing | |||
There was a problem hiding this comment.
What was the behavior of this test when this was not fixed? Will it crash because the code object is partially instrumented, or will we miss some trace func calls because the code object is not instrumented at all?
There was a problem hiding this comment.
Mostly it showed up as data races reported by TSan. I don't think I saw any crashes for this one.
The test above (SetProfileAllThreadsMultiThreaded) would crash before this PR.
There was a problem hiding this comment.
For clarification, this test is related to the change to _MAYBE_INSTRUMENT with int check_instrumentation = (tstate->tracing == 0);
| assert(is_tstate_valid(current_tstate)); | ||
| assert(current_tstate->interp == interp); | ||
|
|
||
| if (_PySys_Audit(current_tstate, "sys.setprofile", NULL) < 0) { |
There was a problem hiding this comment.
Shouldn't this be sys._setprofileallthreads?
There was a problem hiding this comment.
I think it's less risky to keep the auditing even name the same as it was before.
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
There were a few thread-safety issues when profiling or tracing all threads via PyEval_SetProfileAllThreads or PyEval_SetTraceAllThreads:
c_profilefuncandc_tracefuncwasn't thread-safe on the free threading build.sys._setprofileallthreadsrace condition #137400