slow close file descriptors in subprocess, popen2, os.popen*

[s7v7nislands]

BPO	11284
Nosy	@loewis, @gpshead, @pitrou, @vstinner

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/gpshead'
closed_at = <Date 2011-03-27.09:11:31.917>
created_at = <Date 2011-02-22.09:38:09.873>
labels = ['library', 'performance']
title = 'slow close file descriptors in subprocess, popen2, os.popen*'
updated_at = <Date 2011-03-31.13:32:28.406>
user = 'https://bugs.python.org/s7v7nislands'

bugs.python.org fields:

activity = <Date 2011-03-31.13:32:28.406>
actor = 'vstinner'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2011-03-27.09:11:31.917>
closer = 'rosslagerwall'
components = ['Library (Lib)']
creation = <Date 2011-02-22.09:38:09.873>
creator = 's7v7nislands'
dependencies = []
files = []
hgrepos = []
issue_num = 11284
keywords = ['patch']
message_count = 14.0
messages = ['129043', '129045', '129118', '129154', '129891', '129892', '129894', '129902', '129904', '129912', '129913', '129916', '132296', '132668']
nosy_count = 7.0
nosy_names = ['loewis', 'gregory.p.smith', 'pitrou', 'vstinner', 's7v7nislands', 'neologix', 'rosslagerwall']
pr_nums = []
priority = 'high'
resolution = 'duplicate'
stage = 'patch review'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue11284'
versions = ['Python 3.1', 'Python 2.7', 'Python 3.2', 'Python 3.3']

[s7v7nislands]

when use popen*() and close_fds is True, python will close unused fds. but the MAXFD is not the real max. especially in freebsd, subprocess.MAXFD=655000. so python will try to close to many fd, it's too slow, in my test on freebsd, using about 3 seconds.

poor english.

patch for 2.7 trunck:
jiangxiaobing@s7v7nislands ~/source/svn/python27 $ svn diff
Index: Lib/subprocess.py
===================================================================

--- Lib/subprocess.py	(revision 88499)
+++ Lib/subprocess.py	(working copy)
@@ -1065,11 +1065,16 @@
 
 
         def _close_fds(self, but):
+            maxfd = MAX_FD
+            try:
+                maxfd = os.dup(0) + 1
+            except:
+                pass
             if hasattr(os, 'closerange'):
                 os.closerange(3, but)
-                os.closerange(but + 1, MAXFD)
+                os.closerange(but + 1, maxfd)
             else:
-                for i in xrange(3, MAXFD):
+                for i in xrange(3, maxfd):
                     if i == but:
                         continue
                     try:
Index: Lib/popen2.py
===================================================================
--- Lib/popen2.py	(revision 88499)
+++ Lib/popen2.py	(working copy)
@@ -82,8 +82,13 @@
     def _run_child(self, cmd):
         if isinstance(cmd, basestring):
             cmd = ['/bin/sh', '-c', cmd]
-        os.closerange(3, MAXFD)
+        maxfd = MAXFD
         try:
+            maxfd = os.dup(0) + 1
+        except:
+            pass
+        os.closerange(3, maxfd)
+        try:
             os.execvp(cmd[0], cmd)
         finally:
             os._exit(1)



patch for 3.3 truck:
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index c02fb52..98a25b3 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -1112,8 +1112,14 @@ class Popen(object):
                 if fd >= start_fd:
                     os.closerange(start_fd, fd)
                     start_fd = fd + 1
