8000 gh-98627: Add an Optional Check for Extension Module Subinterpreter Compatibility by ericsnowcurrently · Pull Request #99040 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-98627: Add an Optional Check for Extension Module Subinterpreter Compatibility #99040

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 26 commits into from
Feb 16, 2023
Merged

gh-98627: Add an Optional Check for Extension Module Subinterpreter Compatibility #99040

merged 26 commits into from
Feb 16, 2023

Conversation

ericsnowcurrently
Copy link
Member
@ericsnowcurrently ericsnowcurrently commented Nov 3, 2022 •
edited
Loading

Enforcing (optionally) the restriction set by PEP 489 makes sense. Furthermore, this sets the stage for a potential restriction related to a per-interpreter GIL.

This change includes the following:

  • add tests for extension module subinterpreter compatibility
  • add _PyInterpreterConfig.check_multi_interp_extensions
  • add Py_RTFLAGS_MULTI_INTERP_EXTENSIONS
  • add _PyImport_CheckSubinterpIncompatibleExtensionAllowed()
  • fail iff the module does not implement multi-phase init and the current interpreter is configured to check

@bedevere-bot bedevere-bot mentioned this pull request Nov 3, 2022
Open
@ericsnowcurrently ericsnowcurrently marked this pull request as draft November 3, 2022 02:41
@ericsnowcurrently ericsnowcurrently force-pushed the interpreter-multi-interp-extensions-check branch from 93ea024 to 1defec3 Compare January 11, 2023 17:36
@erlend-aasland
Copy link
Contributor

FYI, converting _testinternalcapi.c to multi-phase init resolves four of the test failures.

test_configured_settings (test.test_capi.test_misc.SubinterpreterTest.test_configured_settings)
The config with which an interpreter is created corresponds ... Traceback (most recent call last):
  File "<string>", line 2, in <module>
ImportError: module _testinternalcapi does not support loading in subinterpreters
[...]

👇🏻

test_configured_settings (test.test_capi.test_misc.SubinterpreterTest.test_configured_settings)
The config with which an interpreter is created corresponds ... ok
[...]

@ericsnowcurrently ericsnowcurrently mentioned this pull request Feb 6, 2023
Merged
ericsnowcurrently added a commit that referenced this pull request Feb 6, 2023
@ericsnowcurrently
gh-59956: Partial Fix for GILState API Compatibility with Subinterpre…
132b3f8 
…ters (gh-101431)

The GILState API (PEP 311) implementation from 2003 made the assumption that only one thread state would ever be used for any given OS thread, explicitly disregarding the case of subinterpreters.  However, PyThreadState_Swap() still facilitated switching between subinterpreters, meaning the "current" thread state (holding the GIL), and the GILState thread state could end up out of sync, causing problems (including crashes).

This change addresses the issue by keeping the two in sync in PyThreadState_Swap().  I verified the fix against gh-99040.

Note that the other GILState-subinterpreter incompatibility (with autoInterpreterState) is not resolved here.

#59956
@ericsnowcurrently ericsnowcurrently marked this pull request as ready for review February 6, 2023 22:28
@ericsnowcurrently ericsnowcurrently merged commit 89ac665 into python:main Feb 16, 2023
@ericsnowcurrently ericsnowcurrently deleted the interpreter-multi-interp-extensions-check branch February 16, 2023 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@encukou encukou Awaiting requested review from encukou

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ericsnowcurrently @erlend-aasland @bedevere-bot
0