From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from pigeon.gentoo.org ([208.92.234.80] helo=lists.gentoo.org) by finch.gentoo.org with esmtp (Exim 4.60) (envelope-from ) id 1SCb8K-0005Kg-7u for garchives@archives.gentoo.org; Tue, 27 Mar 2012 18:28:43 +0000 Received: from pigeon.gentoo.org (localhost [127.0.0.1]) by pigeon.gentoo.org (Postfix) with SMTP id E71C4E064A; Tue, 27 Mar 2012 18:28:32 +0000 (UTC) Received: from smtp.gentoo.org (smtp.gentoo.org [140.211.166.183]) by pigeon.gentoo.org (Postfix) with ESMTP id A81A1E064A for ; Tue, 27 Mar 2012 18:28:32 +0000 (UTC) Received: from hornbill.gentoo.org (hornbill.gentoo.org [94.100.119.163]) (using TLSv1 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp.gentoo.org (Postfix) with ESMTPS id E47A01B4008 for ; Tue, 27 Mar 2012 18:28:31 +0000 (UTC) Received: from localhost.localdomain (localhost [127.0.0.1]) by hornbill.gentoo.org (Postfix) with ESMTP id AE793E5402 for ; Tue, 27 Mar 2012 18:28:30 +0000 (UTC) From: "Zac Medico" To: gentoo-commits@lists.gentoo.org Content-type: text/plain; charset=UTF-8 Reply-To: gentoo-dev@lists.gentoo.org, "Zac Medico" Message-ID: <1332872892.144c23efbb4e9565debad03c13c5bcab833a8336.zmedico@gentoo> Subject: [gentoo-commits] proj/portage:master commit in: pym/portage/dbapi/, pym/portage/, pym/_emerge/ X-VCS-Repository: proj/portage X-VCS-Files: pym/_emerge/EbuildFetcher.py pym/portage/dbapi/_MergeProcess.py pym/portage/locks.py pym/portage/process.py X-VCS-Directories: pym/portage/dbapi/ pym/portage/ pym/_emerge/ X-VCS-Committer: zmedico X-VCS-Committer-Name: Zac Medico X-VCS-Revision: 144c23efbb4e9565debad03c13c5bcab833a8336 X-VCS-Branch: master Date: Tue, 27 Mar 2012 18:28:30 +0000 (UTC) Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-Id: Gentoo Linux mail X-BeenThere: gentoo-commits@lists.gentoo.org Content-Transfer-Encoding: quoted-printable X-Archives-Salt: 1bdc0082-cf08-4e7f-931d-073bd01c9396 X-Archives-Hash: 0dad679d7b95bec4533b978f68e9ecbf commit: 144c23efbb4e9565debad03c13c5bcab833a8336 Author: Zac Medico gentoo org> AuthorDate: Tue Mar 27 18:28:12 2012 +0000 Commit: Zac Medico gentoo org> CommitDate: Tue Mar 27 18:28:12 2012 +0000 URL: http://git.overlays.gentoo.org/gitweb/?p=3Dproj/portage.git;a= =3Dcommit;h=3D144c23ef Close fewer file descriptors for fork / no exec. This will fix bug #374335. --- pym/_emerge/EbuildFetcher.py | 14 ++++---------- pym/portage/dbapi/_MergeProcess.py | 13 +++---------- pym/portage/locks.py | 23 ++++++++++++++++++++++- pym/portage/process.py | 14 +++++++++++++- 4 files changed, 42 insertions(+), 22 deletions(-) diff --git a/pym/_emerge/EbuildFetcher.py b/pym/_emerge/EbuildFetcher.py index f6dab54..c0a7fdd 100644 --- 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 =20 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] =20 - # TODO: Find out why PyPy 1.8 with close_fds=3DTrue 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 =3D platform.python_implementation() !=3D 'PyPy' - portage.process._setup_pipes(fd_pipes, close_fds=3Dclose_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=3DFalse) =20 # 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/_Merg= eProcess.py index 301625c..b5f6a0b 100644 --- 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 =20 import io -import platform import signal import sys import traceback @@ -155,15 +154,9 @@ class MergeProcess(SpawnProcess): return [pid] =20 os.close(elog_reader_fd) - - # TODO: Find out why PyPy 1.8 with close_fds=3DTrue 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 =3D platform.python_implementation() !=3D 'PyPy' - portage.process._setup_pipes(fd_pipes, close_fds=3Dclose_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=3DFalse) =20 # 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 9fee5ae..87fbe07 100644 --- 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 =20 __all__ =3D ["lockdir", "unlockdir", "lockfile", "unlockfile", \ @@ -36,6 +36,20 @@ if platform.python_implementation() =3D=3D 'PyPy': # so that it doesn't interfere with the status display. _quiet =3D False =20 + +_open_fds =3D 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=3D0): return lockfile(mydir, wantnewlockfile=3D1, flags=3Dflags) def unlockdir(mylock): @@ -193,6 +207,9 @@ def lockfile(mypath, wantnewlockfile=3D0, unlinkfile=3D= 0, mypath, wantnewlockfile=3Dwantnewlockfile, unlinkfile=3Dunlinkfile, waiting_msg=3Dwaiting_msg, flags=3Dflags) =20 + if myfd !=3D HARDLINK_FD: + _open_fds.add(myfd) + writemsg(str((lockfilename,myfd,unlinkfile))+"\n",1) return (lockfilename,myfd,unlinkfile,locking_method) =20 @@ -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 =20 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) =20 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) =20 return True =20 diff --git a/pym/portage/process.py b/pym/portage/process.py index 94687ea..f3cec88 100644 --- 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) =20 def _setup_pipes(fd_pipes, close_fds=3DTrue): - """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 =3D {} # To protect from cases where direct assignment could # clobber needed fds ({1:2, 2:1}) we first dupe the fds