Skip to content

Commit

Permalink
process.spawn: Use multiprocessing.Process for returnproc
Browse files Browse the repository at this point in the history
Use multiprocessing.Process for returnproc, so that
fork will stop being used when python makes "spawn"
the default multiprocessing start method.

Continue to use _start_fork when returnproc is not
enabled, for backward compatibility. Ultimately,
it can be removed at the same time as the returnpid
parameter.

Bug: https://bugs.gentoo.org/916566
Signed-off-by: Zac Medico <zmedico@gentoo.org>
  • Loading branch information
zmedico committed Feb 3, 2024
1 parent 055c66e commit 3478c19
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 4 deletions.
56 changes: 53 additions & 3 deletions lib/portage/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@

portage.proxy.lazyimport.lazyimport(
globals(),
"portage.util._async.ForkProcess:ForkProcess",
"portage.util._eventloop.global_event_loop:global_event_loop",
"portage.util.futures:asyncio",
"portage.util:dump_traceback,writemsg,writemsg_level",
Expand Down Expand Up @@ -623,7 +624,12 @@ def spawn(
# fork, so that the result is cached in the main process.
bool(groups)

pid = _start_fork(
if returnproc:
start_func = _start_proc
else:
start_func = _start_fork

pid = start_func(
_exec_wrapper,
args=(
binary,
Expand All @@ -649,6 +655,10 @@ def spawn(
close_fds=close_fds,
)

if returnproc:
# _start_proc returns a MultiprocessingProcess instance.
return pid

if not isinstance(pid, int):
raise AssertionError(f"fork returned non-integer: {repr(pid)}")

Expand All @@ -670,8 +680,6 @@ def spawn(
stacklevel=1,
)
return mypids
if returnproc:
return Process(mypids[0])

# Otherwise we clean them up.
while mypids:
Expand Down Expand Up @@ -1370,6 +1378,48 @@ def _start_fork(
return pid


def _start_proc(
target: Callable[..., None],
args: Optional[tuple[Any, ...]] = (),
kwargs: Optional[dict[str, Any]] = {},
fd_pipes: Optional[dict[int, int]] = None,
close_fds: Optional[bool] = False,
) -> MultiprocessingProcess:
"""
Execute the target function using multiprocess.Process.
If the close_fds parameter is True then NotImplementedError
is raised, since it is risky to forcefully close file
descriptors that have references (bug 374335), and PEP 446
should ensure that any relevant file descriptors are
non-inheritable and therefore automatically closed on exec.
Note that for the "spawn" multiprocessing start method,
fd_pipes is sent to the child process asynchronously in a
background thread. Therefore, the caller cannot safely
close the corresponding file descriptors until after the
background thread has finished sending fd_pipes. The
current simplest practice is to close fd_pipes file
descriptors after waiting for the process to exit.
"""
if close_fds:
raise NotImplementedError(
"close_fds is not supported (since file descriptors are non-inheritable by default for exec)"
)

proc = ForkProcess(
scheduler=global_event_loop(),
target=target,
args=args,
kwargs=kwargs,
fd_pipes=fd_pipes,
)
proc.start()

# ForkProcess conveniently holds a MultiprocessingProcess
# instance that is suitable to return here.
return proc._proc


def find_binary(binary):
"""
Given a binary name, find the binary in PATH
Expand Down
6 changes: 5 additions & 1 deletion lib/portage/tests/ebuild/test_doebuild_fd_pipes.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2013-2023 Gentoo Authors
# Copyright 2013-2024 Gentoo Authors
# Distributed under the terms of the GNU General Public License v2

import multiprocessing
Expand Down Expand Up @@ -167,6 +167,10 @@ def testDoebuild(self):
@staticmethod
def _doebuild(db, pw, *args, **kwargs):
QueryCommand._db = db
# Somehow pw.fileno() is inheritable when doebuild is
# implemented with os.fork(), but not when it is implemented
# with multiprocessing.Process.
os.set_inheritable(pw.fileno(), True)
kwargs["fd_pipes"] = {
DoebuildFdPipesTestCase.output_fd: pw.fileno(),
}
Expand Down

0 comments on commit 3478c19

Please sign in to comment.