Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

gh-146302: make Py_IsInitialized() thread-safe and reflect true init completion#146303

Open
gpshead wants to merge 7 commits intopython:mainfrom
gpshead:gh-146302-py-isinitialized-thread-safe
Open

gh-146302: make Py_IsInitialized() thread-safe and reflect true init completion#146303
gpshead wants to merge 7 commits intopython:mainfrom
gpshead:gh-146302-py-isinitialized-thread-safe

Conversation

@gpshead
Copy link
Member

@gpsheadgpshead commentedMar 22, 2026
edited
Loading

Summary

  • Move theruntime->initialized = 1 store from beforesite.py import to the end ofinit_interp_main(), soPy_IsInitialized() only returns true after initialization has fully completed
  • Accessinitialized andcore_initialized through new inline accessors using acquire/release atomics, to also protect from data race undefined behavior
  • PySys_AddAuditHook() now uses the accessor, so with the flag move it correctly skips audit hook invocation during all init phases (matching the documented "after runtime initialization" behavior) ... We could argue that running these earlier would be good even if the intent was never explicitly expressed, but that'd be its own issue.

Motivation

Py_IsInitialized() returned 1 whilePy_InitializeEx() was still running — specifically, beforesite.py had been imported. SeePyO3/pyo3#5900 where a second thread could acquire the GIL and start executing Python with an incompletesys.path becausesite.py hadn't finished.

The flag was also a plainint with no atomic operations, making concurrent reads a C-standard data race, though unlikely to manifest.

Regression test:

The added test properly fails onmain withERROR: Py_IsInitialized() was true during site import.

Fixes#146302


📚 Documentation preview 📚:https://cpython-previews--146303.org.readthedocs.build/

alex reacted with heart emoji
… init completionPy_IsInitialized() previously returned 1 before Py_InitializeEx() hadfinished, because the runtime flag was set before site.py was imported.This caused a race in embedders like PyO3 where a second thread couldobserve an initialized interpreter before sys.path was fully configured.Move the initialized=1 store to the end of init_interp_main(), aftersite import, lazy imports, tier 2 optimizer, and dict watcher setup.Access both `initialized` and `core_initialized` through new inlineaccessors that use acquire/release atomics, eliminating the C-standarddata race and ensuring correct visibility on weakly-orderedarchitectures.Fix PySys_AddAuditHook() to check core_initialized (not initialized)when deciding whether a thread state is available, since tstate existsafter core init completes.Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines +413 to +416
.. versionchanged:: next
This function no longer returns true until initialization has fully
completed, including import of the :mod:`site` module. Previously it
could return true while :c:func:`Py_Initialize` was still running.
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

this is technically a bug fix, we don't always call these out in the main docs. i'm happy dropping this, but it feels worthwhile to mention for embedders likely to use this API. and if we choose to backport this as a bugfix it'll include proper 3.14.x and 3.13.x attribution to indicate when people need to work around it.

Copy link
Member

Choose a reason for hiding this comment

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

It is fine to add versionchanged when an important behavior needed to be pointed out in a bugfix release so I don't mind keeping this here.

Copy link
Member

Choose a reason for hiding this comment

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

However considering what it changed I think it is best to keep it in 3.15 only.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Agreed, I'm fine not backporting this. It's rare enough and PyO3 already has a workaround.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
}
Py_XDECREF(warnoptions);

interp->runtime->initialized=1;
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Moving this down after site and the rest of the machinery is the part I'm most interested in seeing tested. Intuitively I would not expect any code site may import to ever care about Py_IsInitialized() itself as extension module code "should" never have a reason to use that API as it shouldn't be called outside of an interpreter.

But this is the one thing in this change that might be an observable behavior change. Even if we don't agree with someone relying on the existing behavior.

gpsheadand others added2 commitsMarch 22, 2026 11:42
The docs say audit hooks are notified "after runtime initialization".With the flag move, initialized=0 throughout init (including siteimport), so hooks are correctly not invoked during any init phase.Using core_initialized would have expanded the invocation windowbeyond the original behavior.Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use a C audit hook registered before Py_Initialize() to observe thevalue of Py_IsInitialized() when the "import" event fires for "site".With the old flag position this would have returned 1; afterpythongh-146302it correctly returns 0.Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@gpsheadgpshead marked this pull request as ready for reviewMarch 22, 2026 19:19
@gpsheadgpsheadforce-pushed thegh-146302-py-isinitialized-thread-safe branch from1cf04e0 todafd62eCompareMarch 22, 2026 19:23
Use _Py_IsCoreInitialized() in preconfig.c and Py_IsInitialized() inPy_InitializeEx(), removing the unnecessary runtime local variable.Thanks picnixz!
@gpsheadgpsheadforce-pushed thegh-146302-py-isinitialized-thread-safe branch fromdafd62e toba28b37CompareMarch 22, 2026 19:28
Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

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

