Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix module init for Python 3.12 #561

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Fix module init for Python 3.12 #561

merged 1 commit into from
Oct 5, 2023

Conversation

jcrist
Copy link
Owner

@jcrist jcrist commented Oct 5, 2023

Python 3.12 changed module init slightly (this may actually be a CPython 3.12 bug, it's not 100% clear). Before Python 3.12 the module wouldn't be available via PyState_FindModule until it was fully initialized. Now the module is immediately available, but in an invalid state leading to segfaults. The workaround is to manually call PyState_AddModule to ensure the module is always available.

In the long run we should move to using multiphase module init which should avoid this problem entirely.

Python 3.12 changed module init slightly (this may actually be a CPython
3.12 bug, it's not 100% clear). Before Python 3.12 the module wouldn't
be available via `PyState_FindModule` until it was fully initialized.
Now the module is immediately available, but in an invalid state
leading to segfaults. The workaround is to manually call
`PyState_AddModule` to ensure the module is always available.

In the long run we should move to using multiphase module init which
should avoid this problem entirely.
@jcrist
Copy link
Owner Author

jcrist commented Oct 5, 2023

For additional context, the segfault here:

  • Only occurs on CPython 3.12
  • Only has a faulty memory access at import time. If msgspec successfully imports then you won't be hit by this issue.
  • Doesn't segfault if there has been sufficient code run before a msgspec import (I'm guessing this is due to the faulty memory access being in a region that was already allocated and used previously in this case). In particular, this means that the test suite never segfaults due to this issue, which is how it went uncaught until the conda-forge builds started failing (here).

Rather than add a test for this esoteric issue, I've:

  • Manually validated that the fix here resolved it
  • More finely read the relevant docs which confirm that the code here is as intended
  • Read the relevant CPython source to also confirm this behavior

I'll cut a new release immediately after merging this.

@jcrist jcrist merged commit 04e7a15 into main Oct 5, 2023
8 checks passed
@jcrist jcrist deleted the fix-module-init branch October 5, 2023 04:19
@akotlar
Copy link

akotlar commented Oct 8, 2023

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants