[Python-checkins] cpython (2.7): avoid crashes and lockups from daemon threads during interpreter shutdown

[benjamin.peterson]

http://hg.python.org/cpython/rev/7741d0dd66ca
changeset:   91229:7741d0dd66ca
branch:      2.7
parent:      91226:6d86e8afc2a5
user:        Benjamin Peterson <benjamin at python.org>
date:        Mon Jun 16 19:39:18 2014 -0700
summary:
  avoid crashes and lockups from daemon threads during interpreter shutdown (#1856)

files:
  Include/pythonrun.h        |   2 +
  Lib/test/test_threading.py |  43 ++++++++++++++++++++++++++
  Misc/NEWS                  |   4 ++
  Python/ceval.c             |   6 +++
  Python/pythonrun.c         |   9 ++++-
  Python/thread_pthread.h    |   4 +-
  6 files changed, 65 insertions(+), 3 deletions(-)


diff --git a/Include/pythonrun.h b/Include/pythonrun.h
--- a/Include/pythonrun.h
+++ b/Include/pythonrun.h
@@ -146,6 +146,8 @@
 PyAPI_FUNC(void) PyOS_FiniInterrupts(void);
 PyAPI_FUNC(void) PyByteArray_Fini(void);
 
+PyAPI_DATA(PyThreadState *) _Py_Finalizing;
+
 /* Stuff with no proper home (yet) */
 PyAPI_FUNC(char *) PyOS_Readline(FILE *, FILE *, char *);
 PyAPI_DATA(int) (*PyOS_InputHook)(void);
diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py
--- a/Lib/test/test_threading.py
+++ b/Lib/test/test_threading.py
@@ -700,6 +700,49 @@
         output = "end of worker thread\nend of main thread\n"
         self.assertScriptHasOutput(script, output)
 
+    @unittest.skipIf(sys.platform in platforms_to_skip, "due to known OS bug")
+    def test_6_daemon_threads(self):
+        # Check that a daemon thread cannot crash the interpreter on shutdown
+        # by manipulating internal structures that are being disposed of in
+        # the main thread.
+        script = """if True:
+            import os
+            import random
+            import sys
+            import time
+            import threading
+
+            thread_has_run = set()
+
+            def random_io():
+                '''Loop for a while sleeping random tiny amounts and doing some I/O.'''
+                while True:
+                    in_f = open(os.__file__, 'rb')
+                    stuff = in_f.read(200)
+                    null_f = open(os.devnull, 'wb')
+                    null_f.write(stuff)
+                    time.sleep(random.random() / 1995)
+                    null_f.close()
+                    in_f.close()
+                    thread_has_run.add(threading.current_thread())
+
+            def main():
+                count = 0
+                for _ in range(40):
+                    new_thread = threading.Thread(target=random_io)
+                    new_thread.daemon = True
+                    new_thread.start()
+                    count += 1
+                while len(thread_has_run) < count:
+                    time.sleep(0.001)
+                # Trigger process shutdown
+                sys.exit(0)
+
+            main()
+            """
+        rc, out, err = assert_python_ok('-c', script)
+        self.assertFalse(err)
+
     @unittest.skipUnless(hasattr(os, 'fork'), "needs os.fork()")
     @unittest.skipIf(sys.platform in platforms_to_skip, "due to known OS bug")
     def test_reinit_tls_after_fork(self):
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,10 @@
 Core and Builtins
 -----------------
 
+- Issue #1856: Avoid crashes and lockups when daemon threads run while the
+  interpreter is shutting down; instead, these threads are now killed when they
+  try to take the GIL.
+
 - Issue #19656: Running Python with the -3 option now also warns about
   non-ascii bytes literals.
 
diff --git a/Python/ceval.c b/Python/ceval.c
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -355,6 +355,12 @@
     if (interpreter_lock) {
         int err = errno;
         PyThread_acquire_lock(interpreter_lock, 1);
+        /* _Py_Finalizing is protected by the GIL */
+        if (_Py_Finalizing && tstate != _Py_Finalizing) {
+            PyThread_release_lock(interpreter_lock);
+            PyThread_exit_thread();
+            assert(0);  /* unreachable */
+        }
         errno = err;
     }
 #endif
diff --git a/Python/pythonrun.c b/Python/pythonrun.c
--- a/Python/pythonrun.c
+++ b/Python/pythonrun.c
@@ -91,6 +91,8 @@
 int Py_NoUserSiteDirectory = 0; /* for -s and site.py */
 int Py_HashRandomizationFlag = 0; /* for -R and PYTHONHASHSEED */
 
+PyThreadState *_Py_Finalizing = NULL;
+
 
 /* Hack to force loading of object files */
 int (*_PyOS_mystrnicmp_hack)(const char *, const char *, Py_ssize_t) = \
@@ -163,6 +165,7 @@
     if (initialized)
         return;
     initialized = 1;
+    _Py_Finalizing = NULL;
 
     if ((p = Py_GETENV("PYTHONDEBUG")) && *p != '\0')
         Py_DebugFlag = add_flag(Py_DebugFlag, p);
@@ -422,12 +425,16 @@
      * the threads created via Threading.
      */
     call_sys_exitfunc();
-    initialized = 0;
 
     /* Get current thread state and interpreter pointer */
     tstate = PyThreadState_GET();
     interp = tstate->interp;
 
+    /* Remaining threads (e.g. daemon threads) will automatically exit
+       after taking the GIL (in PyEval_RestoreThread()). */
+    _Py_Finalizing = tstate;
+    initialized = 0;
+
     /* Disable signal handling */
     PyOS_FiniInterrupts();
 
diff --git a/Python/thread_pthread.h b/Python/thread_pthread.h
--- a/Python/thread_pthread.h
+++ b/Python/thread_pthread.h
@@ -242,9 +242,9 @@
 PyThread_exit_thread(void)
 {
     dprintf(("PyThread_exit_thread called\n"));
-    if (!initialized) {
+    if (!initialized)
         exit(0);
-    }
+    pthread_exit(0);
 }
 
 #ifdef USE_SEMAPHORES

-- 
Repository URL: http://hg.python.org/cpython