Skip to content

gh-148037: remove critical section from PyCode_Addr2Line#148103

Merged
kumaraditya303 merged 7 commits intopython:mainfrom
kumaraditya303:instrumentation
Apr 10, 2026
Merged

gh-148037: remove critical section from PyCode_Addr2Line#148103
kumaraditya303 merged 7 commits intopython:mainfrom
kumaraditya303:instrumentation

Conversation

@kumaraditya303
Copy link
Copy Markdown
Contributor

@kumaraditya303 kumaraditya303 commented Apr 4, 2026 •
edited
Loading

@bedevere-app bedevere-app bot mentioned this pull request Apr 4, 2026
Open
@kumaraditya303 kumaraditya303 marked this pull request as ready for review April 6, 2026 19:36
colesbury
colesbury reviewed Apr 7, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

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

Two comments below.

An additional thought: in light of things like https://discuss.python.org/t/pure-c-structured-traceback/106872, maybe we should just use atomic operations everywhere for these fields (i.e., _Py_atomic instead of FT_ATOMIC) because PyCode_Addr2Lineis called from _Py_DumpTraceback, which may be called from a signal handler or without holding the GIL.

colesbury
colesbury reviewed Apr 9, 2026
View reviewed changes
colesbury
colesbury approved these changes Apr 9, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

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

LGTM

Doesn't have to be part of this PR, but I'd like to remove _PyCode_SafeAddr2Line in a follow up. The atomic loads for checking are safer w.r.t. signal handlers and we can lift the bounds checking to the two call sites.

@kumaraditya303 kumaraditya303 merged commit d3b7b93 into python:main Apr 10, 2026
58 checks passed
@kumaraditya303 kumaraditya303 deleted the instrumentation branch April 10, 2026 12:28
@kumaraditya303 kumaraditya303 added the needs backport to 3.14 bugs and security fixes label Apr 10, 2026
@miss-islington-app
Copy link
Copy Markdown

miss-islington-app bot commented Apr 10, 2026

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

@miss-islington-app
Copy link
Copy Markdown

miss-islington-app bot commented Apr 10, 2026

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

cherry_picker d3b7b93cbbbf53061a95eb60cc116c9fec31c5b4 3.14

kumaraditya303 added a commit to kumaraditya303/cpython that referenced this pull request Apr 10, 2026
@kumaraditya303
[3.14] pythongh-148037: remove critical section from `PyCode_Addr2Lin…
ab5dcd6 
…e` (pythonGH-148103)

(cherry picked from commit d3b7b93)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@bedevere-app
Copy link
Copy Markdown

bedevere-app bot commented Apr 10, 2026

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

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Apr 10, 2026
kumaraditya303 added a commit that referenced this pull request Apr 10, 2026
@kumaraditya303
[3.14] gh-148037: remove critical section from PyCode_Addr2Line (GH… (
f36da66 
#148353)

[3.14] gh-148037: remove critical section from `PyCode_Addr2Line` (GH-148103)
(cherry picked from commit d3b7b93)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@colesbury colesbury colesbury approved these changes

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

Assignees

@kumaraditya303 kumaraditya303

Labels

awaiting merge topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kumaraditya303 @colesbury