From 292d37e874848e7c7c02869146a932c86174a890 Mon Sep 17 00:00:00 2001 From: Stephen Hansen Date: Tue, 3 Dec 2024 20:43:51 -0500 Subject: [PATCH 1/8] Correct pthread_sigmask in resource_tracker to restore old signals Using SIG_UNBLOCK to remove blocked "ignored signals" may accidentally cause side effects if the calling parent already had said signals blocked to begin with and did not intend to unblock them when creating a pool. Use SIG_SETMASK instead with the previous mask of blocked signals to restore the original blocked set. --- Lib/multiprocessing/resource_tracker.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/multiprocessing/resource_tracker.py b/Lib/multiprocessing/resource_tracker.py index 20ddd9c50e3d88..90e036ae905afa 100644 --- a/Lib/multiprocessing/resource_tracker.py +++ b/Lib/multiprocessing/resource_tracker.py @@ -155,13 +155,14 @@ def ensure_running(self): # that can make the child die before it registers signal handlers # for SIGINT and SIGTERM. The mask is unregistered after spawning # the child. + prev_sigmask = None try: if _HAVE_SIGMASK: - signal.pthread_sigmask(signal.SIG_BLOCK, _IGNORED_SIGNALS) + prev_sigmask = signal.pthread_sigmask(signal.SIG_BLOCK, _IGNORED_SIGNALS) pid = util.spawnv_passfds(exe, args, fds_to_pass) finally: - if _HAVE_SIGMASK: - signal.pthread_sigmask(signal.SIG_UNBLOCK, _IGNORED_SIGNALS) + if prev_sigmask is not None: + signal.pthread_sigmask(signal.SIG_SETMASK, prev_sigmask) except: os.close(w) raise From 5e4797cf2d38aba985c6a1c290a6314ab6d0ab88 Mon Sep 17 00:00:00 2001 From: Stephen Hansen Date: Tue, 3 Dec 2024 20:44:09 -0500 Subject: [PATCH 2/8] Adding NEWS on mp.Pool signal handling --- .../Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst diff --git a/Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst b/Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst new file mode 100644 index 00000000000000..2b35de7407015c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst @@ -0,0 +1,6 @@ +:class:`multiprocessing.Pool` now properly restores blocked signal handlers +of the parent thread when creating processes via either *spawn* or +*forkserver*. Previously it would unblock **SIGTERM** and **SIGINT** in the +parent thread on construction, even if either signal handler was +intentionally blocked prior to the call to construct. Additionally, child +processes can now inherit said blocked signals. From da95ed303a76bd4021ddfd7326b62a5ddbcf1a09 Mon Sep 17 00:00:00 2001 From: Stephen Hansen Date: Tue, 3 Dec 2024 21:10:45 -0500 Subject: [PATCH 3/8] Fixing mp Pool class path in NEWS --- .../next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst b/Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst index 2b35de7407015c..82fcb94f1fa11c 100644 --- a/Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst +++ b/Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst @@ -1,4 +1,4 @@ -:class:`multiprocessing.Pool` now properly restores blocked signal handlers +:class:`multiprocessing.pool.Pool` now properly restores blocked signal handlers of the parent thread when creating processes via either *spawn* or *forkserver*. Previously it would unblock **SIGTERM** and **SIGINT** in the parent thread on construction, even if either signal handler was From dcc52ba2cecc4d24a389afc0870f9aeb7cc45775 Mon Sep 17 00:00:00 2001 From: Stephen Hansen Date: Mon, 9 Dec 2024 23:07:01 -0500 Subject: [PATCH 4/8] Update Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst Co-authored-by: Peter Bierma --- .../Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst b/Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst index 82fcb94f1fa11c..80217bd4a10503 100644 --- a/Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst +++ b/Misc/NEWS.d/next/Library/2024-12-03-20-28-08.gh-issue-127586.zgotYF.rst @@ -1,6 +1,3 @@ :class:`multiprocessing.pool.Pool` now properly restores blocked signal handlers of the parent thread when creating processes via either *spawn* or -*forkserver*. Previously it would unblock **SIGTERM** and **SIGINT** in the -parent thread on construction, even if either signal handler was -intentionally blocked prior to the call to construct. Additionally, child -processes can now inherit said blocked signals. +*forkserver*. From 41731d7863cc783c206ab5b219549d6f935fc3b1 Mon Sep 17 00:00:00 2001 From: Stephen Hansen Date: Thu, 12 Dec 2024 13:43:26 -0500 Subject: [PATCH 5/8] Adding resource_tracker blocked signals test --- Lib/test/_test_multiprocessing.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 80b08b8ac66899..451457bb84c6ca 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -6044,6 +6044,21 @@ def test_resource_tracker_exit_code(self): self._test_resource_tracker_leak_resources( cleanup=cleanup, ) + + def test_resource_tracker_blocked_signals(self): + # + # gh-127586: Check that resource_tracker does not override blocked signals of caller. + # + from multiprocessing.resource_tracker import ResourceTracker + signals = {signal.SIGTERM, signal.SIGINT, signal.SIGUSR1} + + for sig in signals: + signal.pthread_sigmask(signal.SIG_SETMASK, {sig}) + self.assertEqual(signal.pthread_sigmask(signal.SIG_BLOCK, set()), {sig}) + tracker = ResourceTracker() + tracker.ensure_running() + self.assertEqual(signal.pthread_sigmask(signal.SIG_BLOCK, set()), {sig}) + tracker._stop() class TestSimpleQueue(unittest.TestCase): From 47e7b67bc2be16e881498b2ebe3ca1b4d1add567 Mon Sep 17 00:00:00 2001 From: Stephen Hansen Date: Thu, 12 Dec 2024 13:45:20 -0500 Subject: [PATCH 6/8] Linter fix --- Lib/test/_test_multiprocessing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 451457bb84c6ca..fe1558964724da 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -6044,7 +6044,7 @@ def test_resource_tracker_exit_code(self): self._test_resource_tracker_leak_resources( cleanup=cleanup, ) - + def test_resource_tracker_blocked_signals(self): # # gh-127586: Check that resource_tracker does not override blocked signals of caller. From 070e3e8b56a0908c7d5be258ab4a6142aa226ffa Mon Sep 17 00:00:00 2001 From: Stephen Hansen Date: Sun, 15 Dec 2024 13:47:59 -0500 Subject: [PATCH 7/8] Update Lib/test/_test_multiprocessing.py pthread_sigmask is not available on some platforms Co-authored-by: Peter Bierma --- Lib/test/_test_multiprocessing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index fe1558964724da..3d9ce57b59fc3d 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -6044,7 +6044,7 @@ def test_resource_tracker_exit_code(self): self._test_resource_tracker_leak_resources( cleanup=cleanup, ) - + @unittest.skipUnless(hasattr(signal, pthread_sigmask), "pthread_sigmask is not available") def test_resource_tracker_blocked_signals(self): # # gh-127586: Check that resource_tracker does not override blocked signals of caller. From 82d49e3f4a97259c0d9ef541173c804c95b7ebeb Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sun, 15 Dec 2024 10:56:31 -0800 Subject: [PATCH 8/8] fix the test skip --- Lib/test/_test_multiprocessing.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 3d9ce57b59fc3d..01e92f0d8d6b30 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -6044,7 +6044,7 @@ def test_resource_tracker_exit_code(self): self._test_resource_tracker_leak_resources( cleanup=cleanup, ) - @unittest.skipUnless(hasattr(signal, pthread_sigmask), "pthread_sigmask is not available") + @unittest.skipUnless(hasattr(signal, "pthread_sigmask"), "pthread_sigmask is not available") def test_resource_tracker_blocked_signals(self): # # gh-127586: Check that resource_tracker does not override blocked signals of caller.