From ec4e7e0d32cf813b5fee8c086ec91ae1bda965fd Mon Sep 17 00:00:00 2001 From: albanD Date: Mon, 28 Aug 2023 10:32:35 -0400 Subject: [PATCH 1/8] Support nesting and add test --- Lib/multiprocessing/synchronize.py | 2 +- Lib/test/_test_multiprocessing.py | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/Lib/multiprocessing/synchronize.py b/Lib/multiprocessing/synchronize.py index 2328d332123082..436e97e191ec91 100644 --- a/Lib/multiprocessing/synchronize.py +++ b/Lib/multiprocessing/synchronize.py @@ -103,7 +103,7 @@ def __getstate__(self): if sys.platform == 'win32': h = context.get_spawning_popen().duplicate_for_child(sl.handle) else: - if self.is_fork_ctx: + if getattr(self, "is_fork_ctx", False): raise RuntimeError('A SemLock created in a fork context is being ' 'shared with a process in a spawn context. This is ' 'not supported. Please use the same context to create ' diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 343e9dcf769e86..38644994b8df8c 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -5342,6 +5342,16 @@ def test_ignore_listener(self): finally: conn.close() +# Utility functions used as target for spawn Process +def put_one_in_queue(queue): + queue.put(1) + +def put_two_and_nest_once(queue): + queue.put(2) + process = multiprocessing.Process(target=put_one_in_queue, args=(queue,)) + process.start() + process.join() + class TestStartMethod(unittest.TestCase): @classmethod def _check_context(cls, conn): @@ -5443,6 +5453,19 @@ def test_mixed_startmethod(self): p.start() p.join() + def test_nested_startmethod(self): + queue = multiprocessing.Queue() + + process = multiprocessing.Process(target=put_two_and_nest_once, args=(queue,)) + process.start() + process.join() + + results = [] + while not queue.empty(): + results.append(queue.get()) + + self.assertEqual(results, [2, 1]) + @unittest.skipIf(sys.platform == "win32", "test semantics don't make sense on Windows") From 1b62ab02d61e75a7bbcda20e21ba0d36007fb09a Mon Sep 17 00:00:00 2001 From: albanD Date: Wed, 30 Aug 2023 10:47:14 -0400 Subject: [PATCH 2/8] Cleanup test code --- Lib/test/_test_multiprocessing.py | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index 38644994b8df8c..7a851d3df78781 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -5342,16 +5342,6 @@ def test_ignore_listener(self): finally: conn.close() -# Utility functions used as target for spawn Process -def put_one_in_queue(queue): - queue.put(1) - -def put_two_and_nest_once(queue): - queue.put(2) - process = multiprocessing.Process(target=put_one_in_queue, args=(queue,)) - process.start() - process.join() - class TestStartMethod(unittest.TestCase): @classmethod def _check_context(cls, conn): @@ -5453,10 +5443,23 @@ def test_mixed_startmethod(self): p.start() p.join() + @classmethod + def _put_one_in_queue(cls, queue): + queue.put(1) + + @classmethod + def _put_two_and_nest_once(cls, queue): + queue.put(2) + process = multiprocessing.Process(target=cls._put_one_in_queue, args=(queue,)) + process.start() + process.join() + def test_nested_startmethod(self): + # gh-108520: Regression test to ensure that child process can send its + # arguments to another process queue = multiprocessing.Queue() - process = multiprocessing.Process(target=put_two_and_nest_once, args=(queue,)) + process = multiprocessing.Process(target=self._put_two_and_nest_once, args=(queue,)) process.start() process.join() From 9212635bdbacf8ba471c8d1562f8114edeaa5bd2 Mon Sep 17 00:00:00 2001 From: albanD Date: Wed, 30 Aug 2023 11:15:54 -0400 Subject: [PATCH 3/8] Properly set is_fork_ctx in set_state --- Lib/multiprocessing/synchronize.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Lib/multiprocessing/synchronize.py b/Lib/multiprocessing/synchronize.py index 436e97e191ec91..eaf61c3c45916b 100644 --- a/Lib/multiprocessing/synchronize.py +++ b/Lib/multiprocessing/synchronize.py @@ -103,7 +103,7 @@ def __getstate__(self): if sys.platform == 'win32': h = context.get_spawning_popen().duplicate_for_child(sl.handle) else: - if getattr(self, "is_fork_ctx", False): + if self.is_fork_ctx: raise RuntimeError('A SemLock created in a fork context is being ' 'shared with a process in a spawn context. This is ' 'not supported. Please use the same context to create ' @@ -115,6 +115,8 @@ def __setstate__(self, state): self._semlock = _multiprocessing.SemLock._rebuild(*state) util.debug('recreated blocker with handle %r' % state[0]) self._make_methods() + # SemLock created in fork context cannot be serialized + self.is_fork_ctx = False @staticmethod def _make_name(): From c0ee44340d96763f4ab209c41013ca2da8e4e7cc Mon Sep 17 00:00:00 2001 From: albanD Date: Wed, 30 Aug 2023 11:16:11 -0400 Subject: [PATCH 4/8] Rename is_fork_ctx to _is_fork_ctx --- Lib/multiprocessing/synchronize.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/multiprocessing/synchronize.py b/Lib/multiprocessing/synchronize.py index eaf61c3c45916b..fb46d9bc029a6a 100644 --- a/Lib/multiprocessing/synchronize.py +++ b/Lib/multiprocessing/synchronize.py @@ -50,8 +50,8 @@ class SemLock(object): def __init__(self, kind, value, maxvalue, *, ctx): if ctx is None: ctx = context._default_context.get_context() - self.is_fork_ctx = ctx.get_start_method() == 'fork' - unlink_now = sys.platform == 'win32' or self.is_fork_ctx + self._is_fork_ctx = ctx.get_start_method() == 'fork' + unlink_now = sys.platform == 'win32' or self._is_fork_ctx for i in range(100): try: sl = self._semlock = _multiprocessing.SemLock( @@ -103,7 +103,7 @@ def __getstate__(self): if sys.platform == 'win32': h = context.get_spawning_popen().duplicate_for_child(sl.handle) else: - if self.is_fork_ctx: + if self._is_fork_ctx: raise RuntimeError('A SemLock created in a fork context is being ' 'shared with a process in a spawn context. This is ' 'not supported. Please use the same context to create ' @@ -116,7 +116,7 @@ def __setstate__(self, state): util.debug('recreated blocker with handle %r' % state[0]) self._make_methods() # SemLock created in fork context cannot be serialized - self.is_fork_ctx = False + self._is_fork_ctx = False @staticmethod def _make_name(): From 19225fcbabaa7f0885f37544f6b80b9241cf6b06 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 30 Aug 2023 15:41:48 +0000 Subject: [PATCH 5/8] =?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 --- .../2023-08-30-15-41-47.gh-issue-108520.u0ZGP_.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-08-30-15-41-47.gh-issue-108520.u0ZGP_.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-08-30-15-41-47.gh-issue-108520.u0ZGP_.rst b/Misc/NEWS.d/next/Core and Builtins/2023-08-30-15-41-47.gh-issue-108520.u0ZGP_.rst new file mode 100644 index 00000000000000..5a59e9fa1feaf3 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-08-30-15-41-47.gh-issue-108520.u0ZGP_.rst @@ -0,0 +1,2 @@ +Rename :attr:`multiprocessing.synchronize.SemLock.is_fork_ctx` to :attr:`multiprocessing.synchronize.SemLock._is_fork_ctx` to avoid exposing it as public API. +Fix :meth:`multiprocessing.synchronize.SemLock.__setstate__` to properly initialize :attr:`multiprocessing.synchronize.SemLock._is_fork_ctx`. From 61d402db256a90dc11edd120553b29811d44a27a Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 30 Aug 2023 18:18:05 +0200 Subject: [PATCH 6/8] Blurb nits --- .../2023-08-30-15-41-47.gh-issue-108520.u0ZGP_.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-08-30-15-41-47.gh-issue-108520.u0ZGP_.rst b/Misc/NEWS.d/next/Core and Builtins/2023-08-30-15-41-47.gh-issue-108520.u0ZGP_.rst index 5a59e9fa1feaf3..44131fb11f068c 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2023-08-30-15-41-47.gh-issue-108520.u0ZGP_.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2023-08-30-15-41-47.gh-issue-108520.u0ZGP_.rst @@ -1,2 +1,3 @@ +Fix :meth:`multiprocessing.synchronize.SemLock.__setstate__` to properly initialize :attr:`multiprocessing.synchronize.SemLock._is_fork_ctx`. This fixes a regression when passing a SemLock accross nested processes. + Rename :attr:`multiprocessing.synchronize.SemLock.is_fork_ctx` to :attr:`multiprocessing.synchronize.SemLock._is_fork_ctx` to avoid exposing it as public API. -Fix :meth:`multiprocessing.synchronize.SemLock.__setstate__` to properly initialize :attr:`multiprocessing.synchronize.SemLock._is_fork_ctx`. From 6eae9676d6b32537e69e298b4dc903916d5b5fc9 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 30 Aug 2023 18:19:41 +0200 Subject: [PATCH 7/8] Improve co --- Lib/multiprocessing/synchronize.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/multiprocessing/synchronize.py b/Lib/multiprocessing/synchronize.py index fb46d9bc029a6a..8ddd2459bfd8c4 100644 --- a/Lib/multiprocessing/synchronize.py +++ b/Lib/multiprocessing/synchronize.py @@ -115,7 +115,7 @@ def __setstate__(self, state): self._semlock = _multiprocessing.SemLock._rebuild(*state) util.debug('recreated blocker with handle %r' % state[0]) self._make_methods() - # SemLock created in fork context cannot be serialized + # Ensure that deserialized SemLock can be serialized again (gh-108568). self._is_fork_ctx = False @staticmethod From 051f326ac769e13213bfc05d0fdac1f4e25976b8 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Wed, 30 Aug 2023 18:23:57 +0200 Subject: [PATCH 8/8] Update comment --- Lib/multiprocessing/synchronize.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/multiprocessing/synchronize.py b/Lib/multiprocessing/synchronize.py index 8ddd2459bfd8c4..3ccbfe311c71f3 100644 --- a/Lib/multiprocessing/synchronize.py +++ b/Lib/multiprocessing/synchronize.py @@ -115,7 +115,7 @@ def __setstate__(self, state): self._semlock = _multiprocessing.SemLock._rebuild(*state) util.debug('recreated blocker with handle %r' % state[0]) self._make_methods() - # Ensure that deserialized SemLock can be serialized again (gh-108568). + # Ensure that deserialized SemLock can be serialized again (gh-108520). self._is_fork_ctx = False @staticmethod