OOC there are lots of boolean flags that are still bare ints. Are they all protected against races or?

@gpshead
Copy link
MemberAuthor

gpshead commentedMar 22, 2026
edited
Loading

OOC there are lots of boolean flags that are still bare ints. Are they all protected against races or?

probably not technically... I doubt the data race potential honestly exposes itself much in this area in practice as filling these in happens so much sooner than most anything that could be consuming them that all memory writes long since land. Py_IsInitialized() being a reader intended for concurrent access feels like an uncommon case. But I didn't try to analyze the others.

asking Opus 4.6 to analyze their use for races (my summary):_initialized,preinitializing, andpreinitialized are all fine as used. They're serial during single threaded early internal init phases.Py_FatalError could wind up reading them for informational purposes but accuracy isn't important in that scenario.

[which makes sense to me]

picnixz reacted with thumbs up emoji

@gpsheadgpshead added the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelMar 22, 2026
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by@gpshead for commitabae231 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F146303%2Fmerge

If you want to schedule another build, you need to add the🔨 test-with-buildbots label again.

@bedevere-botbedevere-bot removed the 🔨 test-with-buildbotsTest PR w/ buildbots; report in status section labelMar 22, 2026
@davidhewitt
Copy link
Contributor

davidhewitt commentedMar 22, 2026
edited
Loading

Thanks for doing this for PyO3's sake. Does thread-safe here mean that PyO3 could in principle freely callPy_Initialize from multiple threads and they will all block until whichever thread entered the actual initialization pipeline fully completes? I see atomic load/stores but no mutex hence the question.

(I appreciate what PyO3 is doing is slightly weird, because there's no way to reliably know which one of those threads becomes the "main thread". It's good enough for Rust test suites, at least!)

@davidhewitt
Copy link
Contributor

Sorry, I see I misread the title - this is aboutPy_IsInitialized being thread-safe; this won't make any difference to callingPy_Initialize and PyO3 will presumably continue to need to have a strategy to synchronize that call.

gpshead reacted with thumbs up emoji

@gpshead
Copy link
MemberAuthor

Does thread-safe here mean that PyO3 could in principle freely call this from multiple threads and they will all block until whichever thread entered the actual initialization pipeline fully completes?

No, CPython does not have a block until initialization completes API. That might make sense as a feature request on its own but I'd wonder what the practical motivation for it was and why we'd want people to do it.

davidhewitt reacted with thumbs up emoji

@davidhewitt
Copy link
Contributor

I think it's fine to leave that synchronization downstream in PyO3 rather than complicate the C API. I think in most embedding contexts you want to be deliberate about interpreter lifecycle; what PyO3 does with the concurrent initialization is a quirk of Rust test suites launching multiple threads with no particular need for any proper finalization.

gpshead reacted with thumbs up emoji

@gpshead
Copy link
MemberAuthor

Agreed. We currently have a variety of C API surfaces (like Initialize and IsInterpreter) that are easier when embedding processes take a wide "hey i'm going to have a Python interpreter, let me go ahead and set that up once up front instead of having everything that wants to use it try to do that at time of first use" approach. That is not really practical for all situations. Your Rust example of using one from some test cases is a good example.

There are others, but they might expand into a larger C API and one interpreter vs multiple interpreters vs subinterpreters question. Imagine two independent libraries linked into a process unaware of each-other both wanting to use an embedded Python interpreter for internal use. As independent code, they have nogood way to coordinate in a multithreaded context where they might both be trying at once. Which still ignores the question of if they need isolated interpreters or not. All of that could turn into a discuss.python.org thread...

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@picnixzpicnixzpicnixz left review comments

@ericsnowcurrentlyericsnowcurrentlyAwaiting requested review from ericsnowcurrentlyericsnowcurrently is a code owner

@ZeroIntensityZeroIntensityAwaiting requested review from ZeroIntensityZeroIntensity is a code owner

@FFY00FFY00Awaiting requested review from FFY00FFY00 is a code owner

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Py_IsInitialized() can return 1 beforePy_InitializeEx() has completed.

4 participants

@gpshead@bedevere-bot@davidhewitt@picnixz

[8]ページ先頭

©2009-2026 Movatter.jp