Skip to content

Commit e299737

Browse files
cptpcrdcptpcrd
committed
Fix file descriptor leaks in subprocess.Popen
1 parent 9151bbe commit e299737

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
@@ -870,37 +870,6 @@ def __init__(self, args, bufsize=-1, executable=None,
870870
'and universal_newlines are supplied but '
871871
'different. Pass one or the other.')
872872

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

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

1322-
if stdin is None:
1323-
p2cread = _winapi.GetStdHandle(_winapi.STD_INPUT_HANDLE)
1324-
if p2cread is None:
1325-
p2cread, _ = _winapi.CreatePipe(None, 0)
1326-
p2cread = Handle(p2cread)
1327-
_winapi.CloseHandle(_)
1328-
elif stdin == PIPE:
1329-
p2cread, p2cwrite = _winapi.CreatePipe(None, 0)
1330-
p2cread, p2cwrite = Handle(p2cread), Handle(p2cwrite)
1331-
elif stdin == DEVNULL:
1332-
p2cread = msvcrt.get_osfhandle(self._get_devnull())
1333-
elif isinstance(stdin, int):
1334-
p2cread = msvcrt.get_osfhandle(stdin)
1335-
else:
1336-
# Assuming file-like object
1337-
p2cread = msvcrt.get_osfhandle(stdin.fileno())
1338-
p2cread = self._make_inheritable(p2cread)
1339-
1340-
if stdout is None:
1341-
c2pwrite = _winapi.GetStdHandle(_winapi.STD_OUTPUT_HANDLE)
1342-
if c2pwrite is None:
1343-
_, c2pwrite = _winapi.CreatePipe(None, 0)
1344-
c2pwrite = Handle(c2pwrite)
1345-
_winapi.CloseHandle(_)
1346-
elif stdout == PIPE:
1347-
c2pread, c2pwrite = _winapi.CreatePipe(None, 0)
1348-
c2pread, c2pwrite = Handle(c2pread), Handle(c2pwrite)
1349-
elif stdout == DEVNULL:
1350-
c2pwrite = msvcrt.get_osfhandle(self._get_devnull())
1351-
elif isinstance(stdout, int):
1352-
c2pwrite = msvcrt.get_osfhandle(stdout)
1353-
else:
1354-
# Assuming file-like object
1355-
c2pwrite = msvcrt.get_osfhandle(stdout.fileno())
1356-
c2pwrite = self._make_inheritable(c2pwrite)
1357-
1358-
if stderr is None:
1359-
errwrite = _winapi.GetStdHandle(_winapi.STD_ERROR_HANDLE)
1360-
if errwrite is None:
1361-
_, errwrite = _winapi.CreatePipe(None, 0)
1362-
errwrite = Handle(errwrite)
1363-
_winapi.CloseHandle(_)
1364-
elif stderr == PIPE:
1365-
errread, errwrite = _winapi.CreatePipe(None, 0)
1366-
errread, errwrite = Handle(errread), Handle(errwrite)
1367-
elif stderr == STDOUT:
1368-
errwrite = c2pwrite
1369-
elif stderr == DEVNULL:
1370-
errwrite = msvcrt.get_osfhandle(self._get_devnull())
1371-
elif isinstance(stderr, int):
1372-
errwrite = msvcrt.get_osfhandle(stderr)
1373-
else:
1374-
# Assuming file-like object
1375-
errwrite = msvcrt.get_osfhandle(stderr.fileno())
1376-
errwrite = self._make_inheritable(errwrite)
1324+
stdin_needsclose = False
1325+
stdout_needsclose = False
1326+
stderr_needsclose = False
1327+
1328+
try:
1329+
if stdin is None:
1330+
p2cread = _winapi.GetStdHandle(_winapi.STD_INPUT_HANDLE)
1331+
if p2cread is None:
1332+
stdin_needsclose = True
1333+
p2cread, _ = _winapi.CreatePipe(None, 0)
1334+
p2cread = Handle(p2cread)
1335+
_winapi.CloseHandle(_)
1336+
elif stdin == PIPE:
1337+
stdin_needsclose = True
1338+
p2cread, p2cwrite = _winapi.CreatePipe(None, 0)
1339+
p2cread, p2cwrite = Handle(p2cread), Handle(p2cwrite)
1340+
elif stdin == DEVNULL:
1341+
p2cread = msvcrt.get_osfhandle(self._get_devnull())
1342+
elif isinstance(stdin, int):
1343+
p2cread = msvcrt.get_osfhandle(stdin)
1344+
else:
1345+
# Assuming file-like object
1346+
p2cread = msvcrt.get_osfhandle(stdin.fileno())
1347+
p2cread = self._make_inheritable(p2cread)
1348+
1349+
if stdout is None:
1350+
c2pwrite = _winapi.GetStdHandle(_winapi.STD_OUTPUT_HANDLE)
1351+
if c2pwrite is None:
1352+
stdout_needsclose = True
1353+
_, c2pwrite = _winapi.CreatePipe(None, 0)
1354+
c2pwrite = Handle(c2pwrite)
1355+
_winapi.CloseHandle(_)
1356+
elif stdout == PIPE:
1357+
stdout_needsclose = True
1358+
c2pread, c2pwrite = _winapi.CreatePipe(None, 0)
1359+
c2pread, c2pwrite = Handle(c2pread), Handle(c2pwrite)
1360+
elif stdout == DEVNULL:
1361+
c2pwrite = msvcrt.get_osfhandle(self._get_devnull())
1362+
elif isinstance(stdout, int):
1363+
c2pwrite = msvcrt.get_osfhandle(stdout)
1364+
else:
1365+
# Assuming file-like object
1366+
c2pwrite = msvcrt.get_osfhandle(stdout.fileno())
1367+
c2pwrite = self._make_inheritable(c2pwrite)
1368+
1369+
if stderr is None:
1370+
errwrite = _winapi.GetStdHandle(_winapi.STD_ERROR_HANDLE)
1371+
if errwrite is None:
1372+
stderr_needsclose = True
1373+
_, errwrite = _winapi.CreatePipe(None, 0)
1374+
errwrite = Handle(errwrite)
1375+
_winapi.CloseHandle(_)
1376+
elif stderr == PIPE:
1377+
stderr_needsclose = True
1378+
errread, errwrite = _winapi.CreatePipe(None, 0)
1379+
errread, errwrite = Handle(errread), Handle(errwrite)
1380+
elif stderr == STDOUT:
1381+
errwrite = c2pwrite
1382+
elif stderr == DEVNULL:
1383+
errwrite = msvcrt.get_osfhandle(self._get_devnull())
1384+
elif isinstance(stderr, int):
1385+
errwrite = msvcrt.get_osfhandle(stderr)
1386+
else:
1387+
# Assuming file-like object
1388+
errwrite = msvcrt.get_osfhandle(stderr.fileno())
1389+
errwrite = self._make_inheritable(errwrite)
1390+
1391+
except BaseException:
1392+
to_close = []
1393+
if stdin_needsclose and p2cwrite != -1:
1394+
to_close.append(p2cread)
1395+
to_close.append(p2cwrite)
1396+
if stdout_needsclose and p2cwrite != -1:
1397+
to_close.append(c2pread)
1398+
to_close.append(c2pwrite)
1399+
if stderr_needsclose and errwrite != -1:
1400+
to_close.append(errread)
1401+
to_close.append(errwrite)
1402+
for file in to_close:
1403+
if isinstance(file, Handle):
1404+
file.Close()
1405+
else:
1406+
os.close(file)
1407+
if hasattr(self, "_devnull"):
1408+
os.close(self._devnull)
1409+
del self._devnull
1410+
raise
13771411

