gh-132413: Fix crash in _datetime when used at shutdown #132665
Conversation
|
All commit authors signed the Contributor License Agreement. |
There was a problem hiding this comment.
I don't think we should be storing more things in the interpreter dictionary. It's relatively slow, and at a glance, probably a thread-safety issue in the way it's being used here.
The better solution would be to drop the interpreter dictionary usage here entirely, and just store the module state on the interpreter state.
|
It sounds like the concern is not about the issue that needs to be fixed even on 3.13. The current module state access may not be so fast, but I doubt this PR makes it slower. My question for alternatives is that how much we need to care about the thread-safety after |
|
I meant an existing thread-safety issue, but you're right, let's not address that here.
|
|
I agree to the risk of manual invocation during finalization. Two versions work fine:
Both require the interpreter dict. I failed to add a module object to the interpreter state (bad reloading). |
|
The interpreter state isn't part of the public ABI, we can (and do) change it in backports. |
|
Let me know when you're ready for a re-review. |
|
Ready for reviewing. Though, there seems to be fundamental fixes? |
Create a module when it is not importable.This patch gives the interpreter state struct an additional pointer to the
_datetimemodule state. Currently, the module state can be reached via a weak reference to the module, which seems problematic because the weakref counts the active module as invalid earlier at shutdown.(No different than before logically, but the attempt would need to be withdrawn if there should be a reasonable case that the introduced pointer becomes a dangling pointer or one to a reallocated region. #132599 is an alternative.)
(This does not suppress
ImportErrorraised when running thestrptimemethods at shutdown, which is almost traditional.)datetime.timedelta(possibly from datetime's Cdelta_new) #132413