Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

gh-126366: Make native generators thread safe #126371

Draft
wants to merge 44 commits into
base: main
Choose a base branch
from

Conversation

ZeroIntensity
Copy link
Member

@ZeroIntensity ZeroIntensity commented Nov 3, 2024

@corona10
Copy link
Member

corona10 commented Nov 3, 2024

cc @colesbury

@corona10
Copy link
Member

corona10 commented Nov 3, 2024

I will try to take a look at today :)

Objects/listobject.c Outdated Show resolved Hide resolved
@ZeroIntensity ZeroIntensity changed the title gh-126366: Lock generic iterables in list.__init__ gh-126366: Make native generators thread safe Nov 4, 2024
@ZeroIntensity
Copy link
Member Author

Closing in favor of #120327

@Fidget-Spinner
Copy link
Member

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

@ZeroIntensity
Copy link
Member Author

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

@ZeroIntensity ZeroIntensity reopened this Nov 6, 2024
@ZeroIntensity ZeroIntensity removed the needs backport to 3.13 bugs and security fixes label Dec 24, 2024
Copy link
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.

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]

Copy link
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.

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

Objects/genobject.c Outdated Show resolved Hide resolved
Objects/genobject.c Outdated Show resolved Hide resolved
Objects/genobject.c Outdated Show resolved Hide resolved
Objects/genobject.c Outdated Show resolved Hide resolved
@colesbury
Copy link
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 ZeroIntensity marked this pull request as draft January 2, 2025 17:55
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants