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-134160: Improve multi-phase init note on isolation & subinterpreters#134775

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

Merged
encukou merged 4 commits intopython:mainfromencukou:multiphase-init-doc
May 30, 2025

Conversation

encukou
Copy link
Member

@encukouencukou commentedMay 27, 2025
edited by AA-Turner
Loading

(This isn't strictly related to#134160, but I think it's more useful to group this PR with the other ones there.)


📚 Documentation preview 📚:https://cpython-previews--134775.org.readthedocs.build/en/134775/c-api/module.html

Comment on lines 309 to 311
- :ref:`raising an error on repeated initialization <isolating-extensions-optout>`, or
- limiting a module to the main interpreter using
:c:data:`Py_mod_multiple_interpreters`.
Copy link
Member

Choose a reason for hiding this comment

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

It's a bit weird to present this as a way of supporting sub-interpreters, no?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Yeah. The original wording from PEP 489 would work a bit better:

All modules created using multi-phase initialization are expected to support:ref:`sub-interpreters<sub-interpreter-support>` and multiple:c:func:`Py_Initialize`/:c:func:`Py_Finalize` cycles correctly.Typically, extensions ensure this in one of these ways:

or should we expand the “correctly“ to something like “work as expected or raise exceptions, rather than crash”?

Copy link
Member

Choose a reason for hiding this comment

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

I would definitely not call this "support". So perhaps something like:

Modules created using multi-phase initialization are expected to handlesub-interpreters in either of the following two ways:* either they fully support multiple interpreters by:ref:`isolating module instances<isolating-extensions-howto>`;* or they signal that multiple interpreters are not supported by:ref:`raising an error on repeated initialization<isolating-extensions-optout>`  and/or limiting a module to the main interpreter using:c:data:`Py_mod_multiple_interpreters`.This way, users will not encounter crashes or undefined behavior whentrying to use these modules from multiple interpreters.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I guess right thing would be to take step back and not make this so much about subinterpreters:

Multi-phase initialization allows creating multiple independent instances of themodule object.Extensions are expected to handle this in one of the following ways:* either by:ref:`isolating module instances<isolating-extensions-howto>`;* and/or by:ref:`raising an error on repeated initialization<isolating-extensions-optout>`;* and/or limiting a module to the main interpreter using:c:data:`Py_mod_multiple_interpreters`.This way, users will not encounter crashes or undefined behavior whentrying to use these modules from:ref:`sub-interpreters<sub-interpreter-support>`.The first two ways also prevent crashes when reusing the module afterPython runtime reinitialization (:c:func:`Py_Finalize` and:c:func:`Py_Initialize`)and reimporting a module after it is removed from:py:attr:`sys.modules`.

Thanks for suggesting the effects on the users! It helps to make this less of athou shalt commandment and more like something where you can consider the trade-offs.

Copy link
Member

Choose a reason for hiding this comment

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

This makes me wonder: why is there a third way if the second is better (and not more difficult to implement)?

Choose a reason for hiding this comment

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

Not that it is critical for this PR, but I've been thinking about something related that might at least a little insight:gh-132861.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

@pitrou: Let's say there's subtle nuance that doesn't really belong on this page of the docs :)
It would be good to explain it in the HOWTO, of course.

AFAIK, the main interpreter is special; sometimes, being loadableonce but inany interpreter isn't enough.

Copy link
Member

Choose a reason for hiding this comment

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

The extant prior paragraph covers much of this, so here's an alternative:

When using per-module state, module instances should be:ref:`isolated<isolating-extensions-howto>` to ensure that they are free of unwantedside-effects. If the module still uses global state, or it otherwise has notyet been isolated, either or both of of the following two options should be used:*:ref:`raising an error on repeated initialization<isolating-extensions-optout>`* limiting a module to the main interpreter using:c:data:`Py_mod_multiple_interpreters`.This way, users will not encounter crashes or undefined behavior whentrying to use these modules from:ref:`sub-interpreters<sub-interpreter-support>`.Ensuring a module is properly isolated or raising an explicit error also preventcrashes when reusing the module after Python runtime reinitialization(:c:func:`Py_Finalize` and:c:func:`Py_Initialize`) or reimporting a module afterit has been removed from:py:attr:`sys.modules`.

Typically, extensions ensure this in one of these ways:

- :ref:`isolating module instances <isolating-extensions-howto>`,
- :ref:`raising an error on repeated initialization <isolating-extensions-optout>`, or

Choose a reason for hiding this comment

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

