GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — gh-117953: Always Run Extension Init Func in Main Interpreter First by ericsnowcurrently · Pull Request #118157 · python/cpython

Conversation

ericsnowcurrently commented Apr 22, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

This change makes sure all extension/builtin modules have their init function run first by the main interpreter before proceeding with import in the original interpreter (main or otherwise). This means when the import of a single-phase init module fails in an isolated subinterpreter, it won't tie any global state/callbacks to the subinterpreter.

This supercedes gh-117487.

Issue: Single-Phase Init Extension Module Init Functions Still Run in Isolated Interpreters #117953

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-app Bot mentioned this pull request

Apr 22, 2024

Single-Phase Init Extension Module Init Functions Still Run in Isolated Interpreters #117953

Closed

ericsnowcurrently added 5 commits

May 6, 2024 09:50


          Load extension modules under the main interpreter first.

a1a3b4c


          Tweak a comment.

9d630f1


          Do not switch if already on the main interpreter.

2d7ee11


          maybe_switch_to_main_interpreter() -> switch_to_main_interpreter()

f9e132f


          Update the global cache while still under the main interpreter.

b09c445

ericsnowcurrently force-pushed the load-ext-in-main-interp-2 branch from 8d04512 to b09c445 Compare

May 6, 2024 16:48

ericsnowcurrently added 2 commits

May 6, 2024 10:58


          Add a NEWS entry.

8ca8033


          Fix the main_tstate checks.

122761c

ericsnowcurrently marked this pull request as ready for review

May 6, 2024 17:43

ericsnowcurrently requested review from brettcannon, kumaraditya303, ncoghlan and warsaw as code owners

May 6, 2024 17:43

bedevere-app Bot added the awaiting core review label

May 6, 2024

ericsnowcurrently requested review from zooba and removed request for brettcannon, kumaraditya303, ncoghlan and warsaw

May 6, 2024 17:43

ericsnowcurrently commented May 6, 2024

Copy link

Copy Markdown

Member Author

FYI, there's a refleak somewhere in here that I'm tracking down.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ericsnowcurrently added 3 commits

May 6, 2024 12:43


          Switch back if there is an exception.

bb3402a


          Cache the module only on success.

164a9eb


          Add a missing assert.

0c74b25

zooba reviewed

May 6, 2024

View reviewed changes

