Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-128639: Don't assume one thread in subinterpreter finalization with fixed daemon thread support#134606
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
gh-128639: Don't assume one thread in subinterpreter finalization with fixed daemon thread support #134606
Uh oh!
There was an error while loading. Please reload this page.
Conversation
ZeroIntensity commented May 24, 2025 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
…inalization (pythongh-128640)" (pythongh-134256) This reverts commit 27bd082.
This comment was marked as outdated.
This comment was marked as outdated.
bedevere-bot commented May 24, 2025
🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit a569355 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F134606%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
ZeroIntensity commented May 25, 2025
!buildbot refleak |
bedevere-bot commented May 25, 2025
🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit f85a291 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F134606%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
ZeroIntensity commented May 26, 2025
@ericsnowcurrently Yay, buildbots passed on this one! |
ericsnowcurrently left a comment
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.
Like with gh-128640, mostly LGTM. I've left a few minor comments.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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 |
This comment has been minimized.
This comment has been minimized.
bedevere-bot commented Jun 8, 2025
🤖 New build scheduled with the buildbot fleet by @ZeroIntensity for commit 62bd653 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F134606%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
These cause crashes due to an existing bug with subinterpreters.
ZeroIntensity commented Jun 17, 2025
I have made the requested changes; please review again |
Thanks for making the requested changes! @ericsnowcurrently: please review the changes made to this pull request. |
…interp-thread-shutdown
ericsnowcurrently left a comment
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.
LGTM
a648813 into python:mainUh oh!
There was an error while loading. Please reload this page.
Thanks @ZeroIntensity for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…on with fixed daemon thread support (pythonGH-134606) This reapplies pythonGH-128640. (cherry picked from commit a648813) Co-authored-by: Peter Bierma <[email protected]>
GH-139050 is a backport of this pull request to the 3.14 branch. |
This is a reapplication of #128640, with daemon thread hanging fixed.
The key part that I was missing was this:
My fix predated these lines, so it didn't make it in originally.
The reason this only failed on the iOS buildbots was because they're single-process. Out of random choice, I picked 100 seconds as the sleep time, but since the process would exit before that 100 seconds was over on all the other buildbots, there wasn't any problem. On iOS, the test suite continued to run in the same process after the 100 seconds were up, so the daemon threads would start trying to run again under a deallocated interpreter, causing all sorts of weird crashes.