_setup_pipes: close unnecessary duplicate fds

diff --git a/pym/portage/process.py b/pym/portage/process.py
index 23a9094669b76b7742a83c704dfeac4462c166c2..d677b9fa8c2fee8ff576dd15dc4f381a9394a138 100644 (file)
--- a/pym/portage/process.py
+++ b/pym/portage/process.py
@@ -419,6 +419,13 @@ def _exec(binary, mycommand, opt_name, fd_pipes, env, gid, groups, uid, umask,
 def _setup_pipes(fd_pipes, close_fds=True):
        """Setup pipes for a forked process.
 
+       Even when close_fds is False, file descriptors referenced as
+       values in fd_pipes are automatically closed if they do not also
+       occur as keys in fd_pipes. It is assumed that the caller will
+       explicitely add them to the fd_pipes keys if they are intended
+       to remain open. This allows for convenient elimination of
+       unnecessary duplicate file descriptors.
+
        WARNING: When not followed by exec, the close_fds behavior
        can trigger interference from destructors that close file
        descriptors. This interference happens when the garbage
@@ -445,21 +452,51 @@ def _setup_pipes(fd_pipes, close_fds=True):
        actually does nothing in this case), which avoids possible
        interference.
        """
-       my_fds = {}
+       reverse_map = {}
        # To protect from cases where direct assignment could
-       # clobber needed fds ({1:2, 2:1}) we first dupe the fds
-       # into unused fds.
-       for fd in fd_pipes:
-               my_fds[fd] = os.dup(fd_pipes[fd])
-       # Then assign them to what they should be.
-       for fd in my_fds:
-               os.dup2(my_fds[fd], fd)
+       # clobber needed fds ({1:2, 2:1}) we create a reverse map
+       # in order to know when it's necessary to create temporary
+       # backup copies with os.dup().
+       for newfd, oldfd in fd_pipes.items():
+               newfds = reverse_map.get(oldfd)
+               if newfds is None:
+                       newfds = []
+                       reverse_map[oldfd] = newfds
+               newfds.append(newfd)
+
+       # Assign newfds via dup2(), making temporary backups when
+       # necessary, and closing oldfd if the caller has not
+       # explicitly requested for it to remain open by adding
+       # it to the keys of fd_pipes.
+       while reverse_map:
+
+               oldfd, newfds = reverse_map.popitem()
+
+               for newfd in newfds:
+                       if newfd in reverse_map:
+                               # Make a temporary backup before re-assignment, assuming
+                               # that backup_fd won't collide with a key in reverse_map
+                               # (since all of the keys correspond to open file
+                               # descriptors, and os.dup() only allocates a previously
+                               # unused file discriptors).
+                               backup_fd = os.dup(newfd)
+                               reverse_map[backup_fd] = reverse_map.pop(newfd)
+                       if oldfd != newfd:
+                               os.dup2(oldfd, newfd)
+
+               if oldfd not in fd_pipes:
+                       # If oldfd is not a key in fd_pipes, then it's safe
+                       # to close now, since we've already made all of the
+                       # requested duplicates. This also closes every
+                       # backup_fd that may have been created on previous
+                       # iterations of this loop.
+                       os.close(oldfd)
 
        if close_fds:
                # Then close _all_ fds that haven't been explicitly
                # requested to be kept open.
                for fd in get_open_fds():
-                       if fd not in my_fds:
+                       if fd not in fd_pipes:
                                try:
                                        os.close(fd)
                                except OSError: