Skip to content

Commit

Permalink
[3.12] gh-108520: Fix bad fork detection in nested multiprocessing us…
Browse files Browse the repository at this point in the history
…e case (GH-108568) (#108691)

gh-108520: Fix bad fork detection in nested multiprocessing use case (GH-108568)

gh-107275 introduced a regression where a SemLock would fail being passed along nested child processes, as the `is_fork_ctx` attribute would be left missing after the first deserialization.

---------

(cherry picked from commit add8d45)

Co-authored-by: albanD <desmaison.alban@gmail.com>
Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com>
Co-authored-by: Antoine Pitrou <pitrou@free.fr>
  • Loading branch information
4 people committed Aug 30, 2023
1 parent 420db10 commit 320d398
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 3 deletions.
8 changes: 5 additions & 3 deletions Lib/multiprocessing/synchronize.py
Expand Up @@ -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(
Expand Down Expand Up @@ -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 '
Expand All @@ -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()
# Ensure that deserialized SemLock can be serialized again (gh-108520).
self._is_fork_ctx = False

@staticmethod
def _make_name():
Expand Down
26 changes: 26 additions & 0 deletions Lib/test/_test_multiprocessing.py
Expand Up @@ -5405,6 +5405,32 @@ 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=self._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")
Expand Down
@@ -0,0 +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.

0 comments on commit 320d398

Please sign in to comment.