Skip to content

Commit b31e6ae

Browse files
chgnrdvblurb-it[bot]gpshead
authored and
miss-islington
committed
pythongh-104690 Disallow thread creation and fork at interpreter finalization (pythonGH-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: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Gregory P. Smith <greg@krypto.org>
1 parent f629d5f commit b31e6ae

8 files changed

Lines changed: 97 additions & 30 deletions

File tree

‎Doc/library/atexit.rst‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,16 @@ a cleanup function is undefined.
4848
This function returns *func*, which makes it possible to use it as a
4949
decorator.
5050

51+
.. warning::
52+
Starting new threads or calling :func:`os.fork` from a registered
53+
function can lead to race condition between the main Python
54+
runtime thread freeing thread states while internal :mod:`threading`
55+
routines or the new process try to use that state. This can lead to
56+
crashes rather than clean shutdown.
57+
58+
.. versionchanged:: 3.12
59+
Attempts to start a new thread or :func:`os.fork` a new process
60+
in a registered function now leads to :exc:`RuntimeError`.
5161

5262
.. function:: unregister(func)
5363

‎Lib/test/test_os.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4700,6 +4700,22 @@ def test_fork_warns_when_non_python_thread_exists(self):
47004700
self.assertEqual(err.decode("utf-8"), "")
47014701
self.assertEqual(out.decode("utf-8"), "")
47024702

4703+
def test_fork_at_exit(self):
4704+
code = """if 1:
4705+
import atexit
4706+
import os
4707+
4708+
def exit_handler():
4709+
pid = os.fork()
4710+
if pid != 0:
4711+
print("shouldn't be printed")
4712+
4713+
atexit.register(exit_handler)
4714+
"""
4715+
_, out, err = assert_python_ok("-c", code)
4716+
self.assertEqual(b"", out)
4717+
self.assertIn(b"can't fork at interpreter shutdown", err)
4718+
47034719

47044720
# Only test if the C version is provided, otherwise TestPEP519 already tested
47054721
# the pure Python implementation.

‎Lib/test/test_subprocess.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from test.support import import_helper
66
from test.support import os_helper
77
from test.support import warnings_helper
8+
from test.support.script_helper import assert_python_ok
89
import subprocess
910
import sys
1011
import signal
@@ -3329,6 +3330,24 @@ def test_communicate_repeated_call_after_stdout_close(self):
33293330
except subprocess.TimeoutExpired:
33303331
pass
33313332

3333+
def test_preexec_at_exit(self):
3334+
code = f"""if 1:
3335+
import atexit
3336+
import subprocess
3337+
3338+
def dummy():
3339+
pass
3340+
3341+
def exit_handler():
3342+
subprocess.Popen({ZERO_RETURN_CMD}, preexec_fn=dummy)
3343+
print("shouldn't be printed")
3344+
3345+
atexit.register(exit_handler)
3346+
"""
3347+
_, out, err = assert_python_ok("-c", code)
3348+
self.assertEqual(out, b'')
3349+
self.assertIn(b"preexec_fn not supported at interpreter shutdown", err)
3350+
33323351

33333352
@unittest.skipUnless(mswindows, "Windows specific tests")
33343353
class Win32ProcessTestCase(BaseTestCase):

‎Lib/test/test_threading.py‎

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -531,34 +531,6 @@ def test_daemon_param(self):
531531
t = threading.Thread(daemon=True)
532532
self.assertTrue(t.daemon)
533533

534-
@support.requires_fork()
535-
def test_fork_at_exit(self):
536-
# bpo-42350: Calling os.fork() after threading._shutdown() must
537-
# not log an error.
538-
code = textwrap.dedent("""
539-
import atexit
540-
import os
541-
import sys
542-
from test.support import wait_process
543-
544-
# Import the threading module to register its "at fork" callback
545-
import threading
546-
547-
def exit_handler():
548-
pid = os.fork()
549-
if not pid:
550-
print("child process ok", file=sys.stderr, flush=True)
551-
# child process
552-
else:
553-
wait_process(pid, exitcode=0)
554-
555-
# exit_handler() will be called after threading._shutdown()
556-
atexit.register(exit_handler)
557-
""")
558-
_, out, err = assert_python_ok("-c", code)
559-
self.assertEqual(out, b'')
560-
self.assertEqual(err.rstrip(), b'child process ok')
561-
562534
@support.requires_fork()
563535
def test_dummy_thread_after_fork(self):
564536
# Issue #14308: a dummy thread in the active list doesn't mess up
@@ -1048,6 +1020,22 @@ def import_threading():
10481020
self.assertEqual(out, b'')
10491021
self.assertEqual(err, b'')
10501022

1023+
def test_start_new_thread_at_exit(self):
1024+
code = """if 1:
1025+
import atexit
1026+
import _thread
1027+
1028+
def f():
1029+
print("shouldn't be printed")
1030+
1031+
def exit_handler():
1032+
_thread.start_new_thread(f, ())
1033+
1034+
atexit.register(exit_handler)
1035+
"""
1036+
_, out, err = assert_python_ok("-c", code)
1037+
self.assertEqual(out, b'')
1038+
self.assertIn(b"can't create new thread at interpreter shutdown", err)
10511039

10521040
class ThreadJoinOnShutdown(BaseTestCase):
10531041

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

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

‎Modules/_posixsubprocess.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include "Python.h"
77
#include "pycore_fileutils.h"
8+
#include "pycore_pystate.h"
89
#if defined(HAVE_PIPE2) && !defined(_GNU_SOURCE)
910
# define _GNU_SOURCE
1011
#endif
@@ -943,6 +944,11 @@ subprocess_fork_exec_impl(PyObject *module, PyObject *process_args,
943944
Py_ssize_t fds_to_keep_len = PyTuple_GET_SIZE(py_fds_to_keep);
944945

945946
PyInterpreterState *interp = PyInterpreterState_Get();
947+
if ((preexec_fn != Py_None) && interp->finalizing) {
948+
PyErr_SetString(PyExc_RuntimeError,
949+
"preexec_fn not supported at interpreter shutdown");
950+
return NULL;
951+
}
946952
if ((preexec_fn != Py_None) && (interp != PyInterpreterState_Main())) {
947953
PyErr_SetString(PyExc_RuntimeError,
948954
"preexec_fn not supported within subinterpreters");

‎Modules/_threadmodule.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1155,6 +1155,11 @@ thread_PyThread_start_new_thread(PyObject *self, PyObject *fargs)
11551155
"thread is not supported for isolated subinterpreters");
11561156
return NULL;
11571157
}
1158+
if (interp->finalizing) {
1159+
PyErr_SetString(PyExc_RuntimeError,
1160+
"can't create new thread at interpreter shutdown");
1161+
return NULL;
1162+
}
11581163

11591164
struct bootstate *boot = PyMem_NEW(struct bootstate, 1);
11601165
if (boot == NULL) {

‎Modules/posixmodule.c‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7620,7 +7620,13 @@ os_fork1_impl(PyObject *module)
76207620
{
76217621
pid_t pid;
76227622

7623-
if (!_Py_IsMainInterpreter(_PyInterpreterState_GET())) {
7623+
PyInterpreterState *interp = _PyInterpreterState_GET();
7624+
if (interp->finalizing) {
7625+
PyErr_SetString(PyExc_RuntimeError,
7626+
"can't fork at interpreter shutdown");
7627+
return NULL;
7628+
}
7629+
if (!_Py_IsMainInterpreter(interp)) {
76247630
PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters");
76257631
return NULL;
76267632
}
@@ -7656,6 +7662,11 @@ os_fork_impl(PyObject *module)
76567662
{
76577663
pid_t pid;
76587664
PyInterpreterState *interp = _PyInterpreterState_GET();
7665+
if (interp->finalizing) {
7666+
PyErr_SetString(PyExc_RuntimeError,
7667+
"can't fork at interpreter shutdown");
7668+
return NULL;
7669+
}
76597670
if (!_PyInterpreterState_HasFeature(interp, Py_RTFLAGS_FORK)) {
76607671
PyErr_SetString(PyExc_RuntimeError,
76617672
"fork not supported for isolated subinterpreters");
@@ -8327,7 +8338,13 @@ os_forkpty_impl(PyObject *module)
83278338
int master_fd = -1;
83288339
pid_t pid;
83298340

8330-
if (!_Py_IsMainInterpreter(_PyInterpreterState_GET())) {
8341+
PyInterpreterState *interp = _PyInterpreterState_GET();
8342+
if (interp->finalizing) {
8343+
PyErr_SetString(PyExc_RuntimeError,
8344+
"can't fork at interpreter shutdown");
8345+
return NULL;
8346+
}
8347+
if (!_Py_IsMainInterpreter(interp)) {
83318348
PyErr_SetString(PyExc_RuntimeError, "fork not supported for subinterpreters");
83328349
return NULL;
83338350
}

0 commit comments

Comments
 (0)