zooba left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Nothing jumps out at me looking like a refleak, unfortunately. Possible the first import is leaking something in the main interpreter and it's not being counted right in the subinterpreter? (Or vice versa)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Python/import.c Outdated

		* and then continue loading like normal. */

		PyThreadState *main_tstate = NULL;
		if (!_Py_IsMainInterpreter(tstate->interp)) {

zooba May 6, 2024

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Is this necessary if switch_to_main_interpreter is doing the same check?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ericsnowcurrently May 6, 2024

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It isn't necessary, but I wanted to be explicit about it. I'll see about cleaning that up.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ericsnowcurrently May 6, 2024

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Python/import.c Outdated

		@@ -1941,15 +2005,73 @@ import_run_extension(PyThreadState *tstate, PyModInitFunction p0,
		if (cached == NULL) {
		goto error;

zooba May 6, 2024

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can we do this without switching back?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ericsnowcurrently May 6, 2024

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yeah, I noticed that too and have already fixed it locally. 😄

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ericsnowcurrently May 6, 2024

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ericsnowcurrently added 5 commits

May 6, 2024 13:44


          Clean up the temprary thread state.

7b9a29d


          Do not do an extra _Py_IsMainInterpreter() check.

7965d57


          Drop the superfluous SinglephaseInitTests.tearDownClass().

2abce96


          Work around a refleak.

0b16f3f


          Skip some tests when the GIL is disabled.

f7d0ff9

ericsnowcurrently added 3 commits

May 6, 2024 21:20


          Merge branch 'main' into load-ext-in-main-interp-2

f3b015f


          Fix the tests.

048342f


          Disable all interpreters tests under Py_GIL_DISABLED.

6d5311e

ericsnowcurrently enabled auto-merge (squash)

May 7, 2024 03:52

ericsnowcurrently merged commit b2cd54a into python:main

May 7, 2024

bedevere-app Bot removed the awaiting core review label

May 7, 2024

ericsnowcurrently deleted the load-ext-in-main-interp-2 branch

May 7, 2024 04:22

This was referenced May 7, 2024

"thread sanitizer" (tsan) CI job is hanging #118729

Closed

test_interpreters Failing Intermittently Under Py_DISABLE_GIL #118731

Open

__file__ for Some Dynamic Extension Modules has a Broken Refcount #118733

Open

SonicField pushed a commit to SonicField/cpython that referenced this pull request

May 8, 2024


          pythongh-117953: Always Run Extension Init Func in Main Interpreter F…

c1d5643

…irst (pythongh-118157)

This change makes sure all extension/builtin modules have their init function run first by the main interpreter before proceeding with import in the original interpreter (main or otherwise).  This means when the import of a single-phase init module fails in an isolated subinterpreter, it won't tie any global state/callbacks to the subinterpreter.

aisk mentioned this pull request

Jun 10, 2024

Legacy-mode subinterpreters in Python 3.12: import _tkinter leads to shutdown crash #115649

Closed

nineteendo mentioned this pull request

Jun 18, 2024

gh-117953: Skip test_interpreters properly without GIL #120689

Merged

ericsnowcurrently mentioned this pull request

Jul 8, 2024

gh-121110: Fix Extension Module Tests Under Py_TRACE_REFS Builds #121503

Merged

ericsnowcurrently added a commit that referenced this pull request

Jul 8, 2024


          gh-121110: Fix Extension Module Tests Under Py_TRACE_REFS Builds (gh-…

15d48ae

…121503)

The change in gh-118157 (b2cd54a) should have also updated clear_singlephase_extension() but didn't.  We fix that here.  Note that clear_singlephase_extension() (AKA _PyImport_ClearExtension()) is only used in tests.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Jul 8, 2024


          pythongh-121110: Fix Extension Module Tests Under Py_TRACE_REFS Builds (

b52e053

pythongh-121503)

The change in pythongh-118157 (b2cd54a) should have also updated clear_singlephase_extension() but didn't.  We fix that here.  Note that clear_singlephase_extension() (AKA _PyImport_ClearExtension()) is only used in tests.
(cherry picked from commit 15d48ae)

Co-authored-by: Eric Snow <[email protected]>

miss-islington mentioned this pull request

Jul 8, 2024

[3.13] gh-121110: Fix Extension Module Tests Under Py_TRACE_REFS Builds (gh-121503) #121517

Merged

ericsnowcurrently added a commit that referenced this pull request

Jul 8, 2024


          [3.13] gh-121110: Fix Extension Module Tests Under Py_TRACE_REFS Buil…

c1891e2

…ds (gh-121517)

The change in gh-118157 (b2cd54a) should have also updated clear_singlephase_extension() but didn't.  We fix that here.  Note that clear_singlephase_extension() (AKA _PyImport_ClearExtension()) is only used in tests.

(cherry picked from commit 15d48ae, AKA gh-121503)

Co-authored-by: Eric Snow <[email protected]>

estyxx pushed a commit to estyxx/cpython that referenced this pull request

Jul 17, 2024


          pythongh-121110: Fix Extension Module Tests Under Py_TRACE_REFS Builds (

2b6968f

pythongh-121503)

The change in pythongh-118157 (b2cd54a) should have also updated clear_singlephase_extension() but didn't.  We fix that here.  Note that clear_singlephase_extension() (AKA _PyImport_ClearExtension()) is only used in tests.

marcan mentioned this pull request

Aug 22, 2025

Python 3.13 breaks circular imports through single-phase extension modules using non-isolated subinterpreters #138045

Closed

Reviewers

zooba

zooba left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

ericsnowcurrently commented Apr 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericsnowcurrently commented May 6, 2024

Uh oh!

zooba left a comment

Choose a reason for hiding this comment

Uh oh!

zooba May 6, 2024

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrently May 6, 2024

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrently May 6, 2024

Choose a reason for hiding this comment

Uh oh!

zooba May 6, 2024

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrently May 6, 2024

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrently May 6, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ericsnowcurrently commented Apr 22, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.