-            if start_fd <= MAXFD:
-                os.closerange(start_fd, MAXFD)
+            maxfd = MAXFD
+            try:
+                maxfd = os.dup(0) + 1
+            except:
+                pass
+
+            if start_fd <= maxfd:
+                os.closerange(start_fd, maxfd)
 
 
         def _execute_child(self, args, executable, preexec_fn, close_fds,

[neologix]

dup(2) returns the lowest numbered available file descriptor: if there's a discontinuity in the FDs allocation, this code is going to close only the FDs up to the first available FD.
Imagine for example the following:
open("/tmp/foo") = 3
open("/tmp/bar") = 4
close(3)

Then dup(0) will return 3, and not the max FD.

[neologix]

To elaborate on this, to my knowledge, there's no portable and reliable way to close all open file descriptors.
Even with the current code, it's still possible that some FD aren't properly closed, since getconf(SC_OPEN_MAX) often returns RLIMIT_NOFILE soft limit, which might have been lowered by the application after having opened a whole bunch of files.
Anyway, if the performance hit is too high, I think you only have two options:

• set your FD as CLOEXEC (note that it's already the case for FD used by popen and friends), and don't pass close_fds argument
• if you don't need many FD, you could explicitely set RLIMIT_NOFILE to a low value so that close_fds doesn't need to try too many FD, e.g.:
  import resource
  resource.setrlimit(resource.RLIMIT_NOFILE, (1024, 1024))

[s7v7nislands]

thanks, neologix.

I think should put this hint in python doc.

[vstinner]

As written by Charles-Francois Natali (msg129890), we can use closefrom(). Pseudo-code: find the biggest fd than that be kept open, call closefrom(highest+1), and then use close() (os.closerange) for fd in 0..highest that have to be closed.

closefrom() is available on OpenBSD, Solaris and NetBSD. I don't know for FreeBSD:
http://unix.derkeiler.com/Mailing-Lists/FreeBSD/hackers/2007-07/msg00035.html

On Linux, we can use os.listdir("/proc/self/fd") to get the list of open files.

[pitrou]

The patch is wrong, as explained by Charles-François.
If someone wants subprocess to use closefrom(), a patch is required :)

But I think at least a mention in the documentation is warranted.

[vstinner]

Benchmark extracted from bpo-11314, Python 3.2 on Linux: subprocess("/bin/false", close_fds=True) is 22% slower than subprocess("/bin/false", close_fds=False).

[neologix]

Attached is a patch adding os.closefrom.
If closefrom(2) is available, it's used.
Otherwise, two options:

• if sysconf and _SC_OPEN_MAX are defined, we close each file descriptor up to _SC_OPEN_MAX
• if not, we choose a default value (256), and close every FD up to this value
  subprocess has been converted to use it, and a test has been added in test_os
  Unfortunately, I only have Linux boxes, so I can't really test it.

Remarks:

• is it OK to provide posix_closefrom even though the underlying platform doesn't support it ?
• no error code is returned (since when closing every FD manually this wouldn't make much sense), even though closefrom(2) does return one
• for the test, I only close FDs > 7 to avoid closing stdin/stdout/stder, but you might have a better idea
• this won't fix the problem for Linux, which doesn't have closefrom(2). Is it worth using /proc/self/fd interface ?

[neologix]

Attached is a new version falling back to /proc/self/fd when closefrom(2) is not available (on Unix), working on Linux.
It's indeed much faster than the current approach.
Note that it's only used if _posixsubprocess is not available, because in that case the FD are closed from _posixsubprocess.c:child_exec.
To make it available to _posixsubprocess, I was thinking of putting the closefrom code in a helper function, which would then be called from posix_closefrom and _posixsubprocess.
Is this the proper way to do ?
If yes, what file would be a good recipient for that ?

[gpshead]

There is no OS API call to provide a *safe* way to get a list of all open file descriptors as part of POSIX in general that can be called after the fork() and before the exec(). It must be async signal safe. The closefrom() call available in Solaris and recent BSDs looks useful, is it async signal safe? I still want to find a way to do this nicely on Linux (even if it means me going and implementing a closefrom syscall to be added to 2.6.39).

Your posix_closefrom() implementation as written today is not safe to call between fork() and exec() due to the opendir/readdir implementation. It can and will hang processes at unexpected times.

Another thought that I've seen done in other subprocess implementations as a compromise:

Offer a way to use the hacky *not guaranteed to close everything if the process is multithreaded* version that has the parent get the list of fds before the fork() and the child closing those before exec(). That leaves the race condition of new fds being opened between the creation of that list and the fork() but would be fast. If we can detect if any other threads exist in the program (they may have been created by C extension modules or by a C program that is embedding Python within it) we could conditionally use that approach vs the close-everything-possible approach so that people using this with generic programs that don't involve threads are not so heavily impacted.

[gpshead]

According to http://www.unix.com/man-page/All/3c/closefrom/ closefrom() is not async-signal-safe. :(