Skip to content

Fix module init for Python 3.12#561

Merged
jcrist merged 1 commit intomainfrom
fix-module-init
Oct 5, 2023
Merged

Fix module init for Python 3.12#561
jcrist merged 1 commit intomainfrom
fix-module-init

Conversation

@jcrist
Copy link
Copy Markdown
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
Copy Markdown
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
@jcrist jcrist deleted the fix-module-init branch October 5, 2023 04:19
@akotlar
Copy link
Copy Markdown

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