bpo-44050: Extension modules can share state when they don't support sub-interpreters.#27794
bpo-44050: Extension modules can share state when they don't support sub-interpreters.#27794miss-islington merged 5 commits into
Conversation
|
I don't understand this PR. It allows Python objects to be shared between interpreters, doesn't it? |
Sorry, something went wrong.
Hi, petr. The details in this PR: 82c83bd |
Sorry, something went wrong.
|
The relevant check is done by the |
Sorry, something went wrong.
Hm. I agree with you. But the user of bpo-44050 got the error in v3.9.6. And I found some module created from |
Sorry, something went wrong.
|
This PR can be removed when all the extension modules convert to multi-phase init. |
Sorry, something went wrong.
|
Why is it important to ask whether the module was created by |
Sorry, something went wrong.
That includes third-party modules, so: this PR can be removed when the API for single-phase init is removed. |
Sorry, something went wrong.
Make sure only the single extension module shared between interpreters. The extension module from multi-phase init can be created seperately in subinterpreter, right? |
Sorry, something went wrong.
|
All modules with non-negative m_size shouldn't be shared across interpreters, regardless of how they're created. |
Sorry, something went wrong.
Make sense. Looks your idea would be better. Thanks:) I will update it soon. |
Sorry, something went wrong.
|
_PyImport_FixupExtensionObject() copies the namespace from the first instance of an extension, to newly created instances of the extension. It's better than using exactly the same module object with the same dict dictionary object. In a perfect world, all extensions would use multi-phase init and so don't go through _PyImport_FixupExtensionObject(). |
Sorry, something went wrong.
vstinner
left a comment
There was a problem hiding this comment.
LGTM, but can you try to write an unit test?
Would it be possible to write an unit test for this? https://bugs.python.org/issue44050 is a regression. It would be better to not reintroduce it by mistake tomorrow.
Maybe using the ssl module: if if it's already imported, skip the test. Otherwise, spawn a subinterpreter to import it, and then import it in the main interpreter, and compare if you get the same objects or not.
See wee-slack/wee-slack#812 (comment) for a more complete example.
To make sure that the ssl module is not imported yet, you can run the test in a subprocess, eg. using assert_python_ok(), or subprocess.
Sorry, something went wrong.
OK, victor. I will update it. |
Sorry, something went wrong.
That won't do; ssl uses multi-phase init (as should all other stdlib modules, one day). This would need to be a new module dedicated for the test. |
Sorry, something went wrong.
|
Looks good, although it looks like it'll only be backported to 3.10.1. |
Sorry, something went wrong.
What do you mean? The regression is in 3.9, the issue is fixed in 3.10, and 3.10.1 doesn't exist yet. Did you mean 3.9.1? |
Sorry, something went wrong.
Maybe we cloud backport to 3.9 and 3.10? |
Sorry, something went wrong.
The "fix" in 3.10 is in the Backporting to 3.10.1 & the next 3.9 sounds fine. |
Sorry, something went wrong.
|
Thanks @shihai1991 for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10. |
Sorry, something went wrong.
|
Sorry, @shihai1991, I could not cleanly backport this to |
Sorry, something went wrong.
…sub-interpreters. (pythonGH-27794) Automerge-Triggered-By: GH:encukou (cherry picked from commit b9bb748) Co-authored-by: Hai Shi <shihai1992@gmail.com>
|
Thanks Petr, Victor for your review and merge. |
Sorry, something went wrong.
…pport sub-interpreters. (pythonGH-27794) Automerge-Triggered-By: GH:encukou. (cherry picked from commit b9bb748) Co-authored-by: Hai Shi <shihai1992@gmail.com>
|
And thanks for your patience; this certainly waited for longer than it should have. |
Sorry, something went wrong.
https://bugs.python.org/issue44050
Automerge-Triggered-By: GH:encukou