Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-128639: Don't assume one thread in subinterpreter finalization#128640

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

ZeroIntensity
Copy link
Member

@ZeroIntensityZeroIntensity commentedJan 8, 2025•
edited by bedevere-appbot
Loading

Incidentally, this also fixed the warning not showing up if a subinterpreter wasn't cleaned up via_interpreters.destroy. I had to update some of the tests as a result.

Py_NewInterpreter();
Py_NewInterpreter();
Py_NewInterpreter();
PyThreadState *tstate = PyThreadState_Get();
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I think this test working before was a fluke.Py_NewInterpreter changes the thread state, and I don't think we support callingPy_Finalize from a subinterpreter.

Choose a reason for hiding this comment

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

It's probably okay but your change helps future-proof the test.

@vstinner
Copy link
Member

cc@ericsnowcurrently

Copy link
Member

@ericsnowcurrentlyericsnowcurrently left a comment

Choose a reason for hiding this comment

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

mostly LGTM

There are a few things that might make more sense in a separate PR.

Py_NewInterpreter();
Py_NewInterpreter();
Py_NewInterpreter();
PyThreadState *tstate = PyThreadState_Get();

Choose a reason for hiding this comment

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

It's probably okay but your change helps future-proof the test.

@bedevere-app
Copy link

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 phraseI 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.

ZeroIntensityand others added2 commitsMay 19, 2025 11:16
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
Copy link
Member

@ericsnowcurrentlyericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks for doing this!

@ericsnowcurrentlyericsnowcurrently merged commit9859791 intopython:mainMay 19, 2025
45 of 46 checks passed
@ericsnowcurrently
Copy link
Member

@hugovk, is this something you'd be okay with us backporting to 3.14?

@ZeroIntensityZeroIntensity deleted the subinterpreter-thread-shutdown branchMay 19, 2025 16:24
@hugovk
Copy link
Member

Yep, crash bugfixes can be backported :)

ZeroIntensity reacted with heart emoji

@ZeroIntensityZeroIntensity added the needs backport to 3.14bugs and security fixes labelMay 19, 2025
@miss-islington-app
Copy link

Thanks@ZeroIntensity for the PR, and@ericsnowcurrently 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 requestMay 19, 2025
…on (pythongh-128640)Incidentally, this also fixed the warning not showing up if a subinterpreter wasn'tcleaned up via _interpreters.destroy. I had to update some of the tests as a result.(cherry picked from commit9859791)Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
@bedevere-app
Copy link

GH-134254 is a backport of this pull request to the3.14 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.14bugs and security fixes labelMay 19, 2025
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotiOS ARM64 Simulator 3.x (tier-3) has failed when building commit9859791.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1380/builds/3740) and take a look at the build logs.
  4. Check if the failure is related to this commit (9859791) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1380/builds/3740

Summary of the results of the build (if available):

Click to see traceback logs
remote:Enumerating objects: 32, done.remote:Counting objects:   3% (1/32)remote:Counting objects:   6% (2/32)remote:Counting objects:   9% (3/32)remote:Counting objects:  12% (4/32)remote:Counting objects:  15% (5/32)remote:Counting objects:  18% (6/32)remote:Counting objects:  21% (7/32)remote:Counting objects:  25% (8/32)remote:Counting objects:  28% (9/32)remote:Counting objects:  31% (10/32)remote:Counting objects:  34% (11/32)remote:Counting objects:  37% (12/32)remote:Counting objects:  40% (13/32)remote:Counting objects:  43% (14/32)remote:Counting objects:  46% (15/32)remote:Counting objects:  50% (16/32)remote:Counting objects:  53% (17/32)remote:Counting objects:  56% (18/32)remote:Counting objects:  59% (19/32)remote:Counting objects:  62% (20/32)remote:Counting objects:  65% (21/32)remote:Counting objects:  68% (22/32)remote:Counting objects:  71% (23/32)remote:Counting objects:  75% (24/32)remote:Counting objects:  78% (25/32)remote:Counting objects:  81% (26/32)remote:Counting objects:  84% (27/32)remote:Counting objects:  87% (28/32)remote:Counting objects:  90% (29/32)remote:Counting objects:  93% (30/32)remote:Counting objects:  96% (31/32)remote:Counting objects: 100% (32/32)remote:Counting objects: 100% (32/32), done.remote:Compressing objects:   6% (1/16)remote:Compressing objects:  12% (2/16)remote:Compressing objects:  18% (3/16)remote:Compressing objects:  25% (4/16)remote:Compressing objects:  31% (5/16)remote:Compressing objects:  37% (6/16)remote:Compressing objects:  43% (7/16)remote:Compressing objects:  50% (8/16)remote:Compressing objects:  56% (9/16)remote:Compressing objects:  62% (10/16)remote:Compressing objects:  68% (11/16)remote:Compressing objects:  75% (12/16)remote:Compressing objects:  81% (13/16)remote:Compressing objects:  87% (14/16)remote:Compressing objects:  93% (15/16)remote:Compressing objects: 100% (16/16)remote:Compressing objects: 100% (16/16), done.remote:Total 17 (delta 15), reused 2 (delta 1), pack-reused 0 (from 0)From https://github.com/python/cpython * branch                    main       -> FETCH_HEADNote:switching to '9859791f9e116c827468f307ac0770286c975c8b'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by switching back to a branch.If you want to create a new branch to retain commits you create, you maydo so (now or later) by using -c with the switch command. Example:  git switch -c <new-branch-name>Or undo this operation with:  git switch -Turn off this advice by setting config variable advice.detachedHead to falseHEAD is now at 9859791f9e1 gh-128639: Don't assume one thread in subinterpreter finalization (gh-128640)Switched to and reset branch 'main'configure:WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)configure:WARNING: pkg-config is missing. Some dependencies may not be detected correctly.configure:WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)configure:WARNING: pkg-config is missing. Some dependencies may not be detected correctly.make:*** [testios] Error 65

ZeroIntensity added a commit to ZeroIntensity/cpython that referenced this pull requestMay 19, 2025
ericsnowcurrently pushed a commit that referenced this pull requestMay 19, 2025
…tion (gh-128640)" (gh-134256)This reverts commit9859791.The original change broke the iOS and android buildbots, where the tests are run single-process.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@sergey-miryanovsergey-miryanovsergey-miryanov left review comments

@ericsnowcurrentlyericsnowcurrentlyericsnowcurrently approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@ZeroIntensity@vstinner@ericsnowcurrently@hugovk@bedevere-bot@sergey-miryanov
[8]ページ先頭
©2009-2025 Movatter.jp