Skip to content

gh-138661: fix data race in PyCode_Addr2Line#138664

Merged
kumaraditya303 merged 5 commits intopython:mainfrom
kumaraditya303:code
Sep 12, 2025
Merged

gh-138661: fix data race in PyCode_Addr2Line#138664
kumaraditya303 merged 5 commits intopython:mainfrom
kumaraditya303:code

Conversation

@kumaraditya303
Copy link
Copy Markdown
Contributor

@kumaraditya303 kumaraditya303 commented Sep 8, 2025 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot mentioned this pull request Sep 8, 2025
Closed
@kumaraditya303 kumaraditya303 requested a review from mpage September 8, 2025 14:18
mpage
mpage previously requested changes Sep 8, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@mpage mpage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix looks good to me, but it looks like test_faulthandler is timing out on the free-threaded builds.

@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Sep 8, 2025

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Sep 10, 2025

The fix looks good to me, but it looks like test_faulthandler is timing out on the free-threaded builds.

faulthandler calls dump_frame() which calls PyUnstable_InterpreterFrame_GetLine() which calls PyCode_Addr2Line(). faulthandler can be triggered without holding the GIL (without having an attached thread state).

@vstinner
Copy link
Copy Markdown
Member

vstinner commented Sep 10, 2025

Maybe faulthandler should use an internal API which avoids the critical section. Such API should be added if it doesn't exist yet.

@kumaraditya303
Copy link
Copy Markdown
Contributor Author

kumaraditya303 commented Sep 11, 2025

faulthandler calls dump_frame() which calls PyUnstable_InterpreterFrame_GetLine() which calls PyCode_Addr2Line(). faulthandler can be triggered without holding the GIL (without having an attached thread state).

Thanks for the pointer, I have fixed it now by adding an internal API which would work without attached tstate.

vstinner
vstinner reviewed Sep 11, 2025
View reviewed changes
vstinner
vstinner approved these changes Sep 11, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kumaraditya303 kumaraditya303 dismissed mpage’s stale review September 11, 2025 13:29

fixed the test_faulthandler issue

@kumaraditya303 kumaraditya303 merged commit ea26f6d into python:main Sep 12, 2025
46 of 47 checks passed
@miss-islington-app
Copy link
Copy Markdown

miss-islington-app bot commented Sep 12, 2025

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 12, 2025
@kumaraditya303 @miss-islington
pythongh-138661: fix data race in PyCode_Addr2Line (pythonGH-138664)
78266bf 
(cherry picked from commit ea26f6d)

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

bedevere-app bot commented Sep 12, 2025

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

@kumaraditya303 kumaraditya303 deleted the code branch September 12, 2025 12:35
@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Sep 12, 2025
kumaraditya303 added a commit that referenced this pull request Oct 7, 2025
@miss-islington @kumaraditya303
[3.14] gh-138661: fix data race in PyCode_Addr2Line (GH-138664) (#1…
bfcd5f2 
…38834)

gh-138661: fix data race in `PyCode_Addr2Line` (GH-138664)
(cherry picked from commit ea26f6d)

Co-authored-by: Kumar Aditya <[email protected]>
@colesbury colesbury mentioned this pull request Apr 3, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

@mpage mpage mpage left review comments

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Assignees

No one assigned

Labels

skip news topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kumaraditya303 @vstinner @mpage