Skip to content

Commit

Permalink
pythongh-113964: Don't prevent new threads until all non-daemon threa…
Browse files Browse the repository at this point in the history
…ds exit (python#116677)

Starting in Python 3.12, we prevented calling fork() and starting new threads
during interpreter finalization (shutdown). This has led to a number of
regressions and flaky tests. We should not prevent starting new threads
(or `fork()`) until all non-daemon threads exit and finalization starts in
earnest.

This changes the checks to use `_PyInterpreterState_GetFinalizing(interp)`,
which is set immediately before terminating non-daemon threads.
  • Loading branch information
colesbury authored and adorilson committed Mar 25, 2024
1 parent 40d80e2 commit adda0e9
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 27 deletions.
17 changes: 9 additions & 8 deletions Lib/test/test_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -5357,20 +5357,21 @@ 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):
def test_fork_at_finalization(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)
class AtFinalization:
def __del__(self):
print("OK")
pid = os.fork()
if pid != 0:
print("shouldn't be printed")
at_finalization = AtFinalization()
"""
_, out, err = assert_python_ok("-c", code)
self.assertEqual(b"", out)
self.assertEqual(b"OK\n", out)
self.assertIn(b"can't fork at interpreter shutdown", err)


Expand Down
13 changes: 7 additions & 6 deletions Lib/test/test_subprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -3398,14 +3398,15 @@ def test_preexec_at_exit(self):
def dummy():
pass
def exit_handler():
subprocess.Popen({ZERO_RETURN_CMD}, preexec_fn=dummy)
print("shouldn't be printed")
atexit.register(exit_handler)
class AtFinalization:
def __del__(self):
print("OK")
subprocess.Popen({ZERO_RETURN_CMD}, preexec_fn=dummy)
print("shouldn't be printed")
at_finalization = AtFinalization()
"""
_, out, err = assert_python_ok("-c", code)
self.assertEqual(out, b'')
self.assertEqual(out.strip(), b"OK")
self.assertIn(b"preexec_fn not supported at interpreter shutdown", err)

@unittest.skipIf(not sysconfig.get_config_var("HAVE_VFORK"),
Expand Down
38 changes: 31 additions & 7 deletions Lib/test/test_threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -1154,21 +1154,21 @@ def import_threading():
self.assertEqual(out, b'')
self.assertEqual(err, b'')

def test_start_new_thread_at_exit(self):
def test_start_new_thread_at_finalization(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)
class AtFinalization:
def __del__(self):
print("OK")
_thread.start_new_thread(f, ())
at_finalization = AtFinalization()
"""
_, out, err = assert_python_ok("-c", code)
self.assertEqual(out, b'')
self.assertEqual(out.strip(), b"OK")
self.assertIn(b"can't create new thread at interpreter shutdown", err)

class ThreadJoinOnShutdown(BaseTestCase):
Expand Down Expand Up @@ -1297,6 +1297,30 @@ def main():
rc, out, err = assert_python_ok('-c', script)
self.assertFalse(err)

def test_thread_from_thread(self):
script = """if True:
import threading
import time
def thread2():
time.sleep(0.05)
print("OK")
def thread1():
time.sleep(0.05)
t2 = threading.Thread(target=thread2)
t2.start()
t = threading.Thread(target=thread1)
t.start()
# do not join() -- the interpreter waits for non-daemon threads to
# finish.
"""
rc, out, err = assert_python_ok('-c', script)
self.assertEqual(err, b"")
self.assertEqual(out.strip(), b"OK")
self.assertEqual(rc, 0)

@skip_unless_reliable_fork
def test_reinit_tls_after_fork(self):
# Issue #13817: fork() would deadlock in a multithreaded program with
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Starting new threads and process creation through :func:`os.fork` are now
only prevented once all non-daemon threads exit.
4 changes: 3 additions & 1 deletion Modules/_posixsubprocess.c
Original file line number Diff line number Diff line change
Expand Up @@ -1031,7 +1031,9 @@ 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) {
if ((preexec_fn != Py_None) &&
_PyInterpreterState_GetFinalizing(interp) != NULL)
{
PyErr_SetString(PyExc_PythonFinalizationError,
"preexec_fn not supported at interpreter shutdown");
return NULL;
Expand Down
2 changes: 1 addition & 1 deletion Modules/_threadmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -1729,7 +1729,7 @@ do_start_new_thread(thread_module_state *state, PyObject *func, PyObject *args,
"thread is not supported for isolated subinterpreters");
return -1;
}
if (interp->finalizing) {
if (_PyInterpreterState_GetFinalizing(interp) != NULL) {
PyErr_SetString(PyExc_PythonFinalizationError,
"can't create new thread at interpreter shutdown");
return -1;
Expand Down
6 changes: 3 additions & 3 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -7842,7 +7842,7 @@ os_fork1_impl(PyObject *module)
pid_t pid;

PyInterpreterState *interp = _PyInterpreterState_GET();
if (interp->finalizing) {
if (_PyInterpreterState_GetFinalizing(interp) != NULL) {
PyErr_SetString(PyExc_PythonFinalizationError,
"can't fork at interpreter shutdown");
return NULL;
Expand Down Expand Up @@ -7886,7 +7886,7 @@ os_fork_impl(PyObject *module)
{
pid_t pid;
PyInterpreterState *interp = _PyInterpreterState_GET();
if (interp->finalizing) {
if (_PyInterpreterState_GetFinalizing(interp) != NULL) {
PyErr_SetString(PyExc_PythonFinalizationError,
"can't fork at interpreter shutdown");
return NULL;
Expand Down Expand Up @@ -8719,7 +8719,7 @@ os_forkpty_impl(PyObject *module)
pid_t pid;

PyInterpreterState *interp = _PyInterpreterState_GET();
if (interp->finalizing) {
if (_PyInterpreterState_GetFinalizing(interp) != NULL) {
PyErr_SetString(PyExc_PythonFinalizationError,
"can't fork at interpreter shutdown");
return NULL;
Expand Down
2 changes: 1 addition & 1 deletion Objects/unicodeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,7 @@ unicode_check_encoding_errors(const char *encoding, const char *errors)

/* Disable checks during Python finalization. For example, it allows to
call _PyObject_Dump() during finalization for debugging purpose. */
if (interp->finalizing) {
if (_PyInterpreterState_GetFinalizing(interp) != NULL) {
return 0;
}

Expand Down

0 comments on commit adda0e9

Please sign in to comment.