From 0e5fd0b2f6d82ad79696093c21c5c58e41fca6eb Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Sun, 8 Mar 2020 11:57:45 +0100 Subject: [PATCH] bpo-19466: Py_Finalize() clears daemon threads earlier Clear the frames of daemon threads earlier during the Python shutdown to call objects destructors. So "unclosed file" resource warnings are now emitted for daemon threads in a more reliable way. Cleanup _PyThreadState_DeleteExcept() code: rename "garbage" to "list". --- Lib/test/test_threading.py | 45 +++++++++++++++++++ .../2020-03-08-12-11-38.bpo-19466.OdOpXP.rst | 3 ++ Python/pylifecycle.c | 10 +++++ Python/pystate.c | 19 +++++--- 4 files changed, 70 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-03-08-12-11-38.bpo-19466.OdOpXP.rst diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index a9d31afbe33e68..87c68df464326c 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -759,6 +759,51 @@ def test_shutdown_locks(self): # Daemon threads must never add it to _shutdown_locks. self.assertNotIn(tstate_lock, threading._shutdown_locks) + def test_locals_at_exit(self): + # bpo-19466: thread locals must not be deleted before destructors + # are called + rc, out, err = assert_python_ok("-c", """if 1: + import threading + + class Atexit: + def __del__(self): + print("thread_dict.atexit = %r" % thread_dict.atexit) + + thread_dict = threading.local() + thread_dict.atexit = "value" + + atexit = Atexit() + """) + self.assertEqual(out.rstrip(), b"thread_dict.atexit = 'value'") + + def test_warnings_at_exit(self): + # bpo-19466: try to call most destructors at Python shutdown before + # destroying Python thread states + filename = __file__ + rc, out, err = assert_python_ok("-Wd", "-c", """if 1: + import time + import threading + from test import support + + def open_sleep(): + # a warning will be emitted when the open file will be + # destroyed (without being explicitly closed) while the daemon + # thread is destroyed + fileobj = open(%a, 'rb') + start_event.set() + time.sleep(support.LONG_TIMEOUT) + + start_event = threading.Event() + + thread = threading.Thread(target=open_sleep, daemon=True) + thread.start() + + # wait until the thread started + start_event.wait() + """ % filename) + self.assertRegex(err.rstrip(), + b"^sys:1: ResourceWarning: unclosed file ") + class ThreadJoinOnShutdown(BaseTestCase): diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-03-08-12-11-38.bpo-19466.OdOpXP.rst b/Misc/NEWS.d/next/Core and Builtins/2020-03-08-12-11-38.bpo-19466.OdOpXP.rst new file mode 100644 index 00000000000000..0950ad62605e3f --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-03-08-12-11-38.bpo-19466.OdOpXP.rst @@ -0,0 +1,3 @@ +Clear the frames of daemon threads earlier during the Python shutdown to +call objects destructors. So "unclosed file" resource warnings are now +emitted for daemon threads in a more reliable way. diff --git a/Python/pylifecycle.c b/Python/pylifecycle.c index c99c3673d739c2..d00bf821c570a4 100644 --- a/Python/pylifecycle.c +++ b/Python/pylifecycle.c @@ -1373,6 +1373,16 @@ Py_FinalizeEx(void) runtime->initialized = 0; runtime->core_initialized = 0; + /* Destroy the state of all threads of the interpreter, except of the + current thread. In practice, only daemon threads should still be alive, + except if wait_for_thread_shutdown() has been cancelled by CTRL+C. + Clear frames of other threads to call objects destructors. Destructors + will be called in the current Python thread. Since + _PyRuntimeState_SetFinalizing() has been called, no other Python thread + can take the GIL at this point: if they try, they will exit + immediately. */ + _PyThreadState_DeleteExcept(runtime, tstate); + /* Flush sys.stdout and sys.stderr */ if (flush_std_files() < 0) { status = -1; diff --git a/Python/pystate.c b/Python/pystate.c index a1eb5239f7f304..f907fc1fc9a6a2 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -895,25 +895,30 @@ void _PyThreadState_DeleteExcept(_PyRuntimeState *runtime, PyThreadState *tstate) { PyInterpreterState *interp = tstate->interp; - PyThreadState *p, *next, *garbage; + HEAD_LOCK(runtime); /* Remove all thread states, except tstate, from the linked list of thread states. This will allow calling PyThreadState_Clear() without holding the lock. */ - garbage = interp->tstate_head; - if (garbage == tstate) - garbage = tstate->next; - if (tstate->prev) + PyThreadState *list = interp->tstate_head; + if (list == tstate) { + list = tstate->next; + } + if (tstate->prev) { tstate->prev->next = tstate->next; - if (tstate->next) + } + if (tstate->next) { tstate->next->prev = tstate->prev; + } tstate->prev = tstate->next = NULL; interp->tstate_head = tstate; HEAD_UNLOCK(runtime); + /* Clear and deallocate all stale thread states. Even if this executes Python code, we should be safe since it executes in the current thread, not one of the stale threads. */ - for (p = garbage; p; p = next) { + PyThreadState *p, *next; + for (p = list; p; p = next) { next = p->next; PyThreadState_Clear(p); PyMem_RawFree(p);