commit:     e8b31c86eaed645a8740fb2844e2935aee161e43
Author:     Zac Medico <zmedico <AT> gentoo <DOT> org>
AuthorDate: Mon Feb  5 05:55:11 2024 +0000
Commit:     Zac Medico <zmedico <AT> gentoo <DOT> org>
CommitDate: Tue Feb  6 01:30:21 2024 +0000
URL:        https://gitweb.gentoo.org/proj/portage.git/commit/?id=e8b31c86

ForkProcess: Prevent redundant pipe and set_term_size recursion

When process.spawn is updated to call ForkProcess
for bug 916566, it needs to avoid recursion via
set_term_size.

Bug: https://bugs.gentoo.org/916566
Bug: https://bugs.gentoo.org/923750
Signed-off-by: Zac Medico <zmedico <AT> gentoo.org>

 lib/_emerge/SpawnProcess.py            | 31 ++++++++++++++++++++++++-------
 lib/portage/util/_async/ForkProcess.py | 28 +++++++++++++++++++++++-----
 2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/lib/_emerge/SpawnProcess.py b/lib/_emerge/SpawnProcess.py
index 7f4a23892b..716e94d665 100644
--- a/lib/_emerge/SpawnProcess.py
+++ b/lib/_emerge/SpawnProcess.py
@@ -41,7 +41,7 @@ class SpawnProcess(SubProcess):
     )
 
     __slots__ = (
-        ("args", "log_filter_file")
+        ("args", "create_pipe", "log_filter_file")
         + _spawn_kwarg_names
         + (
             "_main_task",
@@ -60,15 +60,30 @@ class SpawnProcess(SubProcess):
         else:
             self.fd_pipes = self.fd_pipes.copy()
         fd_pipes = self.fd_pipes
+        log_file_path = None
 
         if fd_pipes or self.logfile or not self.background:
-            master_fd, slave_fd = self._pipe(fd_pipes)
+            if self.create_pipe is not False:
+                master_fd, slave_fd = self._pipe(fd_pipes)
 
-            can_log = self._can_log(slave_fd)
-            if can_log:
-                log_file_path = self.logfile
+                can_log = self._can_log(slave_fd)
+                if can_log:
+                    log_file_path = self.logfile
             else:
-                log_file_path = None
+                if self.logfile:
+                    raise NotImplementedError(
+                        "logfile conflicts with create_pipe=False"
+                    )
+                # When called via process.spawn and ForkProcess._start,
+                # SpawnProcess will have created a pipe earlier, so it
+                # would be redundant to do it here (it could also trigger
+                # spawn recursion via set_term_size as in bug 923750).
+                # Use /dev/null for master_fd, triggering early return
+                # of _main, followed by _async_waitpid.
+                # TODO: Optimize away the need for master_fd here.
+                master_fd = os.open(os.devnull, os.O_RDONLY)
+                slave_fd = None
+                can_log = False
 
             null_input = None
             if not self.background or 0 in fd_pipes:
@@ -97,7 +112,9 @@ class SpawnProcess(SubProcess):
 
             fd_pipes_orig = fd_pipes.copy()
 
-            if log_file_path is not None or self.background:
+            if slave_fd is None:
+                pass
+            elif log_file_path is not None or self.background:
                 fd_pipes[1] = slave_fd
                 fd_pipes[2] = slave_fd
 

diff --git a/lib/portage/util/_async/ForkProcess.py 
b/lib/portage/util/_async/ForkProcess.py
index 3acbe34fc6..cb240d0712 100644
--- a/lib/portage/util/_async/ForkProcess.py
+++ b/lib/portage/util/_async/ForkProcess.py
@@ -75,12 +75,29 @@ class ForkProcess(SpawnProcess):
                 self.fd_pipes.setdefault(0, portage._get_stdin().fileno())
                 self.fd_pipes.setdefault(1, sys.__stdout__.fileno())
                 self.fd_pipes.setdefault(2, sys.__stderr__.fileno())
-                stdout_fd = os.dup(self.fd_pipes[1])
+                if self.create_pipe is not False:
+                    stdout_fd = os.dup(self.fd_pipes[1])
 
             if self._HAVE_SEND_HANDLE:
-                master_fd, slave_fd = self._pipe(self.fd_pipes)
-                self.fd_pipes[1] = slave_fd
-                self.fd_pipes[2] = slave_fd
+                if self.create_pipe is not False:
+                    master_fd, slave_fd = self._pipe(self.fd_pipes)
+                    self.fd_pipes[1] = slave_fd
+                    self.fd_pipes[2] = slave_fd
+                else:
+                    if self.logfile:
+                        raise NotImplementedError(
+                            "logfile conflicts with create_pipe=False"
+                        )
+                    # When called via process.spawn, SpawnProcess
+                    # will have created a pipe earlier, so it would be
+                    # redundant to do it here (it could also trigger spawn
+                    # recursion via set_term_size as in bug 923750). Use
+                    # /dev/null for master_fd, triggering early return
+                    # of _main, followed by _async_waitpid.
+                    # TODO: Optimize away the need for master_fd here.
+                    master_fd = os.open(os.devnull, os.O_RDONLY)
+                    slave_fd = None
+
                 self._files = self._files_dict(connection=connection, 
slave_fd=slave_fd)
 
                 # Create duplicate file descriptors in self._fd_pipes
@@ -167,7 +184,8 @@ class ForkProcess(SpawnProcess):
                     self._files.connection.close()
                     del self._files.connection
                 if hasattr(self._files, "slave_fd"):
-                    os.close(self._files.slave_fd)
+                    if self._files.slave_fd is not None:
+                        os.close(self._files.slave_fd)
                     del self._files.slave_fd
 
         await super()._main(build_logger, pipe_logger, loop=loop)

Reply via email to