gh-123940: Ensure force-terminated daemon threads can be joined

[mpage]

During finalization, daemon threads are forced to exit immediately:

cpython/Python/pylifecycle.c

Lines 2023 to 2026 in 10de360

	/* Remaining daemon threads will automatically exit
	when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
	_PyInterpreterState_SetFinalizing(tstate->interp, tstate);
	_PyRuntimeState_SetFinalizing(runtime, tstate);

without returning through the call-stack normally upon acquiring the GIL:

cpython/Python/ceval_gil.c

Lines 294 to 302 in 10de360

	if (_PyThreadState_MustExit(tstate)){
	/* bpo-39877: If Py_Finalize() has been called and tstate is not the
	thread which called Py_Finalize(), exit immediately the thread.
	This code path can be reached by a daemon thread after Py_Finalize()
	completes. In this case, tstate is a dangling pointer: points to
	PyThreadState freed memory. */
	PyThread_exit_thread();
	}

Finalizers that run after this must be able to join the forcefully terminated threads.

The current implementation notified of thread completion before returning from thread_run:

cpython/Modules/_threadmodule.c

Line 361 in 10de360

_PyEvent_Notify(&handle->thread_is_exiting);

Unfortunately, this code will never execute if the thread is forced to exit during finalization. Any code that attempts to join such a thread will block indefinitely.

To fix this, keep track of daemon threads and have the runtime notify their events after they are forced to exit.

• Issue: Daemonic threads not killed in some circumstances in python 3.13 #123940

[colesbury]

A slight variant where loop() calls time.sleep(1) hard crashes in 3.11 and 3.12. I'm not sure what to make of that, other than this sort of behavior wasn't robust previously either. I think it's noteworthy that the change that led to the issue was from just a few days ago -- it doesn't look like it was some longstanding code that just broke now.

[gpshead]

possibly #87135 related.

[hauntsaninja]

This is the issue #116514 colesbury linked when I asked (I wasn't able to repro just the time.sleep variant mentioned here)

[mpage]

Marking this as a draft while I figure out the linker errors on windows.

[mpage]

@colesbury@ericsnowcurrently - Would you please have another look at this? I think it makes sense to move ThreadHandle from _threadmodule.c into pycore_pythread.h/Python/thread.c but that's a bigger change, so I went with a smaller fix here. I can do the refactor in a separate PR.

[ericsnowcurrently]

mostly LGTM

There's just the matter of where the code lives. It should mostly go in Python/thread.c. However, we have a bit of a mess in threadmodule.c already, which we needn't try to fix all in this PR. I might be okay with punting on that in favor of a follow-up PR, but I'd like the opinion of others on that first.

[ericsnowcurrently]

It would make sense to have a _PyThread_InitThreadHandles() for this, to match _PyThread_ClearThreadHandles().

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[mpage]

@ericsnowcurrently and I chatted in person and the consensus is that this PR is ok as-is, but we should follow it up with a refactor (only on main) to move functionality out of Modules/_threadmodule.c and into Python/thread.c.

[gpshead]

Unfortunately, this code will never execute if the thread is forced to exit during finalization. Any code that attempts to join such a thread will block indefinitely.

To fix this, keep track of daemon threads and have the runtime notify their events after they are forced to exit.

FYI - #105805, if merged, is set to change this so that these threads do not exit because our C API has no way for them to do so cleanly to cause an unwind of the call stack. They'll be made to hang instead.

[encukou]

And now with #130402, you get PythonFinalizationError when joining such a daemon thread (which is either sleeping in PyThread_hang_thread, or will be made to hang when it next tries to acquire the GILattach thread state).

It looks like there's nothing to do here, but, I'm not sure if someone still wants to go back to (and improve) force-killing rather than hanging.

@mpage, do you want to close the PR?