-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
Implement PEP 788 #133110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
ZeroIntensity
wants to merge
18
commits into
python:main
Choose a base branch
from
ZeroIntensity:pep-788-impl
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+389
−5
Draft
Implement PEP 788 #133110
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
1828b9a
Implement interpreter state reference counting.
ZeroIntensity d0895f9
Add a test for interpreter reference counts.
ZeroIntensity 5c3ee8d
Add thread state daemon-ness that doesn't work yet.
ZeroIntensity 7b9ac59
Add untested implementation of non-daemon native threads.
ZeroIntensity 0868c15
Add a test for PyThreadState_SetDaemon().
ZeroIntensity e9ea644
Add untested implementation of Ensure()/Release() that probably doesn…
ZeroIntensity 0ebdca4
Change some comments.
ZeroIntensity 40989de
Add a test that I'm sure doesn't work.
ZeroIntensity d501f35
Use the interpreter's reference count and native thread countdown as …
ZeroIntensity c5ec89c
Fix the countdown decrement.
ZeroIntensity 4e1f599
Remove unused variable.
ZeroIntensity 3127a3f
Test for PyGILState_Ensure()
ZeroIntensity 82b5b9f
Fix the test for the new reference counting.
ZeroIntensity fda9886
Add PyInterpreterState_Lookup()
ZeroIntensity f7723c0
Fix a few bugs and add a test.
ZeroIntensity 62e9549
Add a test for PyThreadState_Ensure() across interpreters.
ZeroIntensity bc60630
Remove an artifact from old approach.
ZeroIntensity 9d8d526
Fix test from earlier semantics.
ZeroIntensity File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs to be unified with
wait_for_thread_shutdown
. Threads created from Python may spawn threads in C and vice versa.As currently written, I think you can get to
wait_for_native_shutdown()
and then have a thread spawn athreading.Thread()
that's not waited on.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I thought this might be an issue. Especially since there seems to be some inclination for a
PyThreadState_GetDaemon
function, unifying these seems like a good idea.My main concern is breakage towards people who are manually using
threading._shutdown
for whatever reason. We'd have to remove that if we treatthreading
threads as native threads (or I guess we could havethreading._shutdown
callwait_for_native_shutdown
?).