New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
subprocess.Popen leaks file descriptors opened for DEVNULL or PIPE stdin/stdout/stderr arguments #87474
Comments
TL;DR: subprocess.Popen's handling of file descriptors opened for DEVNULL or PIPE inputs/outputs has serious problems, and it can be coerced into leaking file descriptors in several ways. This can cause issues related to resource exhaustion. # The basic problem As part of its setup, Popen.__init__() calls Popen._get_handles(), which looks at the given stdin/stdout/stderr arguments and returns a tuple of 6 file descriptors (on Windows, file handles) indicating how stdin/stdout/stderr should be redirected. However, these file descriptors aren't properly closed if exceptions occur in certain cases. # Variant 1: Bad argument errors (introduced in 3.9) The first variant of this bug is shockingly easy to reproduce (note that this only works on platforms with /proc/self/fd, like Linux): import os, subprocess
def show_fds():
for entry in os.scandir("/proc/self/fd"):
print(entry.name, "->", os.readlink(entry.path))
print("Before:")
show_fds()
try:
subprocess.Popen(["ls"], stdin=subprocess.PIPE, user=1.0)
except TypeError as e: # "User must be a string or an integer"
print(e)
print("After:")
show_fds() This produces something like:
The process never got launched (because of the invalid This happens because the code that validates the Variant 2: Error opening file descriptors (seems to have been around in
|
This fixes several ways file descriptors could be leaked from `subprocess.Popen` constructor during error conditions by opening them later and using a context manager "fds to close" registration scheme to ensure they get closed before returning. --------- Co-authored-by: Gregory P. Smith [Google] <greg@krypto.org>
…GH-96351) This fixes several ways file descriptors could be leaked from `subprocess.Popen` constructor during error conditions by opening them later and using a context manager "fds to close" registration scheme to ensure they get closed before returning. --------- (cherry picked from commit 3a4c44b) Co-authored-by: cptpcrd <31829097+cptpcrd@users.noreply.github.com> Co-authored-by: Gregory P. Smith [Google] <greg@krypto.org>
) (#104563) gh-87474: Fix file descriptor leaks in subprocess.Popen (GH-96351) This fixes several ways file descriptors could be leaked from `subprocess.Popen` constructor during error conditions by opening them later and using a context manager "fds to close" registration scheme to ensure they get closed before returning. --------- (cherry picked from commit 3a4c44b) Co-authored-by: cptpcrd <31829097+cptpcrd@users.noreply.github.com> Co-authored-by: Gregory P. Smith [Google] <greg@krypto.org>
Thanks for all your work diagnosing and fixing these. It is a good cleanup! |
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:
bugs.python.org fields:
Linked PRs
The text was updated successfully, but these errors were encountered: