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
This commit is contained in:
Giampaolo Rodola 2019-01-29 22:14:24 +01:00 committed by Gregory P. Smith
parent 742d768656
commit bafa8487f7
2 changed files with 22 additions and 17 deletions

View File

@ -50,6 +50,7 @@ import signal
import sys
import threading
import warnings
import contextlib
from time import monotonic as _time
@ -1072,28 +1073,28 @@ class Popen(object):
# 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

View File

@ -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)