bpo-46070: Fix asyncio initialisation guard

[erlend-aasland]

If init flag is set, exit module init successfully immediately.
If not, only set the flag after successful module initialisation.

https://bugs.python.org/issue46070

[erlend-aasland]

Maybe we should run this through the buildbots.

[erlend-aasland]

This might qualify for a NEWS entry, as it fixes segfaults on 3.10 and 3.9 (and 3.8, but 3.8 is in security-fix-mode-only).

[vstinner]

module_init() is called in parallel multiple times from different interpreters (in different threads). I understand that module_init() leaks references when calling multiple times in parallel, since it doesn't use Py_SETREF(). For now, I suggest to not worry about that. I suggest to rather invest time on converting _asyncio static types to heap types and then port _asyncio to multi-phase init: move static globals into a module state.

[vstinner]

LGTM.

[erlend-aasland]

module_init() is called in parallel multiple times from different interpreters (in different threads). I understand that module_init() leaks references when calling multiple times in parallel, since it doesn't use Py_SETREF(). For now, I suggest to not worry about that. I suggest to rather invest time on converting _asyncio static types to heap types and then port _asyncio to multi-phase init: move static globals into a module state.

Yes, there are ref leaks, but as you say they are best fixed by converting global state to module state and implement proper multi-phase init. I've got a private branch for this work. This demands a separate bpo issue, and we should involve the asyncio maintaner before continuing with that effort :)

Thanks for reviewing!

[vstinner]

This might qualify for a NEWS entry, as it fixes segfaults on 3.10 and 3.9 (and 3.8, but 3.8 is in security-fix-mode-only).

Oh, I didn't notice the failing "news" job. Yes, please document the fix. If I understand correctly, this change fix a crash an importing the asyncio module from different sub-interpreters in parallel. And the workaround was to import asyncio in the main interpreter before spawning sub-interpreters.

[erlend-aasland]

[...] Yes, please document the fix. If I understand correctly, this change fix a crash an importing the asyncio module from different sub-interpreters in parallel. And the workaround was to import asyncio in the main interpreter before spawning sub-interpreters.

Sure, I'll try to write something sensible :) Also, we could probably use a more compact version of your reproducer as a regression test.

[vstinner]

Adding a test is optional for me. It's up to me. But I would like to see a NEWS entry please.

[erlend-aasland]

Adding a test is optional for me. It's up to me. But I would like to see a NEWS entry please.

You got it 🥁

[vstinner]

LGTM.

[miss-islington]

Thanks @erlend-aasland for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

[bedevere-bot]

GH-30453 is a backport of this pull request to the 3.10 branch.

[bedevere-bot]

GH-30454 is a backport of this pull request to the 3.9 branch.