Close fewer file descriptors for fork / no exec.

This will fix bug #374335.

diff --git a/pym/_emerge/EbuildFetcher.py b/pym/_emerge/EbuildFetcher.py
index f6dab5471c9230c7edb03519834502f17617afc8..c0a7fddaa635dd842067d7289509c3f200bd5612 100644 (file)
--- a/pym/_emerge/EbuildFetcher.py
+++ b/pym/_emerge/EbuildFetcher.py
@@ -1,4 +1,4 @@
-# Copyright 1999-2011 Gentoo Foundation
+# Copyright 1999-2012 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
 import traceback
@@ -6,7 +6,6 @@ import traceback
 from _emerge.SpawnProcess import SpawnProcess
 import copy
 import io
-import platform
 import signal
 import sys
 import portage
@@ -170,14 +169,9 @@ class EbuildFetcher(SpawnProcess):
                        portage.process.spawned_pids.append(pid)
                        return [pid]
 
-               # TODO: Find out why PyPy 1.8 with close_fds=True triggers
-               # "[Errno 9] Bad file descriptor" in subprocesses. It could
-               # be due to garbage collection of file objects that were not
-               # closed before going out of scope, since PyPy's garbage
-               # collector does not support the refcounting semantics that
-               # CPython does.
-               close_fds = platform.python_implementation() != 'PyPy'
-               portage.process._setup_pipes(fd_pipes, close_fds=close_fds)
+               portage.locks._close_fds()
+               # Disable close_fds since we don't exec (see _setup_pipes docstring).
+               portage.process._setup_pipes(fd_pipes, close_fds=False)
 
                # Use default signal handlers in order to avoid problems
                # killing subprocesses as reported in bug #353239.

diff --git a/pym/portage/dbapi/_MergeProcess.py b/pym/portage/dbapi/_MergeProcess.py
index 301625c8120346ea6bd85977d9043ca479525c35..b5f6a0b0ba7c6c305e911e5fde74bcb551ca4d7b 100644 (file)
--- a/pym/portage/dbapi/_MergeProcess.py
+++ b/pym/portage/dbapi/_MergeProcess.py
@@ -2,7 +2,6 @@
 # Distributed under the terms of the GNU General Public License v2
 
 import io
-import platform
 import signal
 import sys
 import traceback
@@ -155,15 +154,9 @@ class MergeProcess(SpawnProcess):
                        return [pid]
 
                os.close(elog_reader_fd)
-
-               # TODO: Find out why PyPy 1.8 with close_fds=True triggers
-               # "[Errno 9] Bad file descriptor" in subprocesses. It could
-               # be due to garbage collection of file objects that were not
-               # closed before going out of scope, since PyPy's garbage
-               # collector does not support the refcounting semantics that
-               # CPython does.
-               close_fds = platform.python_implementation() != 'PyPy'
-               portage.process._setup_pipes(fd_pipes, close_fds=close_fds)
+               portage.locks._close_fds()
+               # Disable close_fds since we don't exec (see _setup_pipes docstring).
+               portage.process._setup_pipes(fd_pipes, close_fds=False)
 
                # Use default signal handlers since the ones inherited
                # from the parent process are irrelevant here.

diff --git a/pym/portage/locks.py b/pym/portage/locks.py
index 9fee5ae5dc17a978984ad452c77bd379b056eb36..87fbe0726ab4cf4802112ce27842751361938500 100644 (file)
--- a/pym/portage/locks.py
+++ b/pym/portage/locks.py
@@ -1,5 +1,5 @@
 # portage: Lock management code
-# Copyright 2004-2011 Gentoo Foundation
+# Copyright 2004-2012 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
 
 __all__ = ["lockdir", "unlockdir", "lockfile", "unlockfile", \
@@ -36,6 +36,20 @@ if platform.python_implementation() == 'PyPy':
 # so that it doesn't interfere with the status display.
 _quiet = False
 
+
+_open_fds = set()
+
+def _close_fds():
+       """
+       This is intended to be called after a fork, in order to close file
+       descriptors for locks held by the parent process. This can be called
+       safely after a fork without exec, unlike the _setup_pipes close_fds
+       behavior.
+       .
+       """
+       while _open_fds:
+               os.close(_open_fds.pop())
+
 def lockdir(mydir, flags=0):
        return lockfile(mydir, wantnewlockfile=1, flags=flags)
 def unlockdir(mylock):
@@ -193,6 +207,9 @@ def lockfile(mypath, wantnewlockfile=0, unlinkfile=0,
                        mypath, wantnewlockfile=wantnewlockfile, unlinkfile=unlinkfile,
                        waiting_msg=waiting_msg, flags=flags)
 
+       if myfd != HARDLINK_FD:
+               _open_fds.add(myfd)
+
        writemsg(str((lockfilename,myfd,unlinkfile))+"\n",1)
        return (lockfilename,myfd,unlinkfile,locking_method)
 
@@ -233,6 +250,7 @@ def unlockfile(mytuple):
                writemsg(_("lockfile does not exist '%s'\n") % lockfilename,1)
                if myfd is not None:
                        os.close(myfd)
+                       _open_fds.remove(myfd)
                return False
 
        try:
@@ -243,6 +261,7 @@ def unlockfile(mytuple):
        except OSError:
                if isinstance(lockfilename, basestring):
                        os.close(myfd)
+                       _open_fds.remove(myfd)
                raise IOError(_("Failed to unlock file '%s'\n") % lockfilename)
 
        try:
@@ -264,6 +283,7 @@ def unlockfile(mytuple):
                        else:
                                writemsg(_("lockfile does not exist '%s'\n") % lockfilename, 1)
                                os.close(myfd)
+                               _open_fds.remove(myfd)
                                return False
        except SystemExit:
                raise
@@ -276,6 +296,7 @@ def unlockfile(mytuple):
        # open fd closed automatically on them.
        if isinstance(lockfilename, basestring):
                os.close(myfd)
+               _open_fds.remove(myfd)
 
        return True
 

diff --git a/pym/portage/process.py b/pym/portage/process.py
index 94687eaa0cc999aab86cfacf78a3ee3422170934..f3cec88153cbae3d4a82bd88b8b1d8905ecfd14e 100644 (file)
--- a/pym/portage/process.py
+++ b/pym/portage/process.py
@@ -401,7 +401,19 @@ def _exec(binary, mycommand, opt_name, fd_pipes, env, gid, groups, uid, umask,
        os.execve(binary, myargs, env)
 
 def _setup_pipes(fd_pipes, close_fds=True):
-       """Setup pipes for a forked process."""
+       """Setup pipes for a forked process.
+
+       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
+       collector intermittently executes such destructors after their
+       corresponding file descriptors have been re-used, leading
+       to intermittent "[Errno 9] Bad file descriptor" exceptions in
+       forked processes. This problem has been observed with PyPy 1.8,
+       and also with CPython under some circumstances (as triggered
+       by xmpppy in bug #374335). In order to close a safe subset of
+       file descriptors, see portage.locks._close_fds().
+       """
        my_fds = {}
        # To protect from cases where direct assignment could
        # clobber needed fds ({1:2, 2:1}) we first dupe the fds