New submission from STINNER Victor <vstin...@python.org>:

To make subprocess faster, I propose to no longer close all file descriptors by 
default in subprocess: change Popen close_fds parameter default to False 
(close_fds=False).

Using close_fds=False, subprocess can use posix_spawn() which is safer and 
faster than fork+exec. For example, on Linux, the glibc implements it as a 
function using vfork which is faster than fork if the parent allocated a lot of 
memory. On macOS, posix_spawn() is even a syscall.

The main drawback is that close_fds=False reopens a security vulnerability if a 
file descriptor is not marked as non-inheritable. The PEP 446 "Make newly 
created file descriptors non-inheritable" was implemented in Python 3.4: Python 
should only create non-inheritable FDs, if it's not the case, Python should be 
fixed. Sadly, 3rd party Python modules may not implement the PEP 446. In this 
case, close_fds=True should be used explicitly, or these modules should be 
fixed. os.set_inheritable() can be used to make FDs as non-inheritable.

close_fds=True has a cost on subprocess performance. When the maximum number of 
file descriptors is larger than 10,000 and Python has no way to list open file 
descriptors, calling close(fd) once per file descriptor can take several 
milliseconds. When I wrote the PEP 446 (in 2013), on a FreeBSD buildbot with 
MAXFD=655,000, closing all FDs took 300 ms (see bpo-11284 "slow close file 
descriptors").

FreeBSD has been fixed recently by using closefrom() function which makes 
_posixsubprocess and os.closerange() faster.

In 2020, my Red Hat colleagues still report the issue in Linux containers 
using... Python 2.7, since Python 2.7 subprocess also close all file 
descriptors in a loop (there was no code to list open file descriptors). The 
problem still exists in Python 3 if subprocess cannot open /proc/self/fd/ 
directory, when /proc pseudo filesystem is not mounted (or if the access is 
blocked, ex: by a sandbox). The problem is that some containers are created a 
very high limit for the maximum number of FDs: os.sysconf("SC_OPEN_MAX") 
returns 1,048,576. Calling close() more than 1 million of times is slow...

See also related issue bpo-38435 "Start the deprecation cycle for subprocess 
preexec_fn".

--

Notes about close_fds=True.

Python 3.9 can now use closefrom() function on FreeBSD: bpo-38061.

Linux 5.10 has a new closerange() syscall: https://lwn.net/Articles/789023/

Linux 5.11 (not released yet) will add a new CLOSE_RANGE_CLOEXEC flag to 
close_range(): https://lwn.net/Articles/837816/

--

History of the close_fds parameter default value.

In Python 2.7, subprocess didn't close all file descriptors by default: 
close_fds=False by default.

Dec 4, 2010: In Python 3.2 (bpo-7213, bpo-2320), subprocess.Popen started to 
emit a deprecating warning when close_fds was not specified explicitly (commit 
d23047b62c6f885def9020bd9b304110f9b9c52d):

+        if close_fds is None:
+            # Notification for http://bugs.python.org/issue7213 & issue2320
+            warnings.warn(
+                    'The close_fds parameter was not specified.  Its default'
+                    ' will change from False to True in a future Python'
+                    ' version.  Most users should set it to True.  Please'
+                    ' update your code explicitly set close_fds.',
+                    DeprecationWarning)

Dec 13 2010, bpo-7213: close_fds default value was changed to True on 
non-Windows platforms, and False on Windows (commit 
f5604853889bfbbf84b48311c63c0e775dff38cc). The implementation was adjusted in 
bpo-6559 (commit 8edd99d0852c45f70b6abc851e6b326d4250cd33) to use a new 
_PLATFORM_DEFAULT_CLOSE_FDS singleton object.

See issues:

* bpo-2320: Race condition in subprocess using stdin
* bpo-6559: add pass_fds paramter to subprocess.Popen()
* bpo-7213: subprocess leaks open file descriptors between Popen instances 
causing hangs

--

On Windows, there is also the question of handles (HANDLE type). Python 3.7 
added the support for the PROC_THREAD_ATTRIBUTE_HANDLE_LIST in 
subprocess.STARTUPINFO: lpAttributeList['handle_list'] parameter. Hopefully, 
Windows has a way better default than Unix: all handles are created as 
non-inheritable by default, so these is no need to explicitly close them.

----------
components: Library (Lib)
messages: 383739
nosy: gregory.p.smith, vstinner
priority: normal
severity: normal
status: open
title: subprocess: don't close all file descriptors by default (close_fds=False)
versions: Python 3.10

_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue42738>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to