8000 Refactor to use context manager · python/cpython@783e6f8 · GitHub
[go: up one dir, main page]
Skip to content

Commit 783e6f8

Browse files
cptpcrdgpshead
cptpcrd
and
gpshead
committed
Refactor to use context manager
Simplifies the logic and should help avoid mistakes in the future. Co-authored-by: Gregory P. Smith <greg@krypto.org>
1 parent 6270ccd commit 783e6f8

File tree

1 file changed

+31
-49
lines changed

1 file changed

+31
-49
lines changed

‎Lib/subprocess.py

Lines changed: 31 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1308,6 +1308,26 @@ def _close_pipe_fds(self,
13081308
# Prevent a double close of these handles/fds from __init__ on error.
13091309
self._closed_child_pipe_fds = True
13101310

1311+
@contextlib.contextmanager
1312+
def _on_error_fd_closer(self):
1313+
"""Helper to ensure file descriptors opened in _get_handles are closed"""
1314+
to_close = []
1315+
try:
1316+
yield to_close
1317+
except:
1318+
if hasattr(self, '_devnull'):
1319+
to_close.append(self._devnull)
1320+
del self._devnull
1321+
for fd in to_close:
1322+
try:
1323+
if _mswindows and isinstance(fd, Handle):
1324+
fd.Close()
1325+
else:
1326+
os.close(fd)
1327+
except OSError:
1328+
pass
1329+
raise
1330+
13111331
if _mswindows:
13121332
#
13131333
# Windows methods
@@ -1323,22 +1343,18 @@ def _get_handles(self, stdin, stdout, stderr):
13231343
c2pread, c2pwrite = -1, -1
13241344
errread, errwrite = -1, -1
13251345

1326-
stdin_needsclose = False
1327-
stdout_needsclose = False
1328-
stderr_needsclose = False
1329-
1330-
try:
1346+
with self._on_error_fd_closer() as err_close_fds:
13311347
if stdin is None:
13321348
p2cread = _winapi.GetStdHandle(_winapi.STD_INPUT_HANDLE)
13331349
if p2cread is None:
1334-
stdin_needsclose = True
13351350
p2cread, _ = _winapi.CreatePipe(None, 0)
13361351
p2cread = Handle(p2cread)
13371352
_winapi.CloseHandle(_)
1353+
err_close_fds.append(p2cread)
13381354
elif stdin == PIPE:
1339-
stdin_needsclose = True
13401355
p2cread, p2cwrite = _winapi.CreatePipe(None, 0)
13411356
p2cread, p2cwrite = Handle(p2cread), Handle(p2cwrite)
1357+
err_close_fds.extend((p2cread, p2cwrite))
13421358
elif stdin == DEVNULL:
13431359
p2cread = msvcrt.get_osfhandle(self._get_devnull())
13441360
elif isinstance(stdin, int):
@@ -1351,14 +1367,14 @@ def _get_handles(self, stdin, stdout, stderr):
13511367
if stdout is None:
13521368
c2pwrite = _winapi.GetStdHandle(_winapi.STD_OUTPUT_HANDLE)
13531369
if c2pwrite is None:
1354-
stdout_needsclose = True
13551370
_, c2pwrite = _winapi.CreatePipe(None, 0)
13561371
c2pwrite = Handle(c2pwrite)
13571372
_winapi.CloseHandle(_)
1373+
err_close_fds.append(c2pwrite)
13581374
elif stdout == PIPE:
1359-
stdout_needsclose = True
13601375
c2pread, c2pwrite = _winapi.CreatePipe(None, 0)
13611376
c2pread, c2pwrite = Handle(c2pread), Handle(c2pwrite)
1377+
err_close_fds.extend((c2pread, c2pwrite))
13621378
elif stdout == DEVNULL:
13631379
c2pwrite = msvcrt.get_osfhandle(self._get_devnull())
13641380
elif isinstance(stdout, int):
@@ -1371,14 +1387,14 @@ def _get_handles(self, stdin, stdout, stderr):
13711387
if stderr is None:
13721388
errwrite = _winapi.GetStdHandle(_winapi.STD_ERROR_HANDLE)
13731389
if errwrite is None:
1374-
stderr_needsclose = True
13751390
_, errwrite = _winapi.CreatePipe(None, 0)
13761391
errwrite = Handle(errwrite)
13771392
_winapi.CloseHandle(_)
1393+
err_close_fds.append(errwrite)
13781394
elif stderr == PIPE:
1379-
stderr_needsclose = True
13801395
errread, errwrite = _winapi.CreatePipe(None, 0)
13811396
errread, errwrite = Handle(errread), Handle(errwrite)
1397+
err_close_fds.extend((errread, errwrite))
13821398
elif stderr == STDOUT:
13831399
errwrite = c2pwrite
13841400
elif stderr == DEVNULL:
@@ -1390,27 +1406,6 @@ def _get_handles(self, stdin, stdout, stderr):
13901406
errwrite = msvcrt.get_osfhandle(stderr.fileno())
13911407
errwrite = self._make_inheritable(errwrite)
13921408

1393-
except BaseException:
1394-
to_close = []
1395-
if stdin_needsclose and p2cwrite != -1:
1396-
to_close.append(p2cread)
1397-
to_close.append(p2cwrite)
1398-
if stdout_needsclose and p2cwrite != -1:
1399-
to_close.append(c2pread)
1400-
to_close.append(c2pwrite)
1401-
if stderr_needsclose and errwrite != -1:
1402-
to_close.append(errread)
1403-
to_close.append(errwrite)
1404-
for file in to_close:
1405-
if isinstance(file, Handle):
1406-
file.Close()
1407-
else:
1408-
os.close(file)
1409-
if hasattr(self, "_devnull"):
1410-
os.close(self._devnull)
1411-
del self._devnull
1412-
raise
1413-
14141409
return (p2cread, p2cwrite,
14151410
c2pread, c2pwrite,
14161411
errread, errwrite)
@@ -1696,13 +1691,14 @@ def _get_handles(self, stdin, stdout, stderr):
16961691
c2pread, c2pwrite = -1, -1
16971692
errread, errwrite = -1, -1
16981693

1699-
try:
1694+
with self._on_error_fd_closer() as err_close_fds:
17001695
if stdin is None:
17011696
pass
17021697
elif stdin == PIPE:
17031698
p2cread, p2cwrite = os.pipe()
17041699
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
17051700
fcntl.fcntl(p2cwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1701+
err_close_fds.extend((p2cread, p2cwrite))
17061702
elif stdin == DEVNULL:
17071703
p2cread = self._get_devnull()
17081704
elif isinstance(stdin, int):
@@ -1717,6 +1713,7 @@ def _get_handles(self, stdin, stdout, stderr):
17171713
c2pread, c2pwrite = os.pipe()
17181714
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
17191715
fcntl.fcntl(c2pwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1716+
err_close_fds.extend((c2pread, c2pwrite))
17201717
elif stdout == DEVNULL:
17211718
c2pwrite = self._get_devnull()
17221719
elif isinstance(stdout, int):
@@ -1731,6 +1728,7 @@ def _get_handles(self, stdin, stdout, stderr):
17311728
errread, errwrite = os.pipe()
17321729
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
17331730
fcntl.fcntl(errwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1731+
err_close_fds.extend((errread, errwrite))
17341732
elif stderr == STDOUT:
17351733
if c2pwrite != -1:
17361734
errwrite = c2pwrite
@@ -1744,22 +1742,6 @@ def _get_handles(self, stdin, stdout, stderr):
17441742
# Assuming file-like object
17451743
errwrite = stderr.fileno()
17461744

1747-
except BaseException:
1748-
# Close the file descriptors we opened to avoid leakage
1749-
if stdin == PIPE and p2cwrite != -1:
1750-
os.close(p2cread)
1751-
os.close(p2cwrite)
1752-
if stdout == PIPE and c2pwrite != -1:
1753-
os.close(c2pread)
1754-
os.close(c2pwrite)
1755-
if stderr == PIPE and errwrite != -1:
1756-
os.close(errread)
1757-
os.close(errwrite)
1758-
if hasattr(self, "_devnull"):
1759-
os.close(self._devnull)
1760-
del self._devnull
1761-
raise
1762-
17631745
return (p2cread, p2cwrite,
17641746
c2pread, c2pwrite,
17651747
errread, errwrite)

0 commit comments

Comments
 (0)
0