13781412
return (p2cread, p2cwrite,
13791413
c2pread, c2pwrite,
@@ -1644,52 +1678,69 @@ def _get_handles(self, stdin, stdout, stderr):
16441678
c2pread, c2pwrite = -1, -1
16451679
errread, errwrite = -1, -1
16461680

1647-
if stdin is None:
1648-
pass
1649-
elif stdin == PIPE:
1650-
p2cread, p2cwrite = os.pipe()
1651-
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1652-
fcntl.fcntl(p2cwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1653-
elif stdin == DEVNULL:
1654-
p2cread = self._get_devnull()
1655-
elif isinstance(stdin, int):
1656-
p2cread = stdin
1657-
else:
1658-
# Assuming file-like object
1659-
p2cread = stdin.fileno()
1681+
try:
1682+
if stdin is None:
1683+
pass
1684+
elif stdin == PIPE:
1685+
p2cread, p2cwrite = os.pipe()
1686+
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1687+
fcntl.fcntl(p2cwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1688+
elif stdin == DEVNULL:
1689+
p2cread = self._get_devnull()
1690+
elif isinstance(stdin, int):
1691+
p2cread = stdin
1692+
else:
1693+
# Assuming file-like object
1694+
p2cread = stdin.fileno()
16601695

1661-
if stdout is None:
1662-
pass
1663-
elif stdout == PIPE:
1664-
c2pread, c2pwrite = os.pipe()
1665-
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1666-
fcntl.fcntl(c2pwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1667-
elif stdout == DEVNULL:
1668-
c2pwrite = self._get_devnull()
1669-
elif isinstance(stdout, int):
1670-
c2pwrite = stdout
1671-
else:
1672-
# Assuming file-like object
1673-
c2pwrite = stdout.fileno()
1696+
if stdout is None:
1697+
pass
1698+
elif stdout == PIPE:
1699+
c2pread, c2pwrite = os.pipe()
1700+
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1701+
fcntl.fcntl(c2pwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1702+
elif stdout == DEVNULL:
1703+
c2pwrite = self._get_devnull()
1704+
elif isinstance(stdout, int):
1705+
c2pwrite = stdout
1706+
else:
1707+
# Assuming file-like object
1708+
c2pwrite = stdout.fileno()
16741709

1675-
if stderr is None:
1676-
pass
1677-
elif stderr == PIPE:
1678-
errread, errwrite = os.pipe()
1679-
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1680-
fcntl.fcntl(errwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1681-
elif stderr == STDOUT:
1682-
if c2pwrite != -1:
1683-
errwrite = c2pwrite
1684-
else: # child's stdout is not set, use parent's stdout
1685-
errwrite = sys.__stdout__.fileno()
1686-
elif stderr == DEVNULL:
1687-
errwrite = self._get_devnull()
1688-
elif isinstance(stderr, int):
1689-
errwrite = stderr
1690-
else:
1691-
# Assuming file-like object
1692-
errwrite = stderr.fileno()
1710+
if stderr is None:
1711+
pass
1712+
elif stderr == PIPE:
1713+
errread, errwrite = os.pipe()
1714+
if self.pipesize > 0 and hasattr(fcntl, "F_SETPIPE_SZ"):
1715+
fcntl.fcntl(errwrite, fcntl.F_SETPIPE_SZ, self.pipesize)
1716+
elif stderr == STDOUT:
1717+
if c2pwrite != -1:
1718+
errwrite = c2pwrite
1719+
else: # child's stdout is not set, use parent's stdout
1720+
errwrite = sys.__stdout__.fileno()
1721+
elif stderr == DEVNULL:
1722+
errwrite = self._get_devnull()
1723+
elif isinstance(stderr, int):
1724+
errwrite = stderr
1725+
else:
1726+
# Assuming file-like object
1727+
errwrite = stderr.fileno()
1728+
1729+
except BaseException:
1730+
# Close the file descriptors we opened to avoid leakage
1731+
if stdin == PIPE and p2cwrite != -1:
1732+
os.close(p2cread)
1733+
os.close(p2cwrite)
1734+
if stdout == PIPE and c2pwrite != -1:
1735+
os.close(c2pread)
1736+
os.close(c2pwrite)
1737+
if stderr == PIPE and errwrite != -1:
1738+
os.close(errread)
1739+
os.close(errwrite)
1740+
if hasattr(self, "_devnull"):
1741+
os.close(self._devnull)
1742+
del self._devnull
1743+
raise
16931744

16941745
return (p2cread, p2cwrite,
16951746
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)