From 49785b06ded19c7c4afce186bac90fea707470ea Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 22 Jan 2024 16:14:42 +0200 Subject: [PATCH] gh-102512: Turn _DummyThread into _MainThread after os.fork() called from a foreign thread (GH-113261) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Always set a _MainThread as a main thread after os.fork() is called from a thread started not by the threading module. A new _MainThread was already set as a new main thread after fork if threading.current_thread() was not called for a foreign thread before fork. Now, if it was called before fork, the implicitly created _DummyThread will be turned into _MainThread after fork. It fixes, in particularly, an incompatibility of _DummyThread with the threading shutdown logic which relies on the main thread having tstate_lock. Co-authored-by: Marek Marczykowski-Górecki --- Lib/test/test_threading.py | 98 +++++++++++++++++-- Lib/threading.py | 9 +- ...-03-08-00-02-30.gh-issue-102512.LiugDr.rst | 3 + 3 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-03-08-00-02-30.gh-issue-102512.LiugDr.rst diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py index 3060af44fd7e3d..7160c53d691ba2 100644 --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -115,6 +115,7 @@ def tearDown(self): class ThreadTests(BaseTestCase): + maxDiff = 9999 @cpython_only def test_name(self): @@ -676,19 +677,25 @@ def test_main_thread_after_fork(self): import os, threading from test import support + ident = threading.get_ident() pid = os.fork() if pid == 0: + print("current ident", threading.get_ident() == ident) main = threading.main_thread() - print(main.name) - print(main.ident == threading.current_thread().ident) - print(main.ident == threading.get_ident()) + print("main", main.name) + print("main ident", main.ident == ident) + print("current is main", threading.current_thread() is main) else: support.wait_process(pid, exitcode=0) """ _, out, err = assert_python_ok("-c", code) data = out.decode().replace('\r', '') self.assertEqual(err, b"") - self.assertEqual(data, "MainThread\nTrue\nTrue\n") + self.assertEqual(data, + "current ident True\n" + "main MainThread\n" + "main ident True\n" + "current is main True\n") @skip_unless_reliable_fork @unittest.skipUnless(hasattr(os, 'waitpid'), "test needs os.waitpid()") @@ -698,15 +705,17 @@ def test_main_thread_after_fork_from_nonmain_thread(self): from test import support def func(): + ident = threading.get_ident() with warnings.catch_warnings(record=True) as ws: warnings.filterwarnings( "always", category=DeprecationWarning) pid = os.fork() if pid == 0: + print("current ident", threading.get_ident() == ident) main = threading.main_thread() - print(main.name) - print(main.ident == threading.current_thread().ident) - print(main.ident == threading.get_ident()) + print("main", main.name, type(main).__name__) + print("main ident", main.ident == ident) + print("current is main", threading.current_thread() is main) # stdout is fully buffered because not a tty, # we have to flush before exit. sys.stdout.flush() @@ -722,7 +731,80 @@ def func(): _, out, err = assert_python_ok("-c", code) data = out.decode().replace('\r', '') self.assertEqual(err.decode('utf-8'), "") - self.assertEqual(data, "Thread-1 (func)\nTrue\nTrue\n") + self.assertEqual(data, + "current ident True\n" + "main Thread-1 (func) Thread\n" + "main ident True\n" + "current is main True\n" + ) + + @unittest.skipIf(sys.platform in platforms_to_skip, "due to known OS bug") + @support.requires_fork() + @unittest.skipUnless(hasattr(os, 'waitpid'), "test needs os.waitpid()") + def test_main_thread_after_fork_from_foreign_thread(self, create_dummy=False): + code = """if 1: + import os, threading, sys, traceback, _thread + from test import support + + def func(lock): + ident = threading.get_ident() + if %s: + # call current_thread() before fork to allocate DummyThread + current = threading.current_thread() + print("current", current.name, type(current).__name__) + print("ident in _active", ident in threading._active) + # flush before fork, so child won't flush it again + sys.stdout.flush() + pid = os.fork() + if pid == 0: + print("current ident", threading.get_ident() == ident) + main = threading.main_thread() + print("main", main.name, type(main).__name__) + print("main ident", main.ident == ident) + print("current is main", threading.current_thread() is main) + print("_dangling", [t.name for t in list(threading._dangling)]) + # stdout is fully buffered because not a tty, + # we have to flush before exit. + sys.stdout.flush() + try: + threading._shutdown() + os._exit(0) + except: + traceback.print_exc() + sys.stderr.flush() + os._exit(1) + else: + try: + support.wait_process(pid, exitcode=0) + except Exception: + # avoid 'could not acquire lock for + # <_io.BufferedWriter name=''> at interpreter shutdown,' + traceback.print_exc() + sys.stderr.flush() + finally: + lock.release() + + join_lock = _thread.allocate_lock() + join_lock.acquire() + th = _thread.start_new_thread(func, (join_lock,)) + join_lock.acquire() + """ % create_dummy + # "DeprecationWarning: This process is multi-threaded, use of fork() + # may lead to deadlocks in the child" + _, out, err = assert_python_ok("-W", "ignore::DeprecationWarning", "-c", code) + data = out.decode().replace('\r', '') + self.assertEqual(err.decode(), "") + self.assertEqual(data, + ("current Dummy-1 _DummyThread\n" if create_dummy else "") + + f"ident in _active {create_dummy!s}\n" + + "current ident True\n" + "main MainThread _MainThread\n" + "main ident True\n" + "current is main True\n" + "_dangling ['MainThread']\n") + + def test_main_thread_after_fork_from_dummy_thread(self, create_dummy=False): + self.test_main_thread_after_fork_from_foreign_thread(create_dummy=True) def test_main_thread_during_shutdown(self): # bpo-31516: current_thread() should still point to the main thread diff --git a/Lib/threading.py b/Lib/threading.py index 85aff58968082d..c561800a128059 100644 --- a/Lib/threading.py +++ b/Lib/threading.py @@ -1489,7 +1489,6 @@ class _DummyThread(Thread): def __init__(self): Thread.__init__(self, name=_newname("Dummy-%d"), daemon=_daemon_threads_allowed()) - self._started.set() self._set_ident() if _HAVE_THREAD_NATIVE_ID: @@ -1508,6 +1507,14 @@ def is_alive(self): def join(self, timeout=None): raise RuntimeError("cannot join a dummy thread") + def _after_fork(self, new_ident=None): + if new_ident is not None: + self.__class__ = _MainThread + self._name = 'MainThread' + self._daemonic = False + self._set_tstate_lock() + Thread._after_fork(self, new_ident=new_ident) + # Global API functions diff --git a/Misc/NEWS.d/next/Library/2023-03-08-00-02-30.gh-issue-102512.LiugDr.rst b/Misc/NEWS.d/next/Library/2023-03-08-00-02-30.gh-issue-102512.LiugDr.rst new file mode 100644 index 00000000000000..659cba73cbf34e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-03-08-00-02-30.gh-issue-102512.LiugDr.rst @@ -0,0 +1,3 @@ +When :func:`os.fork` is called from a foreign thread (aka ``_DummyThread``), +the type of the thread in a child process is changed to ``_MainThread``. +Also changed its name and daemonic status, it can be now joined.