[Python-checkins] [3.12] gh-104690 Disallow thread creation and fork at interpreter finalization (GH-104826) (#105277)
gpshead
webhook-mailer at python.org
Sun Jun 4 00:32:07 EDT 2023
https://github.com/python/cpython/commit/c7a9d96a25a646d37cb97506019e82ee7493d1b3
commit: c7a9d96a25a646d37cb97506019e82ee7493d1b3
branch: 3.12
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: gpshead <greg at krypto.org>
date: 2023-06-04T04:32:00Z
summary:
[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 ce558e69d4087dd3653207de78345fbb8a2c7835)
Co-authored-by: chgnrdv <52372310+chgnrdv at users.noreply.github.com>
Co-authored-by: Gregory P. Smith <greg at krypto.org>
files:
A Misc/NEWS.d/next/Core and Builtins/2023-05-24-12-10-54.gh-issue-104690.HX3Jou.rst
M Doc/library/atexit.rst
M Lib/test/test_os.py
M Lib/test/test_subprocess.py
M Lib/test/test_threading.py
M Modules/_posixsubprocess.c
M Modules/_threadmodule.c
M Modules/posixmodule.c
diff --git a/Doc/library/atexit.rst b/Doc/library/atexit.rst
index a2bd85b31c9a..3dbef69580d9 100644
--- a/Doc/library/atexit.rst
+++ b/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)
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
index 584cc05ca82a..945374213266 100644
--- a/Lib/test/test_os.py
+++ b/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.
diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py
index 92f81eaafb1c..51ba423a0f1c 100644
--- a/Lib/test/test_subprocess.py
+++ b/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):
diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py
index 97165264b34b..9e4972ecb640 100644
--- a/Lib/test/test_threading.py
+++ b/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):
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-05-24-12-10-54.gh-issue-104690.HX3Jou.rst b/Misc/NEWS.d/next/Core and Builtins/2023-05-24-12-10-54.gh-issue-104690.HX3Jou.rst
new file mode 100644
index 000000000000..7934dd23b106
--- /dev/null
+++ b/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.
diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c
index 36470804c6a1..2d88f5e9ba16 100644
--- a/Modules/_posixsubprocess.c
+++ b/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");
diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c
index 5d753b4a0ebc..b6f878e07526 100644
--- a/Modules/_threadmodule.c
+++ b/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) {
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index abc50b4d335b..f5e653dd023a 100644
--- a/Modules/posixmodule.c
+++ b/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;
}
More information about the Python-checkins
mailing list