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.
colesbury
added
the
🔨 test-with-refleak-buildbots
|
🤖 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. |
bedevere-bot
removed
the
🔨 test-with-refleak-buildbots
colesbury
changed the title
colesbury
added
needs backport to 3.14
colesbury
requested review from
ericsnowcurrently,
ZeroIntensity and
markshannon
as code owners
| @@ -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);
Co-authored-by: Kumar Aditya <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
|
Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
|
Sorry, @colesbury, I could not cleanly backport this to |
|
Sorry, @colesbury, I could not cleanly backport this to |
…hreads (pythongh-137518) 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. (cherry picked from commit a10152f) Co-authored-by: Sam Gross <[email protected]>
|
GH-137730 is a backport of this pull request to the 3.14 branch. |
…hreads (pythongh-137518) 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. (cherry picked from commit a10152f) Co-authored-by: Sam Gross <[email protected]>
|
GH-137733 is a backport of this pull request to the 3.13 branch. |
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