GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-134160: Improve multi-phase init note on isolation & subinterpreters by encukou · Pull Request #134775 · python/cpython Conversation
|
| - :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`. |
There was a problem 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?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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”?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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 handle sub-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 when trying to use these modules from multiple interpreters.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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 the module 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 when trying to use these modules from :ref:`sub-interpreters <sub-interpreter-support>`. The first two ways also prevent crashes when reusing the module after Python 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 a thou shalt commandment and more like something where you can consider the trade-offs.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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)?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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 loadable once but in any interpreter isn't enough.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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 unwanted side-effects. If the module still uses global state, or it otherwise has not yet 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 when trying to use these modules from :ref:`sub-interpreters <sub-interpreter-support>`. Ensuring a module is properly isolated or raising an explicit error also prevent crashes when reusing the module after Python runtime reinitialization (:c:func:`Py_Finalize` and :c:func:`Py_Initialize`) or reimporting a module after it has been removed from :py:attr:`sys.modules`.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| 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 |
There was a problem 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 the Py_mod_multiple_interpreters slot to Py_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).
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| 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>`. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
| :ref:`sub-interpreters <sub-interpreter-support>`. | |
| :ref:`sub-interpreters <sub-interpreter-support>`, or otherwise explicitly | |
| signal a lack of support. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| - :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`. |
There was a problem 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 unwanted side-effects. If the module still uses global state, or it otherwise has not yet 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 when trying to use these modules from :ref:`sub-interpreters <sub-interpreter-support>`. Ensuring a module is properly isolated or raising an explicit error also prevent crashes when reusing the module after Python runtime reinitialization (:c:func:`Py_Finalize` and :c:func:`Py_Initialize`) or reimporting a module after it has been removed from :py:attr:`sys.modules`.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 make |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Wordsmithing:
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Co-authored-by: Adam Turner <[email protected]>
Co-authored-by: Adam Turner <[email protected]>
Is this a blocker for merging this, or can we follow up later? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@ericsnowcurrently @pitrou, does this look good? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…rpreters (pythonGH-134775) (cherry picked from commit eb145fa) Co-authored-by: Petr Viktorin <[email protected]> Co-authored-by: Adam Turner <[email protected]>
|
GH-134932 is a backport of this pull request to the 3.14 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…erpreters (GH-134775) (#134932) gh-134160: Improve multi-phase init note on isolation & subinterpreters (GH-134775) (cherry picked from commit eb145fa) Co-authored-by: Petr Viktorin <[email protected]> Co-authored-by: Adam Turner <[email protected]>
|
Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…rpreters (pythonGH-134775) (cherry picked from commit eb145fa) Co-authored-by: Petr Viktorin <[email protected]> Co-authored-by: Adam Turner <[email protected]>
|
GH-134983 is a backport of this pull request to the 3.13 branch. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…erpreters (GH-134775) (GH-134983) (cherry picked from commit eb145fa) Co-authored-by: Petr Viktorin <[email protected]> Co-authored-by: Adam Turner <[email protected]>
…rpreters (pythonGH-134775) Co-authored-by: Adam Turner <[email protected]>
…rpreters (pythonGH-134775) Co-authored-by: Adam Turner <[email protected]>
…rpreters (pythonGH-134775) Co-authored-by: Adam Turner <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.