GV

GeezerVison

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

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-116738: Make cProfile module thread-safe by yoney · Pull Request #138229 · python/cpython

Conversation

@yoney

@yoney yoney commented Aug 28, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

This change uses critical sections to make cProfile (_lsprof) thread-safe when the GIL is disabled. It does this by applying the @critical_section clinic directive to ProfilerObject.

Protecting ProfilerObject prevents crashes, and the thread sanitizer does not report errors for the new FT test. However, the stats still show incorrect recursive call information.

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
10000/538    0.085    0.000    0.186    0.000 {method 'insert' of 'list' objects}

This happens because of limitations in the cProfile module, and these issues are not directly related to the FT Python build. By adjusting sys.setswitchinterval(), you can see similar incorrect recursive call reports even in a non-FT Python build.

This is a quick fix to prevent crashes. It is also a request for comments on whether we should make bigger changes to the cProfile module to handle multithreading better, such as collecting profile data per thread.

cc: @mpage, @colesbury, @Yhg1s

  • Issue: Audit all built-in modules for thread safety #116738

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@yoney
pythongh-116738: Make cProfile module thread-safe
3bb0e14
@bedevere-app bedevere-app Bot mentioned this pull request Aug 28, 2025
Audit all built-in modules for thread safety #116738
Open
@yoney
yoney marked this pull request as ready for review August 28, 2025 18:03
@bedevere-app bedevere-app Bot added the awaiting review label Aug 28, 2025
@mpage
mpage requested review from Yhg1s, colesbury and mpage September 2, 2025 15:49
@mpage mpage added the topic-free-threading label Sep 3, 2025
@mpage

mpage commented Sep 3, 2025

Copy link
Copy Markdown
Contributor

@yoney - LGTM. I think it's worth doing the work to make cProfile work well in a multi-threaded environment in the free-threaded builds. Do you have a sense of the scope of the changes that will be required to make cProfile work correctly in a multi-threaded setting? Unless they're similar in scope to these changes I think it's probably worth merging this PR (and requesting a backport to 3.14) since it shouldn't introduce too much additional churn.
yoney reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

yoney commented Sep 3, 2025

Copy link
Copy Markdown
Contributor Author

@yoney - LGTM. I think it's worth doing the work to make cProfile work well in a multi-threaded environment in the free-threaded builds. Do you have a sense of the scope of the changes that will be required to make cProfile work correctly in a multi-threaded setting? Unless they're similar in scope to these changes I think it's probably worth merging this PR (and requesting a backport to 3.14) since it shouldn't introduce too much additional churn.

@mpage Thanks for the review! I think collecting profiling data per thread and merging it lazily before reporting could work. This kind of refactoring would likely benefit subinterpreters too. I’m happy to continue working on those changes as a follow-up, but I agree that the scope would be different from this PR.

For now, this PR addresses the immediate issue that causes crashes. While the recursive-count might still be incorrect, the other counts like call-count are accurate, and this fix resolves the crash. I think it makes sense to land this now and consider the follow-up separately.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
kumaraditya303
kumaraditya303 reviewed Sep 5, 2025
View reviewed changes
Comment thread Modules/_lsprof.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@yoney
pythongh-116738: Remove the unnecessary critical_section parameter
d4d4d51
kumaraditya303
kumaraditya303 reviewed Sep 5, 2025
View reviewed changes
Comment thread Modules/_lsprof.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@yoney
pythongh-116738: Remove the critical_section for profiler_init()
42942c5
kumaraditya303
kumaraditya303 approved these changes Sep 6, 2025
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Sep 6, 2025
@kumaraditya303
Merge branch 'main' into ft_cprofile
0f0a658
kumaraditya303
kumaraditya303 reviewed Sep 6, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-08-28-09-29-46.gh-issue-116738.yLZJpV.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@kumaraditya303
Update Misc/NEWS.d/next/Core_and_Builtins/2025-08-28-09-29-46.gh-issu…
e6eeb1e 
…e-116738.yLZJpV.rst
@kumaraditya303 kumaraditya303 added the needs backport to 3.14 bugs and security fixes label Sep 6, 2025
@kumaraditya303
kumaraditya303 merged commit 8554c09 into python:main Sep 6, 2025
@miss-islington-app

miss-islington-app Bot commented Sep 6, 2025

Copy link
Copy Markdown

Thanks @yoney for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label Sep 6, 2025
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 6, 2025
@yoney @kumaraditya303 @miss-islington
pythongh-116738: make cProfile module thread-safe (pythonGH-138229)
5ba44ee 
(cherry picked from commit 8554c09)

Co-authored-by: Alper <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
@bedevere-app

bedevere-app Bot commented Sep 6, 2025

Copy link
Copy Markdown

GH-138575 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 bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Sep 6, 2025
lkollar pushed a commit to lkollar/cpython that referenced this pull request Sep 9, 2025
@yoney @kumaraditya303 @lkollar
pythongh-116738: make cProfile module thread-safe (python#138229)
3a2d60c 
Co-authored-by: Kumar Aditya <[email protected]>
@kumaraditya303 kumaraditya303 mentioned this pull request Sep 13, 2025
cProfile hangs / segfaults with 3.13t #125165
Closed
kumaraditya303 added a commit that referenced this pull request Oct 7, 2025
@miss-islington @yoney @kumaraditya303
[3.14] gh-116738: make cProfile module thread-safe (GH-138229) (#13…
4429554 
…8575)

gh-116738: make `cProfile` module thread-safe (GH-138229)
(cherry picked from commit 8554c09)

Co-authored-by: Alper <[email protected]>
Co-authored-by: Kumar Aditya <[email protected]>
@yoney
yoney deleted the ft_cprofile branch October 19, 2025 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes
@mpage mpage Awaiting requested review from mpage
@colesbury colesbury Awaiting requested review from colesbury
@Yhg1s Yhg1s Awaiting requested review from Yhg1s

Uh oh!

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

Assignees

No one assigned

Labels

topic-free-threading

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.

3 participants

@yoney @mpage @kumaraditya303
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.