[3.12] gh-104690 Disallow thread creation and fork at interpreter finalization (GH-104826) (#105277)

gh-104690 Disallow thread creation and fork at interpreter finalization (GH-104826)

Disallow thread creation and fork at interpreter finalization.

in the following functions, check if interpreter is finalizing and raise `RuntimeError` with appropriate message:
* `_thread.start_new_thread` and thus `threading`
* `posix.fork`
* `posix.fork1`
* `posix.forkpty`
* `_posixsubprocess.fork_exec` when a `preexec_fn=` is supplied.

---------

(cherry picked from commit ce558e6)

Co-authored-by: chgnrdv <52372310+chgnrdv@users.noreply.github.com>
Co-authored-by: Gregory P. Smith <greg@krypto.org>

Author: 3 people

Doc/library/atexit.rst

@@ -48,6 +48,16 @@ a cleanup function is undefined.
    This function returns *func*, which makes it possible to use it as a
    decorator.
 
+   .. warning::
+       Starting new threads or calling :func:`os.fork` from a registered
+       function can lead to race condition between the main Python
+       runtime thread freeing thread states while internal :mod:`threading`
+       routines or the new process try to use that state. This can lead to
+       crashes rather than clean shutdown.
+
+   .. versionchanged:: 3.12
+       Attempts to start a new thread or :func:`os.fork` a new process
+       in a registered function now leads to :exc:`RuntimeError`.
 
 .. function:: unregister(func)
 

Lib/test/test_os.py

@@ -4700,6 +4700,22 @@ def test_fork_warns_when_non_python_thread_exists(self):
         self.assertEqual(err.decode("utf-8"), "")
         self.assertEqual(out.decode("utf-8"), "")
 
+    def test_fork_at_exit(self):
+        code = """if 1:
+            import atexit
+            import os
+
+            def exit_handler():
+                pid = os.fork()
+                if pid != 0:
+                    print("shouldn't be printed")
+
+            atexit.register(exit_handler)
+        """
+        _, out, err = assert_python_ok("-c", code)
+        self.assertEqual(b"", out)
+        self.assertIn(b"can't fork at interpreter shutdown", err)
+
 
 # Only test if the C version is provided, otherwise TestPEP519 already tested
 # the pure Python implementation.

Lib/test/test_subprocess.py

@@ -5,6 +5,7 @@
 from test.support import import_helper
 from test.support import os_helper
 from test.support import warnings_helper
+from test.support.script_helper import assert_python_ok
 import subprocess
 import sys
 import signal
@@ -3329,6 +3330,24 @@ def test_communicate_repeated_call_after_stdout_close(self):
             except subprocess.TimeoutExpired:
                 pass
 
+    def test_preexec_at_exit(self):
+        code = f"""if 1:
+        import atexit
+        import subprocess
+
+        def dummy():
+            pass
+
+        def exit_handler():
+            subprocess.Popen({ZERO_RETURN_CMD}, preexec_fn=dummy)
+            print("shouldn't be printed")
+
+        atexit.register(exit_handler)
+        """
+        _, out, err = assert_python_ok("-c", code)
+        self.assertEqual(out, b'')
+        self.assertIn(b"preexec_fn not supported at interpreter shutdown", err)
+
 
 @unittest.skipUnless(mswindows, "Windows specific tests")
 class Win32ProcessTestCase(BaseTestCase):

Lib/test/test_threading.py

@@ -531,34 +531,6 @@ def test_daemon_param(self):
         t = threading.Thread(daemon=True)
         self.assertTrue(t.daemon)
 
-    @support.requires_fork()
-    def test_fork_at_exit(self):
-        # bpo-42350: Calling os.fork() after threading._shutdown() must
-        # not log an error.
-        code = textwrap.dedent("""
-            import atexit
-            import os
-            import sys
-            from test.support import wait_process
-
-            # Import the threading module to register its "at fork" callback
-            import threading
-
-            def exit_handler():
-                pid = os.fork()
-                if not pid:
-                    print("child process ok", file=sys.stderr, flush=True)
-                    # child process
-                else:
-                    wait_process(pid, exitcode=0)
-
-            # exit_handler() will be called after threading._shutdown()
-            atexit.register(exit_handler)
-        """)
-        _, out, err = assert_python_ok("-c", code)
-        self.assertEqual(out, b'')
-        self.assertEqual(err.rstrip(), b'child process ok')
-
     @support.requires_fork()
     def test_dummy_thread_after_fork(self):
         # Issue #14308: a dummy thread in the active list doesn't mess up
@@ -1048,6 +1020,22 @@ def import_threading():
         self.assertEqual(out, b'')
         self.assertEqual(err, b'')
 
+    def test_start_new_thread_at_exit(self):
+        code = """if 1:
+            import atexit
+            import _thread
+
+            def f():
+                print("shouldn't be printed")
+
+            def exit_handler():
+                _thread.start_new_thread(f, ())
+
+            atexit.register(exit_handler)
+        """
+        _, out, err = assert_python_ok("-c", code)
+        self.assertEqual(out, b'')
+        self.assertIn(b"can't create new thread at interpreter shutdown", err)
 
 class ThreadJoinOnShutdown(BaseTestCase):
 

Misc/NEWS.d/next/Core and Builtins/2023-05-24-12-10-54.gh-issue-104690.HX3Jou.rst

@@ -0,0 +1,6 @@
+Starting new threads and process creation through :func:`os.fork` during interpreter
+shutdown (such as from :mod:`atexit` handlers) is no longer supported.  It can lead
+to race condition between the main Python runtime thread freeing thread states while
+internal :mod:`threading` routines are trying to allocate and use the state of just
+created threads. Or forked children trying to use the mid-shutdown runtime and thread
+state in the child process.

Modules/_posixsubprocess.c

@@ -5,6 +5,7 @@
 
 #include "Python.h"
 #include "pycore_fileutils.h"
+#include "pycore_pystate.h"
 #if defined(HAVE_PIPE2) && !defined(_GNU_SOURCE)
 # define _GNU_SOURCE
 #endif
@@ -943,6 +944,11 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_args,
     Py_ssize_t fds_to_keep_len = PyTuple_GET_SIZE(py_fds_to_keep);
 
     PyInterpreterState *interp = PyInterpreterState_Get();
+    if ((preexec_fn != Py_None) && interp->finalizing) {
+        PyErr_SetString(PyExc_RuntimeError,
+                        "preexec_fn not supported at interpreter shutdown");
+        return NULL;
+    }
     if ((preexec_fn != Py_None) && (interp != PyInterpreterState_Main())) {
         PyErr_SetString(PyExc_RuntimeError,
                         "preexec_fn not supported within subinterpreters");

Modules/_threadmodule.c

@@ -1155,6 +1155,11 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs)
                         "thread is not supported for isolated subinterpreters");
         return NULL;
     }