FWIW, I just noticed that the referenced section does not mention opting out by setting thePy_mod_multiple_interpreters slot toPy_MOD_MULTIPLE_INTERPRETERS_NOT_SUPPORTED. (I didn't think to update the docs when I added that value.) Fixing that doesn't need to be part of this PR, but it is relevant to this bullet (and the third bullet).

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

(Please read this in a friendly voice; I can't find a way to reword this so it's clear and doesn't have a terrible sarcastic interpretation.)

Yeah. I'm not a good person to write that, as I never fully understood why a slot is better than explicitly raising an exception when you're about to clobber shared state.
It would be good to record that in the HOWTO, could you do it?

ericsnowcurrently reacted with thumbs up emoji
@@ -302,8 +302,13 @@ using :c:func:`PyModule_GetState`), or its contents (such as the module's
:attr:`~object.__dict__` or individual classes created with :c:func:`PyType_FromSpec`).

All modules created using multi-phase initialization are expected to support
:ref:`sub-interpreters <sub-interpreter-support>`. Making sure multiple modules
are independent is typically enough to achieve this.
:ref:`sub-interpreters <sub-interpreter-support>`.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
:ref:`sub-interpreters<sub-interpreter-support>`.
:ref:`sub-interpreters<sub-interpreter-support>`, or otherwise explicitly
signal a lack of support.

Comment on lines 309 to 311
- :ref:`raising an error on repeated initialization <isolating-extensions-optout>`, or
- limiting a module to the main interpreter using
:c:data:`Py_mod_multiple_interpreters`.
Copy link
Member

Choose a reason for hiding this comment

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

The extant prior paragraph covers much of this, so here's an alternative:

When using per-module state, module instances should be:ref:`isolated<isolating-extensions-howto>` to ensure that they are free of unwantedside-effects. If the module still uses global state, or it otherwise has notyet been isolated, either or both of of the following two options should be used:*:ref:`raising an error on repeated initialization<isolating-extensions-optout>`* limiting a module to the main interpreter using:c:data:`Py_mod_multiple_interpreters`.This way, users will not encounter crashes or undefined behavior whentrying to use these modules from:ref:`sub-interpreters<sub-interpreter-support>`.Ensuring a module is properly isolated or raising an explicit error also preventcrashes when reusing the module after Python runtime reinitialization(:c:func:`Py_Finalize` and:c:func:`Py_Initialize`) or reimporting a module afterit has been removed from:py:attr:`sys.modules`.

@encukou
Copy link
MemberAuthor

I integrated this into the existing section. It's a bigger change now, but hopefully for the better :)

@ericsnowcurrently, I'm specifically interested in suggestions to makePy_mod_multiple_interpreters look more important.

Copy link
Member

@AA-TurnerAA-Turner left a comment

Choose a reason for hiding this comment

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

Wordsmithing:

encukouand others added2 commitsMay 28, 2025 09:46
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@encukouencukou changed the titlegh-134160: Improve note on multi-phase init & subinterpretersgh-134160: Improve multi-phase init note on isolation & subinterpretersMay 28, 2025
@AA-Turner
Copy link
Member

@ericsnowcurrently, I'm specifically interested in suggestions to makePy_mod_multiple_interpreters look more important.

Is this a blocker for merging this, or can we follow up later?

@encukou
Copy link
MemberAuthor

@ericsnowcurrently@pitrou, does this look good?
I plan to merge this on Friday if I don't hear back. Shout if you need more review time.

AA-Turner reacted with thumbs up emoji

@encukouencukou merged commiteb145fa intopython:mainMay 30, 2025
24 checks passed
@github-project-automationgithub-project-automationbot moved this fromTodo toDone inDocs PRsMay 30, 2025
@encukouencukou deleted the multiphase-init-doc branchMay 30, 2025 14:27
@encukouencukou added the needs backport to 3.14bugs and security fixes labelMay 30, 2025
@miss-islington-app
Copy link

Thanks@encukou for the PR 🌮🎉.. 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 30, 2025
…rpreters (pythonGH-134775)(cherry picked from commiteb145fa)Co-authored-by: Petr Viktorin <encukou@gmail.com>Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@bedevere-app
Copy link

GH-134932 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 30, 2025
AA-Turner added a commit that referenced this pull requestMay 31, 2025
…erpreters (GH-134775) (#134932)gh-134160: Improve multi-phase init note on isolation & subinterpreters (GH-134775)(cherry picked from commiteb145fa)Co-authored-by: Petr Viktorin <encukou@gmail.com>Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@AA-TurnerAA-Turner added the needs backport to 3.13bugs and security fixes labelMay 31, 2025
@miss-islington-app
Copy link

Thanks@encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestMay 31, 2025
…rpreters (pythonGH-134775)(cherry picked from commiteb145fa)Co-authored-by: Petr Viktorin <encukou@gmail.com>Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@bedevere-app
Copy link

GH-134983 is a backport of this pull request to the3.13 branch.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@ericsnowcurrentlyericsnowcurrentlyericsnowcurrently left review comments

@pitroupitroupitrou left review comments

@AA-TurnerAA-TurnerAA-Turner approved these changes

Assignees
No one assigned
Labels
docsDocumentation in the Doc dirskip news
Projects
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@encukou@AA-Turner@ericsnowcurrently@pitrou

[8]ページ先頭

©2009-2025 Movatter.jp