gh-100228: Warn from os.fork() if other threads exist. (#100229)

Not comprehensive, best effort warning. There are cases when threads exist on some platforms that this code cannot detect. macOS when API permissions allow and Linux with a readable /proc procfs present are the currently supported cases where a warning should show up reliably.

Starting with a DeprecationWarning for now, it is less disruptive than something like RuntimeWarning and most likely to only be seen in people's CI tests - a good place to start with this messaging.

Include/internal/pycore_global_strings.h

@@ -219,6 +219,7 @@ struct _Py_global_strings {
         STRUCT_FOR_ID(__xor__)
         STRUCT_FOR_ID(_abc_impl)
         STRUCT_FOR_ID(_abstract_)
+        STRUCT_FOR_ID(_active)
         STRUCT_FOR_ID(_annotation)
         STRUCT_FOR_ID(_anonymous_)
         STRUCT_FOR_ID(_argtypes_)
@@ -239,6 +240,7 @@ struct _Py_global_strings {
         STRUCT_FOR_ID(_initializing)
         STRUCT_FOR_ID(_is_text_encoding)
         STRUCT_FOR_ID(_length_)
+        STRUCT_FOR_ID(_limbo)
         STRUCT_FOR_ID(_lock_unlock_module)
         STRUCT_FOR_ID(_loop)
         STRUCT_FOR_ID(_needs_com_addref_)

Lib/test/fork_wait.py

@@ -13,6 +13,7 @@
 import threading
 from test import support
 from test.support import threading_helper
+import warnings
 
 
 LONGSLEEP = 2
@@ -63,19 +64,17 @@ def test_wait(self):
 
         prefork_lives = self.alive.copy()
 
-        if sys.platform in ['unixware7']:
-            cpid = os.fork1()
-        else:
-            cpid = os.fork()
-
-        if cpid == 0:
-            # Child
-            time.sleep(LONGSLEEP)
-            n = 0
-            for key in self.alive:
-                if self.alive[key] != prefork_lives[key]:
-                    n += 1
-            os._exit(n)
-        else:
-            # Parent
-            self.wait_impl(cpid, exitcode=0)
+        # Ignore the warning about fork with threads.
+        with warnings.catch_warnings(category=DeprecationWarning,
+                                     action="ignore"):
+            if (cpid := os.fork()) == 0:
+                # Child
+                time.sleep(LONGSLEEP)
+                n = 0
+                for key in self.alive:
+                    if self.alive[key] != prefork_lives[key]:
+                        n += 1
+                os._exit(n)
+            else:
+                # Parent
+                self.wait_impl(cpid, exitcode=0)

Lib/test/test_os.py

@@ -4577,6 +4577,34 @@ def test_fork(self):
         assert_python_ok("-c", code)
         assert_python_ok("-c", code, PYTHONMALLOC="malloc_debug")
 
+    @unittest.skipUnless(sys.platform in ("linux", "darwin"),
+                         "Only Linux and macOS detect this today.")
+    def test_fork_warns_when_non_python_thread_exists(self):
+        code = """if 1:
+            import os, threading, warnings
+            from _testcapi import _spawn_pthread_waiter, _end_spawned_pthread
+            _spawn_pthread_waiter()
+            try:
+                with warnings.catch_warnings(record=True) as ws:
+                    warnings.filterwarnings(
+                            "always", category=DeprecationWarning)
+                    if os.fork() == 0:
+                        assert not ws, f"unexpected warnings in child: {ws}"
+                        os._exit(0)  # child
+                    else:
+                        assert ws[0].category == DeprecationWarning, ws[0]
+                        assert 'fork' in str(ws[0].message), ws[0]
+                        # Waiting allows an error in the child to hit stderr.
+                        exitcode = os.wait()[1]
+                        assert exitcode == 0, f"child exited {exitcode}"
+                assert threading.active_count() == 1, threading.enumerate()
+            finally:
+                _end_spawned_pthread()
+        """
+        _, out, err = assert_python_ok("-c", code, PYTHONOPTIMIZE='0')
+        self.assertEqual(err.decode("utf-8"), "")
+        self.assertEqual(out.decode("utf-8"), "")
+
 
 # Only test if the C version is provided, otherwise TestPEP519 already tested
 # the pure Python implementation.

Lib/test/test_thread.py

@@ -5,6 +5,7 @@
 from test.support import threading_helper
 import _thread as thread
 import time
+import warnings
 import weakref
 
 from test import lock_tests
@@ -238,11 +239,13 @@ def test_forkinthread(self):
         def fork_thread(read_fd, write_fd):
             nonlocal pid
 
-            # fork in a thread
-            pid = os.fork()
-            if pid:
-                # parent process
-                return
+            # Ignore the warning about fork with threads.
+            with warnings.catch_warnings(category=DeprecationWarning,
+                                         action="ignore"):
+                # fork in a thread (DANGER, undefined per POSIX)
+                if (pid := os.fork()):
+                    # parent process
+                    return
 
             # child process
             try:

Lib/test/test_threading.py

@@ -20,6 +20,7 @@
 import signal
 import textwrap
 import traceback
+import warnings
 
 from unittest import mock
 from test import lock_tests
@@ -563,7 +564,7 @@ def test_dummy_thread_after_fork(self):
         # Issue #14308: a dummy thread in the active list doesn't mess up
         # the after-fork mechanism.
         code = """if 1:
-            import _thread, threading, os, time
+            import _thread, threading, os, time, warnings
 
             def background_thread(evt):
                 # Creates and registers the _DummyThread instance
@@ -575,11 +576,16 @@ def background_thread(evt):
             _thread.start_new_thread(background_thread, (evt,))
             evt.wait()
             assert threading.active_count() == 2, threading.active_count()
-            if os.fork() == 0:
-                assert threading.active_count() == 1, threading.active_count()
-                os._exit(0)
-            else:
-                os.wait()
+            with warnings.catch_warnings(record=True) as ws:
+                warnings.filterwarnings(
+                        "always", category=DeprecationWarning)
+                if os.fork() == 0:
+                    assert threading.active_count() == 1, threading.active_count()
+                    os._exit(0)
+                else:
+                    assert ws[0].category == DeprecationWarning, ws[0]
+                    assert 'fork' in str(ws[0].message), ws[0]
+                    os.wait()
         """
         _, out, err = assert_python_ok("-c", code)
         self.assertEqual(out, b'')
@@ -598,13 +604,15 @@ def test_is_alive_after_fork(self):
         for i in range(20):
             t = threading.Thread(target=lambda: None)
             t.start()
-            pid = os.fork()
-            if pid == 0:
-                os._exit(11 if t.is_alive() else 10)
-            else:
-                t.join()
+            # Ignore the warning about fork with threads.
+            with warnings.catch_warnings(category=DeprecationWarning,
+                                         action="ignore"):
+                if (pid := os.fork()) == 0:
+                    os._exit(11 if t.is_alive() else 10)
+                else:
+                    t.join()
 
-                support.wait_process(pid, exitcode=10)
+                    support.wait_process(pid, exitcode=10)
 
     def test_main_thread(self):
         main = threading.main_thread()
@@ -645,29 +653,34 @@ def test_main_thread_after_fork(self):
     @unittest.skipUnless(hasattr(os, 'waitpid'), "test needs os.waitpid()")
     def test_main_thread_after_fork_from_nonmain_thread(self):
         code = """if 1:
-            import os, threading, sys
+            import os, threading, sys, warnings
             from test import support
 
             def func():
-                pid = os.fork()
-                if pid == 0:
-                    main = threading.main_thread()
-                    print(main.name)
-                    print(main.ident == threading.current_thread().ident)
-                    print(main.ident == threading.get_ident())
-                    # stdout is fully buffered because not a tty,
-                    # we have to flush before exit.
-                    sys.stdout.flush()
-                else:
-                    support.wait_process(pid, exitcode=0)
+                with warnings.catch_warnings(record=True) as ws:
+                    warnings.filterwarnings(
+                            "always", category=DeprecationWarning)
+                    pid = os.fork()
+                    if pid == 0:
+                        main = threading.main_thread()
+                        print(main.name)
+                        print(main.ident == threading.current_thread().ident)
+                        print(main.ident == threading.get_ident())
+                        # stdout is fully buffered because not a tty,
+                        # we have to flush before exit.
+                        sys.stdout.flush()
+                    else:
+                        assert ws[0].category == DeprecationWarning, ws[0]
+                        assert 'fork' in str(ws[0].message), ws[0]
+                        support.wait_process(pid, exitcode=0)
 
             th = threading.Thread(target=func)
             th.start()
             th.join()
         """
         _, out, err = assert_python_ok("-c", code)
         data = out.decode().replace('\r', '')
-        self.assertEqual(err, b"")
+        self.assertEqual(err.decode('utf-8'), "")
         self.assertEqual(data, "Thread-1 (func)\nTrue\nTrue\n")
 
     def test_main_thread_during_shutdown(self):
@@ -1173,15 +1186,18 @@ def do_fork_and_wait():
             else:
                 os._exit(50)
 
-        # start a bunch of threads that will fork() child processes
-        threads = []
-        for i in range(16):
-            t = threading.Thread(target=do_fork_and_wait)
-            threads.append(t)
-            t.start()
+        # Ignore the warning about fork with threads.
+        with warnings.catch_warnings(category=DeprecationWarning,
+                                     action="ignore"):
+            # start a bunch of threads that will fork() child processes
+            threads = []
+            for i in range(16):
+                t = threading.Thread(target=do_fork_and_wait)
+                threads.append(t)
+                t.start()
 
-        for t in threads:
-            t.join()
+            for t in threads:
+                t.join()
 
     @support.requires_fork()
     def test_clear_threads_states_after_fork(self):
@@ -1194,18 +1210,22 @@ def test_clear_threads_states_after_fork(self):
             threads.append(t)
             t.start()
 
-        pid = os.fork()
-        if pid == 0:
-            # check that threads states have been cleared
-            if len(sys._current_frames()) == 1:
-                os._exit(51)
-            else:
-                os._exit(52)
-        else:
-            support.wait_process(pid, exitcode=51)
-
-        for t in threads:
-            t.join()
+        try:
+            # Ignore the warning about fork with threads.
+            with warnings.catch_warnings(category=DeprecationWarning,
+                                         action="ignore"):
+                pid = os.fork()
+                if pid == 0:
+                    # check that threads states have been cleared
+                    if len(sys._current_frames()) == 1:
+                        os._exit(51)
+                    else:
+                        os._exit(52)
+                else:
+                    support.wait_process(pid, exitcode=51)
+        finally:
+            for t in threads:
+                t.join()
 
 
 class SubinterpThreadingTests(BaseTestCase):

Lib/threading.py

@@ -1490,6 +1490,8 @@ def active_count():
     enumerate().
 
     """
+    # NOTE: if the logic in here ever changes, update Modules/posixmodule.c
+    # warn_about_fork_with_threads() to match.
     with _active_limbo_lock:
         return len(_active) + len(_limbo)
 

Misc/NEWS.d/next/Library/2022-12-13-17-29-09.gh-issue-100228.bgtzMV.rst

@@ -0,0 +1,5 @@
+A :exc:`DeprecationWarning` may be raised when :func:`os.fork()` or
+:func:`os.forkpty()` is called from multi-threaded processes.  Forking
+with threads is unsafe and can cause deadlocks, crashes and subtle
+problems. Lack of a warning does not indicate that the fork call was
+actually safe, as Python may not be aware of all threads.