Skip to content

Commit

Permalink
Mark all daemon threads as done during finalization
Browse files Browse the repository at this point in the history
  • Loading branch information
mpage committed Sep 18, 2024
1 parent 33ab654 commit 192b260
Show file tree
Hide file tree
Showing 8 changed files with 96 additions and 24 deletions.
1 change: 1 addition & 0 deletions Include/internal/pycore_global_objects_fini_generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Include/internal/pycore_global_strings.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ struct _Py_global_strings {
STRUCT_FOR_ID(_blksize)
STRUCT_FOR_ID(_bootstrap)
STRUCT_FOR_ID(_check_retval_)
STRUCT_FOR_ID(_daemon_threads_exited)
STRUCT_FOR_ID(_dealloc_warn)
STRUCT_FOR_ID(_feature_version)
STRUCT_FOR_ID(_field_types)
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_runtime_init_generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Include/internal/pycore_unicodeobject_generated.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Lib/test/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -1171,7 +1171,6 @@ def __del__(self):
self.assertEqual(out.strip(), b"OK")
self.assertIn(b"can't create new thread at interpreter shutdown", err)

@unittest.skipIf(support.Py_GIL_DISABLED, "gh-124149: daemon threads don't force exit")
def test_join_force_terminated_daemon_thread_in_finalization(self):
# gh-123940: Py_Finalize() forces all daemon threads to exit
# immediately (without unwinding the stack) upon acquiring the
Expand Down
1 change: 1 addition & 0 deletions Lib/threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
get_ident = _thread.get_ident
_get_main_thread_ident = _thread._get_main_thread_ident
_is_main_interpreter = _thread._is_main_interpreter
_daemon_threads_exited = _thread._daemon_threads_exited
try:
get_native_id = _thread.get_native_id
_HAVE_THREAD_NATIVE_ID = True
Expand Down
80 changes: 63 additions & 17 deletions Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,11 @@ typedef struct {

// Linked list of handles to all non-daemon threads created by the
// threading module. We wait for these to finish at shutdown.
struct llist_node shutdown_handles;
struct llist_node non_daemon_handles;

// Linked list of handles to daemon threads created by the threading
// module. We set the events in these handles as done at shutdown.
struct llist_node daemon_handles;
} thread_module_state;

static inline thread_module_state*
Expand Down Expand Up @@ -78,7 +82,8 @@ typedef enum {
typedef struct {
struct llist_node node; // linked list node (see _pythread_runtime_state)

// linked list node (see thread_module_state)
// belongs to either `non_daemon_handles` or `daemon_handles` (see
// thread_module_state)
struct llist_node shutdown_node;

// The `ident`, `os_handle`, `has_os_handle`, and `state` fields are
Expand Down Expand Up @@ -143,10 +148,11 @@ ThreadHandle_get_os_handle(ThreadHandle *handle, PyThread_handle_t *os_handle)
}

static void
add_to_shutdown_handles(thread_module_state *state, ThreadHandle *handle)
add_to_shutdown_handles(struct llist_node *shutdown_handles,
ThreadHandle *handle)
{
HEAD_LOCK(&_PyRuntime);
llist_insert_tail(&state->shutdown_handles, &handle->shutdown_node);
llist_insert_tail(shutdown_handles, &handle->shutdown_node);
HEAD_UNLOCK(&_PyRuntime);
}

Expand All @@ -155,7 +161,10 @@ clear_shutdown_handles(thread_module_state *state)
{
HEAD_LOCK(&_PyRuntime);
struct llist_node *node;
llist_for_each_safe(node, &state->shutdown_handles) {
llist_for_each_safe(node, &state->daemon_handles) {
llist_remove(node);
}
llist_for_each_safe(node, &state->non_daemon_handles) {
llist_remove(node);
}
HEAD_UNLOCK(&_PyRuntime);
Expand Down Expand Up @@ -378,7 +387,7 @@ force_done(ThreadHandle *handle)

static int
ThreadHandle_start(ThreadHandle *self, PyObject *func, PyObject *args,
PyObject *kwargs)
PyObject *kwargs, struct llist_node *shutdown_handles)
{
// Mark the handle as starting to prevent any other threads from doing so
PyMutex_Lock(&self->mutex);
Expand All @@ -390,6 +399,11 @@ ThreadHandle_start(ThreadHandle *self, PyObject *func, PyObject *args,
self->state = THREAD_HANDLE_STARTING;
PyMutex_Unlock(&self->mutex);

// Add the handle before starting the thread to avoid adding a handle
// to a thread that has already finished (i.e. if the thread finishes
// before the call to `ThreadHandle_start()` below returns).
add_to_shutdown_handles(shutdown_handles, self);

// Do all the heavy lifting outside of the mutex. All other operations on
// the handle should fail since the handle is in the starting state.

Expand Down Expand Up @@ -441,6 +455,7 @@ ThreadHandle_start(ThreadHandle *self, PyObject *func, PyObject *args,
return 0;

start_failed:
remove_from_shutdown_handles(self);
_PyOnceFlag_CallOnce(&self->once, (_Py_once_fn_t *)force_done, self);
return -1;
}
Expand Down Expand Up @@ -1872,17 +1887,12 @@ do_start_new_thread(thread_module_state *state, PyObject *func, PyObject *args,
return -1;
}

if (!daemon) {
// Add the handle before starting the thread to avoid adding a handle
// to a thread that has already finished (i.e. if the thread finishes
// before the call to `ThreadHandle_start()` below returns).
add_to_shutdown_handles(state, handle);
struct llist_node *shutdown_handles = &state->non_daemon_handles;
if (daemon) {
shutdown_handles = &state->daemon_handles;
}

if (ThreadHandle_start(handle, func, args, kwargs) < 0) {
if (!daemon) {
remove_from_shutdown_handles(handle);
}
if (ThreadHandle_start(handle, func, args, kwargs, shutdown_handles) < 0) {
return -1;
}

Expand Down Expand Up @@ -2372,7 +2382,7 @@ thread_shutdown(PyObject *self, PyObject *args)
// Find a thread that's not yet finished.
HEAD_LOCK(&_PyRuntime);
struct llist_node *node;
llist_for_each_safe(node, &state->shutdown_handles) {
llist_for_each_safe(node, &state->non_daemon_handles) {
ThreadHandle *cur = llist_data(node, ThreadHandle, shutdown_node);
if (cur->ident != ident) {
ThreadHandle_incref(cur);
Expand Down Expand Up @@ -2407,6 +2417,39 @@ PyDoc_STRVAR(shutdown_doc,
\n\
Wait for all non-daemon threads (other than the calling thread) to stop.");

static PyObject *
thread__daemon_threads_exited(PyObject *self, PyObject *args)
{
thread_module_state *state = get_thread_state(self);

for (;;) {
HEAD_LOCK(&_PyRuntime);
if (llist_empty(&state->daemon_handles)) {
HEAD_UNLOCK(&_PyRuntime);
break;
}
struct llist_node *node = state->daemon_handles.next;
ThreadHandle *handle = llist_data(node, ThreadHandle, shutdown_node);
ThreadHandle_incref(handle);
llist_remove(node);
HEAD_UNLOCK(&_PyRuntime);
// gh-123940: Mark daemon threads as done so that they can be joined
// from finalizers.
if (ThreadHandle_set_done(handle) < 0) {
PyErr_WriteUnraisable(NULL);
}
ThreadHandle_decref(handle);
}

Py_RETURN_NONE;
}

PyDoc_STRVAR(_daemon_threads_exited_doc,
"_daemon_threads_exited($module, /)\n\
--\n\
\n\
Mark that daemon threads have exited.");

static PyObject *
thread__make_thread_handle(PyObject *module, PyObject *identobj)
{
Expand Down Expand Up @@ -2486,6 +2529,8 @@ static PyMethodDef thread_methods[] = {
METH_NOARGS, thread__is_main_interpreter_doc},
{"_shutdown", thread_shutdown,
METH_NOARGS, shutdown_doc},
{"_daemon_threads_exited", thread__daemon_threads_exited,
METH_NOARGS, _daemon_threads_exited_doc},
{"_make_thread_handle", thread__make_thread_handle,
METH_O, thread__make_thread_handle_doc},
{"_get_main_thread_ident", thread__get_main_thread_ident,
Expand Down Expand Up @@ -2579,7 +2624,8 @@ thread_module_exec(PyObject *module)
return -1;
}

llist_init(&state->shutdown_handles);
llist_init(&state->daemon_handles);
llist_init(&state->non_daemon_handles);

return 0;
}
Expand Down
31 changes: 25 additions & 6 deletions Python/pylifecycle.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ static PyStatus init_sys_streams(PyThreadState *tstate);
#ifdef __ANDROID__
static PyStatus init_android_streams(PyThreadState *tstate);
#endif
static void wait_for_thread_shutdown(PyThreadState *tstate);
static PyObject *wait_for_thread_shutdown(PyThreadState *tstate);
static void daemon_threads_exited(PyObject *threading_module);
static void finalize_subinterpreters(void);
static void call_ll_exitfuncs(_PyRuntimeState *runtime);

Expand Down Expand Up @@ -1985,7 +1986,7 @@ _Py_Finalize(_PyRuntimeState *runtime)
tstate->interp->finalizing = 1;

// Wrap up existing "threading"-module-created, non-daemon threads.
wait_for_thread_shutdown(tstate);
PyObject *threading_module = wait_for_thread_shutdown(tstate);

// Make any remaining pending calls.
_Py_FinishPendingCalls(tstate);
Expand Down Expand Up @@ -2040,6 +2041,7 @@ _Py_Finalize(_PyRuntimeState *runtime)
before we call destructors. */
PyThreadState *list = _PyThreadState_RemoveExcept(tstate);
_PyEval_StartTheWorldAll(runtime);
daemon_threads_exited(threading_module);
_PyThreadState_DeleteList(list);

/* At this point no Python code should be running at all.
Expand Down Expand Up @@ -2388,7 +2390,7 @@ Py_EndInterpreter(PyThreadState *tstate)
interp->finalizing = 1;

// Wrap up existing "threading"-module-created, non-daemon threads.
wait_for_thread_shutdown(tstate);
PyObject *threading_module = wait_for_thread_shutdown(tstate);

// Make any remaining pending calls.
_Py_FinishPendingCalls(tstate);
Expand All @@ -2403,6 +2405,7 @@ Py_EndInterpreter(PyThreadState *tstate)
when they attempt to take the GIL (ex: PyEval_RestoreThread()). */
_PyInterpreterState_SetFinalizing(interp, tstate);

daemon_threads_exited(threading_module);
// XXX Call something like _PyImport_Disable() here?

_PyImport_FiniExternal(tstate->interp);
Expand Down Expand Up @@ -3328,7 +3331,7 @@ Py_ExitStatusException(PyStatus status)
the threading module was imported in the first place.
The shutdown routine will wait until all non-daemon
"threading" threads have completed. */
static void
static PyObject *
wait_for_thread_shutdown(PyThreadState *tstate)
{
PyObject *result;
Expand All @@ -3338,7 +3341,7 @@ wait_for_thread_shutdown(PyThreadState *tstate)
PyErr_FormatUnraisable("Exception ignored on threading shutdown");
}
/* else: threading not imported */
return;
return NULL;
}
result = PyObject_CallMethodNoArgs(threading, &_Py_ID(_shutdown));
if (result == NULL) {
Expand All @@ -3347,7 +3350,23 @@ wait_for_thread_shutdown(PyThreadState *tstate)
else {
Py_DECREF(result);
}
Py_DECREF(threading);
return threading;
}

static void
daemon_threads_exited(PyObject *threading_module)
{
if (threading_module == NULL) {
return;
}
PyObject *result = PyObject_CallMethodNoArgs(
threading_module, &_Py_ID(_daemon_threads_exited));
if (result == NULL) {
PyErr_FormatUnraisable(
"Exception ignored while marking daemon threads exited");
}
Py_XDECREF(result);
Py_DECREF(threading_module);
}

int Py_AtExit(void (*func)(void))
Expand Down

0 comments on commit 192b260

Please sign in to comment.