-
-
Notifications
You must be signed in to change notification settings - Fork 33.2k
[3.13] gh-137400: Fix thread-safety issues when profiling all threads (gh-137518) #137733
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…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]>
FYI, the ABI change seems fine to me. |
self.set = not self.set | ||
|
||
|
||
@threading_helper.requires_working_threading() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably this test should work regardless of the threading model?
So, maybe just run this test unconditionally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the InstrumentationMultiThreadedMixin
subclasses hang on wasi where there are just no-op pthread stubs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The backport looks good to me!
Though, given that 3.13t is experimental and 3.14 is now out, maybe we should start thinking about not doing backports like this any more...
I generally agree, but one of the reasons I decided to backport this is that I think it will make the free-threaded CI more stable as well as fixing a thread-safety issue. |
There were a few thread-safety issues when profiling or tracing all threads via PyEval_SetProfileAllThreads or PyEval_SetTraceAllThreads:
c_profilefunc
andc_tracefunc
wasn't thread-safe on the free threading build. (cherry picked from commit a10152f)sys._setprofileallthreads
race condition #137400