NotificationsYou must be signed in to change notification settings
Fork32.4k
Star67.9k

gh-136421: Fix crash when _datetime is been initialized in multiple sub-interpreters#136422

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.

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

Already on GitHub?Sign in to your account

Jump to bottom

Open

aisk wants to merge5 commits intopython:main

base:main

Choose a base branch

fromaisk:datetime-subinterpreter-initialize

Open

gh-136421: Fix crash when _datetime is been initialized in multiple sub-interpreters#136422

aisk wants to merge5 commits intopython:mainfromaisk:datetime-subinterpreter-initialize

Conversation

Copy link

Member

aisk commentedJul 8, 2025•
edited by bedevere-appbot
Loading

Issue:Import_datetime in sub-interpreters in the same time may crash the process #136421

Fix crash when _datetime is been initialized in multiple sub-interpre…

f1e7225

…ters

aisk requested review frompganssle andabalkin ascode owners

July 8, 2025 14:01

bedevere-appbot added the awaiting review label

Jul 8, 2025

bedevere-appbot mentioned this pull request

Jul 8, 2025

Import_datetime in sub-interpreters in the same time may crash the process#136421

Open

aisk changed the title~~gh-136421: Fix crash when _datetime is been initialized in multiple sub-interpre…~~gh-136421: Fix crash when _datetime is been initialized in multiple sub-interpreters

Jul 8, 2025

Fix RST markers

b6735f6

serhiy-storchaka reviewed

Jul 8, 2025

View reviewed changes

Modules/_datetimemodule.cShow resolvedHide resolved

Simplify the codes

67cf8fd

ZeroIntensity requested changes

Jul 8, 2025

View reviewed changes

Copy link

Member

ZeroIntensity left a comment

Choose a reason for hiding this comment

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

I think we need a more fundamental fix that prevents static types from being initialized concurrently. This is sort of a known issue, see#129817.

Modules/_datetimemodule.c OutdatedShow resolvedHide resolved

bedevere-appbot removed the awaiting review label

Jul 8, 2025

Copy link

bedevere-appbot commentedJul 8, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

bedevere-appbot added the awaiting changes label

Jul 8, 2025

ZeroIntensity added the topic-subinterpreters label

Jul 8, 2025

neonene reviewed

Jul 12, 2025

View reviewed changes

Modules/_datetimemodule.c OutdatedShow resolvedHide resolved

Increase mutex protect region

1637eb9

ZeroIntensity reviewed

Jul 12, 2025

View reviewed changes

Copy link

Member

ZeroIntensity left a comment

Choose a reason for hiding this comment

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

Please add a test. I'm still not fully convinced that this is the right approach;_PyStaticType_InitForExtension should be thread-safe on its own, or at least in the places where it's used.

I think the problem is that_datetime is special and has static types (because they need to be exposed via the capsule). Instead of initializing them in the module's execution function, let's do it during interpreter initialization inpycore_interp_init.

Misc/NEWS.d/next/Core_and_Builtins/2025-07-08-23-00-58.gh-issue-136421.uzieFA.rst OutdatedShow resolvedHide resolved

Update Misc/NEWS.d/next/Core_and_Builtins/2025-07-08-23-00-58.gh-issu…

d1b8728

…e-136421.uzieFA.rstCo-authored-by: Peter Bierma <zintensitydev@gmail.com>

Copy link

Contributor

neonene commentedJul 12, 2025

An alternative would be to make_interpreters module import_datetime when running the main interpreter.

Copy link

MemberAuthor

aisk commentedJul 12, 2025

Due to issue#136423, importing _datetime into a sub-interpreter concurrently still results in a crash, even after this change, so it's hard to write a test. I also feel that this is not the correct way to fix the root cause, and this PR is more like a workaround.

I'll continue to investigate#136423, as they appear to have the same root cause. Maybe we can find a better way to address them. So maybe we can wait sometime before continue this PR?