From c20f566f3ea7efa5bc1ef195075a2ea156740a9a Mon Sep 17 00:00:00 2001 From: Jiucheng Zang Date: Wed, 21 May 2025 23:34:47 -0400 Subject: [PATCH 1/7] Improve thread safety in _PyThread_AfterFork by preserving not-started handles --- Modules/_threadmodule.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 77286ed2a74669..d594d2de51a39a 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -296,6 +296,11 @@ _PyThread_AfterFork(struct _pythread_runtime_state *state) continue; } + // keep not_start handles – they are safe to start in the child + if (handle->state == THREAD_HANDLE_NOT_STARTED){ + continue; + } + // Mark all threads as done. Any attempts to join or detach the // underlying OS thread (if any) could crash. We are the only thread; // it's safe to set this non-atomically. From c144379bbee39e4941d824d5f60f3196cf660b4a Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Thu, 22 May 2025 14:48:21 +0000 Subject: [PATCH 2/7] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2025-05-22-14-48-19.gh-issue-134381.2BXhth.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-05-22-14-48-19.gh-issue-134381.2BXhth.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-05-22-14-48-19.gh-issue-134381.2BXhth.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-22-14-48-19.gh-issue-134381.2BXhth.rst new file mode 100644 index 00000000000000..f685db6d5e6e8e --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-22-14-48-19.gh-issue-134381.2BXhth.rst @@ -0,0 +1 @@ +Improve thread safety in :class:`_PyThread_AfterFork` by preserving not-started handles From 40da2623ed5429e042e367aaa842829f0e0509b4 Mon Sep 17 00:00:00 2001 From: Jiucheng Zang Date: Thu, 22 May 2025 12:06:40 -0400 Subject: [PATCH 3/7] Add a test case, and improve the NEWS.d --- Lib/test/test_thread.py | 12 ++++++++++++ .../2025-05-22-14-48-19.gh-issue-134381.2BXhth.rst | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_thread.py b/Lib/test/test_thread.py index d94e04250c9307..2116bc40d3a8bd 100644 --- a/Lib/test/test_thread.py +++ b/Lib/test/test_thread.py @@ -337,6 +337,18 @@ def func(): handle = thread.start_joinable_thread(func, handle=None) handle.join() + def test_forked_not_started(self): + handle = thread._ThreadHandle() + pid = os.fork() + if pid == 0: + # child process + try: + self.assertTrue(handle.is_alive()) + self.assertFalse(handle.is_done()) + finally: + os._exit(0) + else: + support.wait_process(pid, exitcode=0) class Barrier: def __init__(self, num_threads): diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-05-22-14-48-19.gh-issue-134381.2BXhth.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-22-14-48-19.gh-issue-134381.2BXhth.rst index f685db6d5e6e8e..aa8900296ae2fc 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-05-22-14-48-19.gh-issue-134381.2BXhth.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-05-22-14-48-19.gh-issue-134381.2BXhth.rst @@ -1 +1 @@ -Improve thread safety in :class:`_PyThread_AfterFork` by preserving not-started handles +Fix :exc:`RuntimeError` when using a not-started :class:`threading.Thread` after calling :func:`os.fork` From be1f512664eedf9b91aa7c7d9c6fc56f65940810 Mon Sep 17 00:00:00 2001 From: Jiucheng Zang Date: Thu, 22 May 2025 12:36:41 -0400 Subject: [PATCH 4/7] Rearrage test location, skip fork test on windows --- Lib/test/test_thread.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_thread.py b/Lib/test/test_thread.py index 2116bc40d3a8bd..e2c59dfc83fe0e 100644 --- a/Lib/test/test_thread.py +++ b/Lib/test/test_thread.py @@ -337,18 +337,6 @@ def func(): handle = thread.start_joinable_thread(func, handle=None) handle.join() - def test_forked_not_started(self): - handle = thread._ThreadHandle() - pid = os.fork() - if pid == 0: - # child process - try: - self.assertTrue(handle.is_alive()) - self.assertFalse(handle.is_done()) - finally: - os._exit(0) - else: - support.wait_process(pid, exitcode=0) class Barrier: def __init__(self, num_threads): @@ -451,6 +439,21 @@ def fork_thread(read_fd, write_fd): self.assertIsNotNone(pid) support.wait_process(pid, exitcode=0) + @support.requires_fork() + @threading_helper.reap_threads + def test_forked_not_started(self): + handle = thread._ThreadHandle() + pid = os.fork() + if pid == 0: + # child process + try: + self.assertTrue(handle.is_alive()) + self.assertFalse(handle.is_done()) + finally: + os._exit(0) + else: + support.wait_process(pid, exitcode=0) + def tearDown(self): try: os.close(self.read_fd) From 9c8f30306cff1db097b9452e7e24cb9a3e8c4ff7 Mon Sep 17 00:00:00 2001 From: Jiucheng Zang Date: Thu, 22 May 2025 18:58:41 -0400 Subject: [PATCH 5/7] Enhance comment message, recreate test scenario in _test_multiprocessing --- Lib/test/_test_multiprocessing.py | 22 ++++++++++++++++++++++ Lib/test/test_thread.py | 15 --------------- Modules/_threadmodule.c | 3 ++- 3 files changed, 24 insertions(+), 16 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 6a20a1eb03e32b..6673dba8fb45e0 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -6844,6 +6844,28 @@ def f(x): return x*x self.assertEqual("332833500", out.decode('utf-8').strip()) self.assertFalse(err, msg=err.decode('utf-8')) + @support.requires_fork() + def test_forked_not_started(self): + # gh-134381 + # Ensure that a forked parent process, which has not been started yet, + # can be initiated within the child process. + + ctx = multiprocessing.get_context("fork") # local “fork” cont + q = ctx.Queue() + t = threading.Thread(target=lambda: q.put("done"), daemon=True) + + def child(): + t.start() + t.join() + + p = ctx.Process(target=child) + p.start() + p.join(support.SHORT_TIMEOUT) + + self.assertEqual(p.exitcode, 0) + self.assertEqual(q.get_nowait(), "done") + close_queue(q) + # # Mixins diff --git a/Lib/test/test_thread.py b/Lib/test/test_thread.py index e2c59dfc83fe0e..d94e04250c9307 100644 --- a/Lib/test/test_thread.py +++ b/Lib/test/test_thread.py @@ -439,21 +439,6 @@ def fork_thread(read_fd, write_fd): self.assertIsNotNone(pid) support.wait_process(pid, exitcode=0) - @support.requires_fork() - @threading_helper.reap_threads - def test_forked_not_started(self): - handle = thread._ThreadHandle() - pid = os.fork() - if pid == 0: - # child process - try: - self.assertTrue(handle.is_alive()) - self.assertFalse(handle.is_done()) - finally: - os._exit(0) - else: - support.wait_process(pid, exitcode=0) - def tearDown(self): try: os.close(self.read_fd) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index d594d2de51a39a..d0b3941321812f 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -296,7 +296,8 @@ _PyThread_AfterFork(struct _pythread_runtime_state *state) continue; } - // keep not_start handles – they are safe to start in the child + // Keep handles for threads that have not been started yet. They are + // safe to start in the child process. if (handle->state == THREAD_HANDLE_NOT_STARTED){ continue; } From 5e0158c49e7319987fd52aee330af1e4dca40580 Mon Sep 17 00:00:00 2001 From: Jiucheng Zang Date: Fri, 23 May 2025 00:01:31 -0400 Subject: [PATCH 6/7] Enhance test case setup, skip if get_start_method() is not set to 'fork' --- Lib/test/_test_multiprocessing.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 6673dba8fb45e0..75f31d858d3306 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -6844,21 +6844,21 @@ def f(x): return x*x self.assertEqual("332833500", out.decode('utf-8').strip()) self.assertFalse(err, msg=err.decode('utf-8')) - @support.requires_fork() - def test_forked_not_started(self): - # gh-134381 - # Ensure that a forked parent process, which has not been started yet, - # can be initiated within the child process. - - ctx = multiprocessing.get_context("fork") # local “fork” cont - q = ctx.Queue() + def test_forked_thread_not_started(self): + # gh-134381: Ensure that a thread that has not been started yet in + # the parent process can be started within a forked child process. + + if multiprocessing.get_start_method() != "fork": + self.skipTest("fork specific test") + + q = multiprocessing.Queue() t = threading.Thread(target=lambda: q.put("done"), daemon=True) def child(): t.start() t.join() - p = ctx.Process(target=child) + p = multiprocessing.Process(target=child) p.start() p.join(support.SHORT_TIMEOUT) From 9827b43b7d2ac7bf65c6726a1fddaf181e409019 Mon Sep 17 00:00:00 2001 From: Jiucheng Zang Date: Fri, 23 May 2025 13:47:26 -0400 Subject: [PATCH 7/7] Addressed formatting --- Modules/_threadmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 48f9e5b4dddd87..cc83be4b5ff311 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -298,7 +298,7 @@ _PyThread_AfterFork(struct _pythread_runtime_state *state) // Keep handles for threads that have not been started yet. They are // safe to start in the child process. - if (handle->state == THREAD_HANDLE_NOT_STARTED){ + if (handle->state == THREAD_HANDLE_NOT_STARTED) { continue; }