Skip to content

gh-99113: Add a check for Py_MOD_PER_INTERPRETER_GIL_SUPPORTED - #104206

Merged
ericsnowcurrently merged 9 commits into
python:mainfrom
ericsnowcurrently:own-gil-extension-check
May 6, 2023
Merged

gh-99113: Add a check for Py_MOD_PER_INTERPRETER_GIL_SUPPORTED#104206
ericsnowcurrently merged 9 commits into
python:mainfrom
ericsnowcurrently:own-gil-extension-check

Conversation

@ericsnowcurrently

@ericsnowcurrentlyericsnowcurrently commented May 5, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Py_MOD_PER_INTERPRETER_GIL_SUPPORTED is a new supported value for Py_mod_multiple_interpreters, added in gh-104205.

(This is blocked until PyInterpreterState.ceval.own_gil is added in gh-104204.)

@bedevere-botbedevere-bot mentioned this pull request May 5, 2023
Closed
@ericsnowcurrentlyericsnowcurrently mentioned this pull request May 5, 2023
Closed
3 tasks
@ericsnowcurrently
ericsnowcurrentlyforce-pushed the own-gil-extension-check branch from 9cd8840 to 78b6ae7CompareMay 5, 2023 20:19
@ericsnowcurrentlyericsnowcurrently mentioned this pull request May 5, 2023
Merged
erlend-aasland
erlend-aasland reviewed May 5, 2023
View reviewed changes
Comment threadModules/_asynciomodule.c Outdated
ericsnowcurrently added a commit that referenced this pull request May 5, 2023
@ericsnowcurrently
gh-99113: Add Py_MOD_PER_INTERPRETER_GIL_SUPPORTED (gh-104205)
a9c6e06
Here we are doing no more than adding the value for Py_mod_multiple_interpreters and using it for stdlib modules. We will start checking for it in gh-104206 (once PyInterpreterState.ceval.own_gil is added in gh-104204).
@ericsnowcurrently
ericsnowcurrentlyforce-pushed the own-gil-extension-check branch from 78b6ae7 to e2ddc27CompareMay 5, 2023 21:16
erlend-aasland
erlend-aasland approved these changes May 6, 2023
View reviewed changes

@erlend-aaslanderlend-aasland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

BTW, the branch in lines 295-300 seems lack coverage (disclaimer: I only ran test_import)

Comment threadModules/_testmultiphase.c
Comment threadLib/test/test_import/__init__.py
@ericsnowcurrently

ericsnowcurrently commented May 6, 2023

Copy link
Copy Markdown
MemberAuthor

Thanks for pointing out about the missing test coverage. I've added it. I also did likewise for the similar case where Py_mod_create is provided multiple times, which did not have test coverage either.

@erlend-aasland

erlend-aasland commented May 6, 2023

Copy link
Copy Markdown
Contributor

Thanks for pointing out about the missing test coverage. I've added it. I also did likewise for the similar case where Py_mod_create is provided multiple times, which did not have test coverage either.

Nice, thanks! I can confirm that those branches are now covered.

@ericsnowcurrently
ericsnowcurrently enabled auto-merge (squash) May 6, 2023 21:52
@ericsnowcurrently
ericsnowcurrently merged commit fff193b into python:mainMay 6, 2023
@ericsnowcurrently
ericsnowcurrently deleted the own-gil-extension-check branch May 6, 2023 21:57
jbower-fb pushed a commit to jbower-fb/cpython that referenced this pull request May 8, 2023
@ericsnowcurrently@jbower-fb
pythongh-99113: Add Py_MOD_PER_INTERPRETER_GIL_SUPPORTED (pythongh-10…
7192a46
…4205)
Here we are doing no more than adding the value for Py_mod_multiple_interpreters and using it for stdlib modules. We will start checking for it in pythongh-104206 (once PyInterpreterState.ceval.own_gil is added in pythongh-104204).
jbower-fb pushed a commit to jbower-fb/cpython that referenced this pull request May 8, 2023
@ericsnowcurrently@jbower-fb
pythongh-99113: Add a check for Py_MOD_PER_INTERPRETER_GIL_SUPPORTED (p…
fc1e63f
…ythongh-104206)
Py_MOD_PER_INTERPRETER_GIL_SUPPORTED is a new supported value for Py_mod_multiple_interpreters, added in pythongh-104205.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aaslanderlend-aaslanderlend-aasland approved these changes
@encukouencukouAwaiting 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