+    if (interp->finalizing) {
+        PyErr_SetString(PyExc_RuntimeError,
+                        "can't create new thread at interpreter shutdown");
+        return NULL;
+    }
 
     struct bootstate *boot = PyMem_NEW(struct bootstate, 1);
     if (boot == NULL) {

Modules/posixmodule.c

@@ -7620,7 +7620,13 @@ os_fork1_impl(PyObject *module)
 {
     pid_t pid;
 
-    if (!_Py_IsMainInterpreter(_PyInterpreterState_GET())) {
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+    if (interp->finalizing) {
+        PyErr_SetString(PyExc_RuntimeError,
+                        "can't fork at interpreter shutdown");
+        return NULL;
+    }
+    if (!_Py_IsMainInterpreter(interp)) {
         PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters");
         return NULL;
     }
@@ -7656,6 +7662,11 @@ os_fork_impl(PyObject *module)
 {
     pid_t pid;
     PyInterpreterState *interp = _PyInterpreterState_GET();
+    if (interp->finalizing) {
+        PyErr_SetString(PyExc_RuntimeError,
+                        "can't fork at interpreter shutdown");
+        return NULL;
+    }
     if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_FORK)) {
         PyErr_SetString(PyExc_RuntimeError,
                         "fork not supported for isolated subinterpreters");
@@ -8327,7 +8338,13 @@ os_forkpty_impl(PyObject *module)
     int master_fd = -1;
     pid_t pid;
 
-    if (!_Py_IsMainInterpreter(_PyInterpreterState_GET())) {
+    PyInterpreterState *interp = _PyInterpreterState_GET();
+    if (interp->finalizing) {
+        PyErr_SetString(PyExc_RuntimeError,
+                        "can't fork at interpreter shutdown");
+        return NULL;
+    }
+    if (!_Py_IsMainInterpreter(interp)) {
         PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters");
         return NULL;
     }