GeezerVison

I think this is okay, but it also adds some additional constraints that will make the GC in the free threading build more fragile. I think we have to make sure that any gen/coro marked as FRAME_EXECUTING is on a PyThreadState's frame stack -- otherwise some deferred references may not be visible to the GC and collected while still in use.

• There must not be any escaping calls between setting gi_frame_state = FRAME_EXECUTING and pushing the frame to the thread's stack.
• There must not be any escaping calls between popping the frame from the stack and setting gi_frame_state to some other value. (i.e., exit_unwind -> clear_gen_frame)

All those things are already true, because we rely on gi_frame_state == FRAME_EXECUTING to guard against sending to an already executing generator.

At shutdown we delete all PyThreadStates except for the main thread. Those threads could be running generators or coroutines stuck in some long running call (like a time.sleep()). We run the GC after that. That seems like it would be unsafe (because we're now hiding deferred _PyStackRef from the GC.

This test program segfaults in the free threading build with this PR:

https://gist.github.com/colesbury/11d59b9987e881a3c016b086bb4ba1ff

If the generator is executing, it is part of the stack, not the heap.
So, I think tp_traverse should not be traversing executing generators.

@colesbury
How does the free-threading GC find references that are on the stack (in normal frames)?
We should do that for executing generators.

Yes, during shutdown, we delete all PyThreadStates except for the main thread.

Let me rephrase:
When deleting the thread state, how do we clean up the references?

Presumably, we should be changing the state of the generator when we clean up its frame but we aren't.

To answer my own question. We don't cleanup the references 🙁

We call PyThreadState_Clear() on the deleted thread states, but that doesn't clean up current_frame. We don't call PyThreadState_Delete() for reasons that are not clear to me. Even if PyThreadState_Clear() cleaned up current_frame that wouldn't be sufficient because we unlink all the thread states before calling PyThreadState_Clear() and (by your definition) that already puts them in an invalid state.

This is all longstanding CPython behavior, as far as I can tell. Changing the shutdown behavior seems likely to cause different shutdown-related bugs than what we experience today.

I think we can work around this by encoding more knowledge about generators in gc_free_threading.c. I don't love that, but it seems less risky than messing with the shutdown behavior. Let me know if you want to go that route -- I can help with the gc_free_threading.c changes.

I'd like to investigate fixing up thread state clean up. While it may well risk introducing bugs in the short term, I think to would be better long term. It is hard to optimize anything if we can't trust our supposed invariants.

A quick fix, until we decide on how to handle this long term, would be to traverse the frame list when deleting threads and mark all generators as suspended.

It's not sufficient to mark generators as zombies in PyThreadState_Clear(). We call PyThreadState_Clear() one at a time on each non-main thread:

• At that point, they are already unlinked from the interpreter's list of PyThreadStates
• The PyThreadState_Clear() calls makes escaping calls (Py_CLEAR()) that can trigger the GC

So the GC may still see generators that are marked as "running", but aren't in any accessible PyThreadState's frame stack.

I wrote "At that point, they are already unlinked from the interpreter's list of PyThreadStates", but I don't think that's accurate. Thread states are unlinked in PyThreadState_Delete or PyThreadState_DeleteCurrent, and those happen after PyThreadState_Clear.

Oh... my Oct 22, 2024 comment was in relation to interpreter shutdown, which reverses the typical order. During shutdown thread states are:

1. Unlinked
2. Cleared
3. Deleted

Whereas during normal thread exit the order is:

1. Cleared
2. Unlinked
3. Deleted

Specifically, in _Py_Finalize() we:

1. First we unlink all the remaining thread states (except the main thread). This is typically for daemon threads, but if you press Ctrl-C it can include non-daemon threads as well:

2. Then we call _PyThreadState_DeleteList(), which calls PyThreadState_Clear() on each thread one at a time, which can execute arbitrary code via finalizers and weakrefs.

I think things should be okay if we mark the generators as zombies from _PyThreadState_RemoveExcept() as well as from PyThreadState_Clear().