8000 Fix file descriptor leaks in subprocess.Popen · python/cpython@6270ccd · GitHub
[go: up one dir, main page]
Skip to content

Commit 6270ccd

Browse files
cptpcrdcptpcrd
committed
Fix file descriptor leaks in subprocess.Popen
1 parent f6ca71a commit 6270ccd

File tree

2 files changed

+182
-130
lines changed

2 files changed

+182
-130
lines changed

‎Lib/subprocess.py

Lines changed: 181 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -872,37 +872,6 @@ def __init__(self, args, bufsize=-1, executable=None,
872872
'and universal_newlines are supplied but '
873873
'different. Pass one or the other.')
874874

875-
# Input and output objects. The general principle is like
876-
# this:
877-
#
878-
# Parent Child
879-
# ------ -----
880-
# p2cwrite ---stdin---> p2cread
881-
# c2pread <--stdout--- c2pwrite
882-
# errread <--stderr--- errwrite
883-
#
884-
# On POSIX, the child objects are file descriptors. On
885-
# Windows, these are Windows file handles. The parent objects
886-
# are file descriptors on both platforms. The parent objects
887-
# are -1 when not using PIPEs. The child objects are -1
888-
# when not redirecting.
889-
890-
(p2cread, p2cwrite,
891-
c2pread, c2pwrite,
892-
errread, errwrite) = self._get_handles(stdin, stdout, stderr)
893-
894-
# We wrap OS handles *before* launching the child, otherwise a
895-
# quickly terminating child could make our fds unwrappable
896-
# (see #8458).
897-
898-
if _mswindows:
899-
if p2cwrite != -1:
900-
p2cwrite = msvcrt.open_osfhandle(p2cwrite.Detach(), 0)
901-
if c2pread != -1:
902-
c2pread = msvcrt.open_osfhandle(c2pread.Detach(), 0)
903-
if errread != -1:
904-
errread = msvcrt.open_osfhandle(errread.Detach(), 0)
905-
906875
self.text_mode = encoding or errors or text or universal_newlines
907876
if self.text_mode and encoding is None:
908877
self.encoding = encoding = _text_encoding()
@@ -1003,6 +972,39 @@ def __init__(self, args, bufsize=-1, executable=None,
1003972
if uid < 0:
1004973
raise ValueError(f"User ID cannot be negative, got {uid}")
1005974

975+
# Input and output objects. The general principle is like
976+
# this:
977+
#
978+
# Parent Child
979+
# ------ -----
980+
# p2cwrite ---stdin---> p2cread
981+
# c2pread <--stdout--- c2pwrite
982+
# errread <--stderr--- errwrite
983+
#
984+
# On POSIX, the child objects are file descriptors. On
985+
# Windows, these are Windows file handles. The parent objects
986+
# are file descriptors on both platforms. The parent objects
987+
# are -1 when not using PIPEs. The child objects are -1
988+
# when not redirecting.
989+
990+
(p2cread, p2cwrite,
991+
c2pread, c2pwrite,
992+
errread, errwrite) = self._get_handles(stdin, stdout, stderr)
993+
994+
# From here on, raising exceptions may cause file descriptor leakage
995+
996+
# We wrap OS handles *before* launching the child, otherwise a
997+
# quickly terminating child could make our fds unwrappable
998+
# (see #8458).
999+
1000+
if _mswindows:
1001+
if p2cwrite != -1:
1002+
p2cwrite = msvcrt.open_osfhandle(p2cwrite.Detach(), 0)
1003+
if c2pread != -1:
1004+
c2pread = msvcrt.open_osfhandle(c2pread.Detach(), 0)
1005+
if errread != -1:
1006+
errread = msvcrt.open_osfhandle(errread.Detach(), 0)
1007+
10061008
try:
10071009
if p2cwrite != -1:
10081010
self.stdin = io.open(p2cwrite, 'wb', bufsize)
@@ -1321,61 +1323,93 @@ def _get_handles(self, stdin, stdout, stderr):
13211323
c2pread, c2pwrite = -1, -1
13221324
errread, errwrite = -1, -1
13231325

1324-
if stdin is None:
1325-
p2cread = _winapi.GetStdHandle(_winapi.STD_INPUT_HANDLE)
1326-
if p2cread is None:
1327-
p2cread, _ = _winapi.CreatePipe(None, 0)
1328-
p2cread = Handle(p2cread)
1329-
_winapi.CloseHandle(_)
1330-
elif stdin == PIPE:
1331-
p2cread, p2cwrite = _winapi.CreatePipe(None, 0)
1332-
p2cread, p2cwrite = Handle(p2cread), Handle(p2cwrite)
1333-
elif stdin == DEVNULL:
1334-
p2cread = msvcrt.get_osfhandle(self._get_devnull())
1335-
elif isinstance(stdin, int):
1336-
p2cread = msvcrt.get_osfhandle(stdin)
1337-
else:
1338-
# Assuming file-like object
1339-
p2cread = msvcrt.get_osfhandle(stdin.fileno())
1340-
p2cread = self._make_inheritable(p2cread)
1341-
1342-
if stdout is None:
1343-
c2pwrite = _winapi.GetStdHandle(_winapi.STD_OUTPUT_HANDLE)
1344-
if c2pwrite is None:
1345-
_, c2pwrite = _winapi.CreatePipe(None, 0)
1346-
c2pwrite = Handle(c2pwrite)
1347-
_winapi.CloseHandle(_)
1348-
elif stdout == PIPE:
1349-
c2pread, c2pwrite = _winapi.CreatePipe(None, 0)
1350-
c2pread, c2pwrite = Handle(c2pread), Handle(c2pwrite)
1351-
elif stdout == DEVNULL:
1352-
c2pwrite = msvcrt.get_osfhandle(self._get_devnull())
1353-
elif isinstance(stdout, int):
1354-
c2pwrite = msvcrt.get_osfhandle(stdout)
1355-
else:
1356-
# Assuming file-like object
1357-
c2pwrite = msvcrt.get_osfhandle(stdout.fileno())
1358-
c2pwrite = self._make_inheritable(c2pwrite)
1359-
1360-
if stderr is None:
1361-
errwrite = _winapi.GetStdHandle(_winapi.STD_ERROR_HANDLE)
1362-
if errwrite is None:
1363-
_, errwrite = _winapi.CreatePipe(None, 0)
1364-
errwrite = Handle(errwrite)
1365-
_winapi.CloseHandle(_)
1366-
elif stderr == PIPE:
1367-
errread, errwrite = _winapi.CreatePipe(None, 0)
1368-
errread, errwrite = Handle(errread), Handle(errwrite)
1369-
elif stderr == STDOUT:
1370-
errwrite = c2pwrite
1371-
elif stderr == DEVNULL:
1372-
errwrite = msvcrt.get_osfhandle(self._get_devnull())
1373-
elif isinstance(stderr, int):
1374-
errwrite = msvcrt.get_osfhandle(stderr)
1375-
else:
1376-
# Assuming file-like object
1377-
errwrite = msvcrt.get_osfhandle(stderr.fileno())
1378-
errwrite = self._make_inheritable(errwrite)
1326+
stdin_needsclose = False
1327+
stdout_needsclose = False
1328+
stderr_needsclose = False
1329+
1330+
try:
1331+
if stdin is None:
1332+
p2cread = _winapi.GetStdHandle(_winapi.STD_INPUT_HANDLE)
1333+
if p2cread is None:
1334+
stdin_needsclose = True
1335+
p2cread, _ = _winapi.CreatePipe(None, 0)
1336+
p2cread = Handle(p2cread)
1337+
_winapi.CloseHandle(_)
1338+
elif stdin == PIPE:
1339+
stdin_needsclose = True
1340+
p2cread, p2cwrite = _winapi.CreatePipe(None, 0)
1341+
p2cread, p2cwrite = Handle(p2cread), Handle(p2cwrite)
1342+
elif stdin == DEVNULL:
1343+
p2cread = msvcrt.get_osfhandle(self._get_devnull())
1344+
elif isinstance(stdin, int):
1345+
p2cread = msvcrt.get_osfhandle(stdin)
1346+
else:
1347+
# Assuming file-like object
1348+
p2cread = msvcrt.get_osfhandle(stdin.fileno())
1349+
p2cread = self._make_inheritable(p2cread)
1350+
1351+
if stdout is None:
1352+
c2pwrite = _winapi.GetStdHandle(_winapi.STD_OUTPUT_HANDLE)
1353+
if c2pwrite is None:
1354+
stdout_needsclose = True
1355+
_, c2pwrite = _winapi.CreatePipe(None, 0)
1356+
c2pwrite = Handle(c2pwrite)
1357+
_winapi.CloseHandle(_)
1358+
elif stdout == PIPE:
1359+
stdout_needsclose = True
1360+
c2pread, c2pwrite = _winapi.CreatePipe(None, 0)
1361+
c2pread, c2pwrite = Handle(c2pread), Handle(c2pwrite)
1362+
elif stdout == DEVNULL:
1363+
c2pwrite = msvcrt.get_osfhandle(self._get_devnull())
1364+
elif isinstance(stdout, int):
1365+
c2pwrite = msvcrt.get_osfhandle(stdout)
1366+
else:
1367+
# Assuming file-like object
1368+
c2pwrite = msvcrt.get_osfhandle(stdout.fileno())
1369+
c2pwrite = self._make_inheritable(c2pwrite)
1370+
1371+
if stderr is None:
1372+
errwrite = _winapi.GetStdHandle(_winapi.STD_ERROR_HANDLE)
1373+
if errwrite is None:
1374+
stderr_needsclose = True
1375+
_, errwrite = _winapi.CreatePipe(None, 0)
1376+
errwrite = Handle(errwrite)
1377+
_winapi.CloseHandle(_)
1378+
elif stderr == PIPE:
1379+
7802 stderr_needsclose = True
1380+
errread, errwrite = _winapi.CreatePipe(None, 0)
1381+
errread, errwrite = Handle(errread), Handle(errwrite)
1382+
elif stderr == STDOUT:
1383+
errwrite = c2pwrite
1384+
elif stderr == DEVNULL:
1385+
errwrite = msvcrt.get_osfhandle(self._get_devnull())
1386+
elif isinstance(stderr, int):
1387+
errwrite = msvcrt.get_osfhandle(stderr)
1388+
else:
1389+
# Assuming file-like object
1390+
errwrite = msvcrt.get_osfhandle(stderr.fileno())
1391+
errwrite = self._make_inheritable(errwrite)
1392+
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
13791413

13801414
return (p2cread, p2cwrite,
13811415
c2pread, c2pwrite,
@@ -1662,52 +1696,69 @@ def _get_handles(self, stdin, stdout, stderr):
16621696
c2pread, c2pwrite = -1, -1
16631697
errread, errwrite = -1, -1
16641698

1665-
if stdin is None:
1666-
pass
1667-
elif stdin == PIPE:
1668-
p2cread, p2cwrite = os.pipe()
1669-
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1670-
fcntl.fcntl(p2cwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1671-
elif stdin == DEVNULL:
1672-
p2cread = self._get_devnull()
1673-
elif isinstance(stdin, int):
1674-
p2cread = stdin
1675-
else:
1676-
# Assuming file-like object
1677-
p2cread = stdin.fileno()
1699+
try:
1700+
if stdin is None:
1701+
pass
1702+
elif stdin == PIPE:
1703+
p2cread, p2cwrite = os.pipe()
1704+
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1705+
fcntl.fcntl(p2cwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1706+
elif stdin == DEVNULL:
1707+
p2cread = self._get_devnull()
1708+
elif isinstance(stdin, int):
1709+
p2cread = stdin
1710+
else:
1711+
# Assuming file-like object
1712+
p2cread = stdin.fileno()
16781713

1679-
if stdout is None:
1680-
pass
1681-
elif stdout == PIPE:
1682-
c2pread, c2pwrite = os.pipe()
1683-
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1684-
fcntl.fcntl(c2pwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1685-
elif stdout == DEVNULL:
1686-
c2pwrite = self._get_devnull()
1687-
elif isinstance(stdout, int):
1688-
c2pwrite = stdout
1689-
else:
1690-
# Assuming file-like object
1691-
c2pwrite = stdout.fileno()
1714+
if stdout is None:
1715+
pass
1716+
elif stdout == PIPE:
1717+
c2pread, c2pwrite = os.pipe()
1718+
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1719+
fcntl.fcntl(c2pwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1720+
elif stdout == DEVNULL:
1721+
c2pwrite = self._get_devnull()
1722+
elif isinstance(stdout, int):
1723+
c2pwrite = stdout
1724+
else:
1725+
# Assuming file-like object
1726+
c2pwrite = stdout.fileno()
16921727

1693-
if stderr is None:
1694-
pass
1695-
elif stderr == PIPE:
1696-
errread, errwrite = os.pipe()
1697-
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1698-
fcntl.fcntl(errwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1699-
elif stderr == STDOUT:
1700-
if c2pwrite != -1:
1701-
errwrite = c2pwrite
1702-
else: # child's stdout is not set, use parent's stdout
1703-
errwrite = sys.__stdout__.fileno()
1704-
elif stderr == DEVNULL:
1705-
errwrite = self._get_devnull()
1706-
elif isinstance(stderr, int):
1707-
errwrite = stderr
1708-
else:
1709-
# Assuming file-like object
1710-
errwrite = stderr.fileno()
1728+
if stderr is None:
1729+
pass
1730+
elif stderr == PIPE:
1731+
errread, errwrite = os.pipe()
1732+
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1733+
fcntl.fcntl(errwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1734+
elif stderr == STDOUT:
1735+
if c2pwrite != -1:
1736+
errwrite = c2pwrite
1737+
else: # child's stdout is not set, use parent's stdout
1738+
errwrite = sys.__stdout__.fileno()
1739+
elif stderr == DEVNULL:
1740+
errwrite = self._get_devnull()
1741+
elif isinstance(stderr, int):
1742+
errwrite = stderr
1743+
else:
1744+
# Assuming file-like object
1745+
errwrite = stderr.fileno()
1746+
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
17111762

17121763
return (p2cread, p2cwrite,
17131764
c2pread, c2pwrite,

‎Misc/NEWS.d/next/Library/2022-08-27-21-41-41.gh-issue-87474.9X-kxt.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix file descriptor leaks in subprocess.Popen

0 commit comments

Comments
 (0)
0