FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-126366: Make native generators thread safe by ZeroIntensity · Pull Request #126371 · python/cpython · GitHub

/ cpython Public

gh-126366: Make native generators thread safe - #126371

Draft
ZeroIntensity wants to merge 44 commits into
python:mainfrom
ZeroIntensity:list-init-thread-safety
ConversationCommits (44)ChecksFiles changed
Draft

gh-126366: Make native generators thread safe#126371
ZeroIntensity wants to merge 44 commits into
python:mainfrom
ZeroIntensity:list-init-thread-safety

Conversation

ZeroIntensity commented Nov 3, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

ZeroIntensity added the needs backport to 3.13 bugs and security fixes label Nov 3, 2024
bedevere-app Bot mentioned this pull request Nov 3, 2024
Closed

corona10 commented Nov 3, 2024

Copy link
Copy Markdown
Member

cc @colesbury

corona10 commented Nov 3, 2024

Copy link
Copy Markdown
Member

I will try to take a look at today :)

colesbury reviewed Nov 4, 2024
View reviewed changes
Comment thread Objects/listobject.c Outdated
ZeroIntensity changed the title gh-126366: Lock generic iterables in list.__init__ gh-126366: Make native generators thread safe Nov 4, 2024

ZeroIntensity commented Nov 4, 2024

Copy link
Copy Markdown
Member Author

Closing in favor of #120327

Fidget-Spinner commented Nov 6, 2024

Copy link
Copy Markdown
Member

Wait I closed my PR in favour of yours... 😆 @ZeroIntensity

ZeroIntensity commented Nov 6, 2024

Copy link
Copy Markdown
Member Author

Ah! I'll reopen this and list you as a co-author if yours has merge conflicts.

ZeroIntensity reopened this Nov 6, 2024
Fidget-Spinner reviewed Nov 6, 2024
View reviewed changes
Comment thread Python/bytecodes.c
ZeroIntensity removed the needs backport to 3.13 bugs and security fixes label Dec 24, 2024
vstinner reviewed Dec 25, 2024
View reviewed changes

vstinner left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

There is a warning:

Check warning on line 129 in Python/ceval_macros.h
‘gen_frame’ may be used uninitialized in this function [-Wmaybe-uninitialized]

vstinner reviewed Dec 25, 2024
View reviewed changes

vstinner left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The overall approach LGTM. I'm feeling more confortable with more critical sections :-)

Comment thread Objects/genobject.c Outdated
Comment thread Objects/genobject.c Outdated
Comment thread Objects/genobject.c Outdated
Comment thread Objects/genobject.c Outdated

colesbury commented Jan 2, 2025

Copy link
Copy Markdown
Contributor
  • Let's avoid critical sections in bytecodes.c. It's too easy to make these kinds of mistakes where you exit out of a critical section without ending it.
  • I don't think the generator running thread safety should use critical sections. We should use something like an atomic-compare-exchange to mark it as running

ZeroIntensity marked this pull request as draft January 2, 2025 17:55

github-actions Bot commented Apr 19, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

vstinner vstinner left review comments
colesbury colesbury left review comments
Fidget-Spinner Fidget-Spinner left review comments
corona10 Awaiting requested review from corona10
markshannon Awaiting requested review from markshannon markshannon is a code owner
mpage Awaiting requested review from mpage

Assignees

No one assigned

Labels

stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL