◐ Shell
reader mode source ↗
Skip to content

gh-145142: Make str.maketrans safe under free-threading#145157

Merged
colesbury merged 13 commits into
python:mainfrom
VanshAgarwal24036:gh-145142-dict-next-critical-section
Feb 27, 2026
Merged

gh-145142: Make str.maketrans safe under free-threading#145157
colesbury merged 13 commits into
python:mainfrom
VanshAgarwal24036:gh-145142-dict-next-critical-section

Conversation

@VanshAgarwal24036

@VanshAgarwal24036 VanshAgarwal24036 commented Feb 23, 2026 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

Replace unsafe PyDict_Next iteration with snapshot iteration using PyDict_Items to avoid crashes when the input dict is mutated concurrently under free-threading.

Adds a regression test.

@bedevere-app bedevere-app Bot mentioned this pull request Feb 23, 2026
Closed
colesbury
colesbury reviewed Feb 23, 2026
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

A few comments below

picnixz
picnixz requested changes Feb 24, 2026
View reviewed changes

@picnixz picnixz left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

I would suggest isolating the loop in a separate function so that we do not have those weird labels jumps and confusing names but this should be benchmarked on PGO/LTO tocheck that we do not introduce an overhead (though I am not that worried as str.maketrans() is usually not a hot function).

@bedevere-app

bedevere-app Bot commented Feb 24, 2026

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

sharktide
sharktide reviewed Feb 24, 2026
View reviewed changes

@sharktide sharktide left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

Bunch of syntax errors you need to fix as well.

I also agree with picnixz’s comments

@VanshAgarwal24036 VanshAgarwal24036 marked this pull request as draft February 24, 2026 12:44
@VanshAgarwal24036 VanshAgarwal24036 force-pushed the gh-145142-dict-next-critical-section branch from d487864 to 05ba3f3 Compare February 24, 2026 15:57
@VanshAgarwal24036 VanshAgarwal24036 marked this pull request as ready for review February 24, 2026 16:37
@VanshAgarwal24036 VanshAgarwal24036 force-pushed the gh-145142-dict-next-critical-section branch from 206f6b2 to 4e715b0 Compare February 24, 2026 17:34
@VanshAgarwal24036

VanshAgarwal24036 commented Feb 24, 2026

Copy link
Copy Markdown
Contributor Author

I have made the requested changes; please review again.

@bedevere-app

bedevere-app Bot commented Feb 24, 2026

Copy link
Copy Markdown

Thanks for making the requested changes!

@picnixz: please review the changes made to this pull request.

@bedevere-app bedevere-app Bot requested a review from picnixz February 24, 2026 17:41
colesbury
colesbury approved these changes Feb 24, 2026
View reviewed changes

@colesbury colesbury left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

LGTM, thanks

@colesbury colesbury added the needs backport to 3.14 bugs and security fixes label Feb 27, 2026
@colesbury colesbury enabled auto-merge (squash) February 27, 2026 15:41
Hide details View details @colesbury colesbury merged commit a249795 into python:main Feb 27, 2026
47 checks passed
@miss-islington-app

miss-islington-app Bot commented Feb 27, 2026

Copy link
Copy Markdown

Thanks @VanshAgarwal24036 for the PR, and @colesbury for merging it 🌮🎉.. 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 Feb 27, 2026
@VanshAgarwal24036 @miss-islington
pythongh-145142: Make str.maketrans safe under free-threading (python… …
0efb6a2 
…gh-145157)

(cherry picked from commit a249795)

Co-authored-by: VanshAgarwal24036 <148854295+VanshAgarwal24036@users.noreply.github.com>
@bedevere-app

bedevere-app Bot commented Feb 27, 2026

Copy link
Copy Markdown

GH-145320 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 Feb 27, 2026
colesbury pushed a commit that referenced this pull request Feb 27, 2026
@miss-islington @VanshAgarwal24036
[3.14] gh-145142: Make str.maketrans safe under free-threading (gh-14… …
d76c56e 
…5157) (#145320)

Co-authored-by: VanshAgarwal24036 <148854295+VanshAgarwal24036@users.noreply.github.com>
@VanshAgarwal24036 VanshAgarwal24036 deleted the gh-145142-dict-next-critical-section branch February 27, 2026 16:42
brijkapadia pushed a commit to brijkapadia/cpython that referenced this pull request Feb 28, 2026
@VanshAgarwal24036 @brijkapadia
pythongh-145142: Make str.maketrans safe under free-threading (python… …
bbcb932 
…gh-145157)
ljfp pushed a commit to ljfp/cpython that referenced this pull request Apr 25, 2026
@VanshAgarwal24036 @ljfp
pythongh-145142: Make str.maketrans safe under free-threading (python… …
1f6c471 
…gh-145157)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eendebakpt eendebakpt eendebakpt left review comments

@colesbury colesbury colesbury approved these changes

@picnixz picnixz

@sharktide sharktide Awaiting requested review from sharktide

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@VanshAgarwal24036 @colesbury @eendebakpt @picnixz @sharktide