[Python-checkins] subprocess: close pipes/fds by using ExitStack (GH-11686)
Gregory P. Smith
webhook-mailer at python.org
Tue Jan 29 16:14:28 EST 2019
https://github.com/python/cpython/commit/bafa8487f77fa076de3a06755399daf81cb75598
commit: bafa8487f77fa076de3a06755399daf81cb75598
branch: master
author: Giampaolo Rodola <g.rodola at gmail.com>
committer: Gregory P. Smith <greg at krypto.org>
date: 2019-01-29T13:14:24-08:00
summary:
subprocess: close pipes/fds by using ExitStack (GH-11686)
Close pipes/fds in subprocess by using ExitStack.
"In case of premature failure on X.Close() or os.close(X) the remaining pipes/fds will remain "open". Perhaps it makes sense to use contextlib.ExitStack."
- Rationale: https://github.com/python/cpython/pull/11575#discussion_r250288394
files:
A Misc/NEWS.d/next/Library/2019-01-29-17-24-52.bpo-35537.Q0ktFC.rst
M Lib/subprocess.py
diff --git a/Lib/subprocess.py b/Lib/subprocess.py
index 1f6eb63b387f..0496b447e8ea 100644
--- a/Lib/subprocess.py
+++ b/Lib/subprocess.py
@@ -50,6 +50,7 @@
import sys
import threading
import warnings
+import contextlib
from time import monotonic as _time
@@ -1072,28 +1073,28 @@ def _close_pipe_fds(self,
# self._devnull is not always defined.
devnull_fd = getattr(self, '_devnull', None)
- if _mswindows:
- if p2cread != -1:
- p2cread.Close()
- if c2pwrite != -1:
- c2pwrite.Close()
- if errwrite != -1:
- errwrite.Close()
- else:
- if p2cread != -1 and p2cwrite != -1 and p2cread != devnull_fd:
- os.close(p2cread)
- if c2pwrite != -1 and c2pread != -1 and c2pwrite != devnull_fd:
- os.close(c2pwrite)
- if errwrite != -1 and errread != -1 and errwrite != devnull_fd:
- os.close(errwrite)
+ with contextlib.ExitStack() as stack:
+ if _mswindows:
+ if p2cread != -1:
+ stack.callback(p2cread.Close)
+ if c2pwrite != -1:
+ stack.callback(c2pwrite.Close)
+ if errwrite != -1:
+ stack.callback(errwrite.Close)
+ else:
+ if p2cread != -1 and p2cwrite != -1 and p2cread != devnull_fd:
+ stack.callback(os.close, p2cread)
+ if c2pwrite != -1 and c2pread != -1 and c2pwrite != devnull_fd:
+ stack.callback(os.close, c2pwrite)
+ if errwrite != -1 and errread != -1 and errwrite != devnull_fd:
+ stack.callback(os.close, errwrite)
- if devnull_fd is not None:
- os.close(devnull_fd)
+ if devnull_fd is not None:
+ stack.callback(os.close, devnull_fd)
# Prevent a double close of these handles/fds from __init__ on error.
self._closed_child_pipe_fds = True
-
if _mswindows:
#
# Windows methods
diff --git a/Misc/NEWS.d/next/Library/2019-01-29-17-24-52.bpo-35537.Q0ktFC.rst b/Misc/NEWS.d/next/Library/2019-01-29-17-24-52.bpo-35537.Q0ktFC.rst
new file mode 100644
index 000000000000..2a9588e745f4
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2019-01-29-17-24-52.bpo-35537.Q0ktFC.rst
@@ -0,0 +1,4 @@
+An ExitStack is now used internally within subprocess.POpen to clean up pipe
+file handles. No behavior change in normal operation. But if closing one
+handle were ever to cause an exception, the others will now be closed
+instead of leaked. (patch by Giampaolo Rodola)
More information about the Python-checkins
mailing list