public inbox for gentoo-commits@lists.gentoo.org
 help / color / mirror / Atom feed
From: "Zac Medico" <zmedico@gentoo.org>
To: gentoo-commits@lists.gentoo.org
Subject: [gentoo-commits] proj/portage:master commit in: pym/portage/dbapi/, pym/portage/, pym/_emerge/
Date: Tue, 14 Feb 2012 12:30:06 +0000 (UTC)	[thread overview]
Message-ID: <1329222542.a75341bf3a66f75edd68d1a8bc5efdb51c0c0740.zmedico@gentoo> (raw)

commit:     a75341bf3a66f75edd68d1a8bc5efdb51c0c0740
Author:     Zac Medico <zmedico <AT> gentoo <DOT> org>
AuthorDate: Tue Feb 14 12:29:02 2012 +0000
Commit:     Zac Medico <zmedico <AT> gentoo <DOT> org>
CommitDate: Tue Feb 14 12:29:02 2012 +0000
URL:        http://git.overlays.gentoo.org/gitweb/?p=proj/portage.git;a=commit;h=a75341bf

After python fork, don't close fds for PyPy 1.8.

If we close all open file descriptors after a fork, with PyPy 1.8 it
triggers "[Errno 9] Bad file descriptor" later in the subprocess.
Apparently it is holding references to file descriptors and closing
them after they've already been closed and re-opened for other
purposes. As a workaround, we don't close the file descriptors, so
that they won't be re-used and therefore we won't be vulnerable to this
kind of interference.

The obvious caveat of not closing the fds is that the subprocess can
hold locks that belonged to the parent process, even after the parent
process has released the locks. Hopefully this won't be a major problem
though, since the subprocess has to exit at release the lock
eventually, when the EbuildFetcher or _MergeProcess task is complete.

---
 pym/_emerge/EbuildFetcher.py       |    6 +++++-
 pym/portage/dbapi/_MergeProcess.py |    7 ++++++-
 pym/portage/process.py             |   20 +++++++++++---------
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/pym/_emerge/EbuildFetcher.py b/pym/_emerge/EbuildFetcher.py
index 61c7848..6ad4341 100644
--- a/pym/_emerge/EbuildFetcher.py
+++ b/pym/_emerge/EbuildFetcher.py
@@ -6,6 +6,7 @@ import traceback
 from _emerge.SpawnProcess import SpawnProcess
 import copy
 import io
+import platform
 import signal
 import sys
 import portage
@@ -166,7 +167,10 @@ class EbuildFetcher(SpawnProcess):
 			portage.process.spawned_pids.append(pid)
 			return [pid]
 
-		portage.process._setup_pipes(fd_pipes)
+		# TODO: Find out why PyPy 1.8 with close_fds=True triggers
+		# "[Errno 9] Bad file descriptor" in subprocesses.
+		close_fds = platform.python_implementation() != 'PyPy'
+		portage.process._setup_pipes(fd_pipes, close_fds=close_fds)
 
 		# 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 9bb67c9..cf59265 100644
--- a/pym/portage/dbapi/_MergeProcess.py
+++ b/pym/portage/dbapi/_MergeProcess.py
@@ -2,6 +2,7 @@
 # Distributed under the terms of the GNU General Public License v2
 
 import io
+import platform
 import signal
 import traceback
 
@@ -143,7 +144,11 @@ class MergeProcess(SpawnProcess):
 			return [pid]
 
 		os.close(elog_reader_fd)
-		portage.process._setup_pipes(fd_pipes)
+
+		# TODO: Find out why PyPy 1.8 with close_fds=True triggers
+		# "[Errno 9] Bad file descriptor" in subprocesses.
+		close_fds = platform.python_implementation() != 'PyPy'
+		portage.process._setup_pipes(fd_pipes, close_fds=close_fds)
 
 		# Use default signal handlers since the ones inherited
 		# from the parent process are irrelevant here.

diff --git a/pym/portage/process.py b/pym/portage/process.py
index 47b0a21..e7313ab 100644
--- a/pym/portage/process.py
+++ b/pym/portage/process.py
@@ -386,7 +386,7 @@ def _exec(binary, mycommand, opt_name, fd_pipes, env, gid, groups, uid, umask,
 	# And switch to the new process.
 	os.execve(binary, myargs, env)
 
-def _setup_pipes(fd_pipes):
+def _setup_pipes(fd_pipes, close_fds=True):
 	"""Setup pipes for a forked process."""
 	my_fds = {}
 	# To protect from cases where direct assignment could
@@ -397,14 +397,16 @@ def _setup_pipes(fd_pipes):
 	# Then assign them to what they should be.
 	for fd in my_fds:
 		os.dup2(my_fds[fd], fd)
-	# 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:
-			try:
-				os.close(fd)
-			except OSError:
-				pass
+
+	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:
+				try:
+					os.close(fd)
+				except OSError:
+					pass
 
 def find_binary(binary):
 	"""



             reply	other threads:[~2012-02-14 12:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-02-14 12:30 Zac Medico [this message]
  -- strict thread matches above, loose matches on Subject: below --
2012-09-29  2:36 [gentoo-commits] proj/portage:master commit in: pym/portage/dbapi/, pym/portage/, pym/_emerge/ Zac Medico
2012-03-27 18:28 Zac Medico
2011-10-24 17:55 Zac Medico
2011-08-26 19:55 Zac Medico

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1329222542.a75341bf3a66f75edd68d1a8bc5efdb51c0c0740.zmedico@gentoo \
    --to=zmedico@gentoo.org \
    --cc=gentoo-commits@lists.gentoo.org \
    --cc=gentoo-dev@lists.gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox