From a07a4457765037bf106efcf7f6b3f711474850d8 Mon Sep 17 00:00:00 2001 From: Zac Medico Date: Thu, 17 Jan 2013 09:22:56 -0800 Subject: _setup_pipes: close unnecessary duplicate fds --- pym/portage/process.py | 55 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/pym/portage/process.py b/pym/portage/process.py index 23a909466..d677b9fa8 100644 --- 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: -- cgit v1.2.3-1-g7c22