gh-121621: Move asyncio running loop to thread state

[Fidget-Spinner]

Follow up to #121622

• Issue: asyncio module state should be locked with mutexes in free-threaded builds #121621

[ericsnowcurrently]

CC @markshannon

[colesbury]

I think it might be worthwhile to split this up into two PRs:

• The running loop change, which should be small and relatively straightforward
• The freelist change, which I think needs more work to avoid duplicating struct definitions

[colesbury]

This doesn't seem right. We don't visit freelists anywhere else. The objects in the freelist should never be tracked by the GC.

[colesbury]

I think we want to avoid this sort of duplication if at all possible.

[colesbury]

A few suggestions:

I think the get_running_loop and set_running_loop are more confusing than helpful at this point. For example, there's a bunch of dead code now to access the module state. I think the code will be easier to read if they are inlined into their callsites.

I think the code will be more robust if the loop field on PyThreadState uses NULL instead of Py_None (i.e., handle the conversion to Py_None in the call sites as necessary). PyThreadState_Clear() still sets the field to NULL, which breaks the non-NULL invariant -- PyThreadState_Clear can still execute Python code via finalizers.

[Fidget-Spinner]

A few suggestions:

I think the get_running_loop and set_running_loop are more confusing than helpful at this point. For example, there's a bunch of dead code now to access the module state. I think the code will be easier to read if they are inlined into their callsites.

I think the code will be more robust if the loop field on PyThreadState uses NULL instead of Py_None (i.e., handle the conversion to Py_None in the call sites as necessary). PyThreadState_Clear() still sets the field to NULL, which breaks the non-NULL invariant -- PyThreadState_Clear can still execute Python code via finalizers.

I agree with the first suggestion.

The second suggestion has one problem --- we'd still need to special case Py_None because the asyncio spec says if the event loop is None, it will raise a RuntimeError. So we'd need an extra branch checking for NULL and converting to Py_None, AND a branch checking if it's non-null but Py_None. That's not clean IMO.

I'd prefer we just set it to Py_None in PyThreadState_Clear to keep the invariant that there are only two states: Py_None and not-none.

[colesbury]

This LGTM. I left a few minor style comments.

...we'd still need to special case Py_None because the asyncio spec says if the event loop is None, it will raise a RuntimeError.

This is a common pattern in CPython. We normalize Py_None to NULL in the setter. For example, _asyncio__set_running_loop would look like:

PyThreadState *ts = _PyThreadState_GET();
if (loop == Py_None) {
    loop = NULL;
}
Py_XSETREF(ts->asyncio_running_loop, Py_XNewRef(loop));

For example, see:

cpython/Objects/funcobject.c

Lines 722 to 723 in 8f25321

	if (value == Py_None)
	value = NULL;

This is a much more common pattern than setting the field to Py_None in the clear function.

[miss-islington-app]

Thanks @Fidget-Spinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

[bedevere-app]

GH-121864 is a backport of this pull request to the 3.13 branch.