Skip to content

Conversation

@sergey-miryanov
Copy link
Contributor

@sergey-miryanov sergey-miryanov commented Nov 13, 2025

Fix warning - suggest using Interpreter.close instead of _interpeters.destroy.

@ZeroIntensity Could you please take a look?

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

  1. This changes a public API, so let's add a news entry.
  2. Since you didn't need to update any tests, that means that this code path is untested. Please add a test case for leaving subinterpreters unclosed.

@sergey-miryanov sergey-miryanov marked this pull request as draft November 13, 2025 19:19
@sergey-miryanov
Copy link
Contributor Author

Tests fail because of #131229

@sergey-miryanov sergey-miryanov marked this pull request as ready for review November 13, 2025 19:56
@sergey-miryanov
Copy link
Contributor Author

@ZeroIntensity Could you please take a look?

sergey-miryanov and others added 2 commits November 14, 2025 10:15
…e-141528.VWdax1.rst

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
@sergey-miryanov
Copy link
Contributor Author

Fixed. Please take a look.

Copy link
Member

@ZeroIntensity ZeroIntensity left a comment

Choose a reason for hiding this comment

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

LGTM

@ZeroIntensity ZeroIntensity added the needs backport to 3.14 bugs and security fixes label Nov 14, 2025
@sergey-miryanov
Copy link
Contributor Author

Thanks!

@ZeroIntensity ZeroIntensity enabled auto-merge (squash) November 14, 2025 11:28
@ZeroIntensity
Copy link
Member

Tests fail because of #131229

Ugh, that seems to fail consistently. I can't merge this until that job passes.

@sergey-miryanov
Copy link
Contributor Author

Yeah, I'm looking into it, but without luck yet.

@vstinner
Copy link
Member

I reran the UBSan job (where test_import was failing).

@ZeroIntensity
Copy link
Member

I did too, but it still failed. It might pass if we rerun it enough times. See #141552 for a skip.

@ZeroIntensity ZeroIntensity enabled auto-merge (squash) November 14, 2025 14:29
@ZeroIntensity ZeroIntensity merged commit fa245df into python:main Nov 14, 2025
46 checks passed
@miss-islington-app
Copy link

Thanks @sergey-miryanov for the PR, and @ZeroIntensity for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @sergey-miryanov and @ZeroIntensity, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker fa245df4a0848c15cf8d907c10fc92819994b866 3.14

@ZeroIntensity
Copy link
Member

@sergey-miryanov Could you handle the backport?

@sergey-miryanov
Copy link
Contributor Author

@ZeroIntensity Yeah, sure.

@bedevere-app
Copy link

bedevere-app bot commented Nov 14, 2025

GH-141566 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label Nov 14, 2025
sergey-miryanov added a commit to sergey-miryanov/cpython that referenced this pull request Nov 14, 2025
…ythonGH-141528)

(cherry picked from commit fa245df)

Co-authored-by: Sergey Miryanov <sergey.miryanov@gmail.com>
Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants