gh-136421: Fix crash when _datetime is been initialized in multiple sub-interpreters - #136422
gh-136421: Fix crash when _datetime is been initialized in multiple sub-interpreters#136422aisk wants to merge 5 commits into
Conversation
ZeroIntensity
left a comment
There was a problem hiding this comment.
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.
Sorry, something went wrong.
|
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 phrase I 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. |
Sorry, something went wrong.
ZeroIntensity
left a comment
There was a problem hiding this comment.
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 in pycore_interp_init.
Sorry, something went wrong.
…e-136421.uzieFA.rst Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
|
An alternative would be to make _interpreters module import _datetime when running the main interpreter. |
Sorry, something went wrong.
|
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? |
Sorry, something went wrong.
|
I've put up #136583 as an alternative that avoids locking, at the cost of moving _datetime to a static library. |
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.