GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-137400: Fix thread-safety issues when profiling all threads by colesbury · Pull Request #137518 · python/cpython

Conversation

colesbury commented Aug 7, 2025 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

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.

Issue: sys._setprofileallthreads race condition #137400

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          pythongh-137400: Fix a thread-safety issues when profiling all threads

bcb2772

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.

bedevere-app Bot mentioned this pull request

Aug 7, 2025

sys._setprofileallthreads race condition #137400

Closed


          Add blurb

09b28c8

colesbury added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label

Aug 7, 2025

bedevere-bot commented Aug 7, 2025

Copy link

Copy Markdown

🤖 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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label

Aug 7, 2025

colesbury changed the title ~~gh-137400: Fix a thread-safety issues when profiling all threads~~ gh-137400: Fix thread-safety issues when profiling all threads

Aug 7, 2025


          Fix error handling

468b8fe

colesbury mentioned this pull request

Aug 7, 2025

gh-137400: Fix a crash when disabling profiling across all threads #137471

Merged


          Merge branch 'main' into pythongh-137400-now-with-fewer-crashes

e79ed43

colesbury added needs backport to 3.14

bugs and security fixes

needs backport to 3.13 only security fixes labels

Aug 7, 2025

colesbury marked this pull request as ready for review

August 7, 2025 16:15

colesbury requested review from ZeroIntensity, ericsnowcurrently and markshannon as code owners

August 7, 2025 16:15

bedevere-app Bot added the awaiting core review label

Aug 7, 2025

colesbury requested review from gaogaotiantian, kumaraditya303 and pablogsal

August 7, 2025 16:15


          Fix another race with tracing

e8ab41d

gaogaotiantian reviewed

Aug 8, 2025

View reviewed changes

Comment thread

Lib/test/test_free_threading/test_monitoring.py

		self.observe_threads(noop, buf)

		def test_trace_concurrent(self):
		# Test calling a function concurrently from a tracing and a non-tracing

gaogaotiantian Aug 8, 2025

Copy link

Copy Markdown

Member

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.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

colesbury Aug 8, 2025

Copy link

Copy Markdown

Contributor Author

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

colesbury Aug 8, 2025

Copy link

Copy Markdown

Contributor Author

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.

For clarification, this test is related to the change to _MAYBE_INSTRUMENT with int check_instrumentation = (tstate->tracing == 0);

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ZeroIntensity reviewed

Aug 8, 2025

View reviewed changes

ZeroIntensity left a comment

Copy link

Copy Markdown

Member

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.

A few minor comments

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Python/ceval.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Python/instrumentation.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

Python/legacy_tracing.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303 reviewed

Aug 11, 2025

View reviewed changes

Comment thread

Python/legacy_tracing.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303 reviewed

Aug 11, 2025

View reviewed changes

Comment thread

Python/legacy_tracing.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

colesbury and others added 3 commits

August 11, 2025 11:50


          Merge branch 'main' into pythongh-137400-now-with-fewer-crashes

a1d8760


          Changes from review

03ec62c


          Update Python/legacy_tracing.c

1b613bd

Co-authored-by: Kumar Aditya <[email protected]>

kumaraditya303 reviewed

Aug 11, 2025

View reviewed changes

Comment thread

Python/legacy_tracing.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303 reviewed

Aug 11, 2025

View reviewed changes

Comment thread

Python/legacy_tracing.c Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Changes from review

0f7e62d

ZeroIntensity approved these changes

Aug 11, 2025

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting core review labels

Aug 11, 2025

kumaraditya303 reviewed

Aug 12, 2025

View reviewed changes

Comment thread

Python/legacy_tracing.c Resolved

Uh oh!

There was an error while loading. Please reload this page.


          Update Python/legacy_tracing.c

9225c9a

Co-authored-by: Kumar Aditya <[email protected]>

kumaraditya303 approved these changes

Aug 12, 2025

View reviewed changes

colesbury merged commit a10152f into python:main

Aug 13, 2025

colesbury deleted the gh-137400-now-with-fewer-crashes branch

August 13, 2025 18:15

bedevere-app Bot removed the awaiting merge label

Aug 13, 2025

miss-islington-app Bot commented Aug 13, 2025

Copy link

Copy Markdown

Thanks @colesbury for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington-app Bot commented Aug 13, 2025

Copy link

Copy Markdown

Sorry, @colesbury, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker a10152f8fd0f4b291e53d646cffe22fbeec73e1e 3.14

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington-app Bot commented Aug 13, 2025

Copy link

Copy Markdown

Sorry, @colesbury, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker a10152f8fd0f4b291e53d646cffe22fbeec73e1e 3.13

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

colesbury added a commit to colesbury/cpython that referenced this pull request

Aug 13, 2025


          [3.14] pythongh-137400: Fix thread-safety issues when profiling all t…

a4b3a4b

…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]>

bedevere-app Bot commented Aug 13, 2025

Copy link

Copy Markdown

GH-137730 is a backport of this pull request to the 3.14 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label

Aug 13, 2025

colesbury added a commit to colesbury/cpython that referenced this pull request

Aug 13, 2025


          [3.13] pythongh-137400: Fix thread-safety issues when profiling all t…

4c8fa53

…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]>

bedevere-app Bot commented Aug 13, 2025

Copy link

Copy Markdown

GH-137733 is a backport of this pull request to the 3.13 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-app Bot removed the needs backport to 3.13 only security fixes label

Aug 13, 2025

Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request

Aug 19, 2025


          pythongh-137400: Fix thread-safety issues when profiling all threads (p…

96ad357

…ythongh-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.

kumaraditya303 mentioned this pull request

Aug 26, 2025

PyEval_SetProfileAllThreads is racy under free-threading #132817

Closed

colesbury added a commit that referenced this pull request

Oct 7, 2025


          [3.14] gh-137400: Fix thread-safety issues when profiling all threads (…

fb699ef

…gh-137518) (gh-137730)

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]>
Co-authored-by: Petr Viktorin <[email protected]>

colesbury added a commit that referenced this pull request

Oct 9, 2025


          [3.13] gh-137400: Fix thread-safety issues when profiling all threads (…

16d2e97

…gh-137518) (gh-137733)

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)

devdanzin mentioned this pull request

Nov 17, 2025

Failed assertion in Python/legacy_tracing.c:431 on a free-threading build #127235

Closed

Reviewers

gaogaotiantian

gaogaotiantian left review comments

ZeroIntensity

ZeroIntensity approved these changes

kumaraditya303

kumaraditya303 approved these changes

ericsnowcurrently

Awaiting requested review from ericsnowcurrently

ericsnowcurrently is a code owner

markshannon

Awaiting requested review from markshannon

markshannon is a code owner

pablogsal

Awaiting requested review from pablogsal

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

colesbury commented Aug 7, 2025 • edited by bedevere-app Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Aug 7, 2025

Uh oh!

gaogaotiantian Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

colesbury Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

colesbury Aug 8, 2025

Choose a reason for hiding this comment

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miss-islington-app Bot commented Aug 13, 2025

Uh oh!

miss-islington-app Bot commented Aug 13, 2025

Uh oh!

miss-islington-app Bot commented Aug 13, 2025

Uh oh!

bedevere-app Bot commented Aug 13, 2025

Uh oh!

bedevere-app Bot commented Aug 13, 2025

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

colesbury commented Aug 7, 2025 •

edited by bedevere-app Bot

Loading

Uh oh!

There was an error while loading. Please reload this page.