[Python-checkins] bpo-19466: Py_Finalize() clears daemon threads earlier (GH-18848)

[Victor Stinner]

https://github.com/python/cpython/commit/9ad58acbe8b90b4d0f2d2e139e38bb5aa32b7fb6
commit: 9ad58acbe8b90b4d0f2d2e139e38bb5aa32b7fb6
branch: master
author: Victor Stinner <vstinner at python.org>
committer: GitHub <noreply at github.com>
date: 2020-03-09T23:37:49+01:00
summary:

bpo-19466: Py_Finalize() clears daemon threads earlier (GH-18848)

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".

files:
A Misc/NEWS.d/next/Core and Builtins/2020-03-08-12-11-38.bpo-19466.OdOpXP.rst
M Lib/test/test_threading.py
M Python/pylifecycle.c
M Python/pystate.c

diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py
index a9d31afbe33e6..87c68df464326 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 0000000000000..0950ad62605e3
--- /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 c99c3673d739c..d00bf821c570a 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 a1eb5239f7f30..f907fc1fc9a6a 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);