Skip to content

Commit f429fb3

Browse files
gh-103847: fix some asyncio subprocess cancellation bugs (#146571)
1 parent c76fb91 commit f429fb3

4 files changed

Lines changed: 18 additions & 9 deletions

File tree

Lib/asyncio/base_subprocess.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ def __init__(self, loop, protocol, args, shell,
2222
self._proc = None
2323
self._pid = None
2424
self._returncode = None
25-
self._exit_waiters = []
25+
self._exit_waiters = set()
2626
self._pending_calls = collections.deque()
2727
self._pipes = {}
2828
self._finished = False
@@ -211,6 +211,14 @@ async def _connect_pipes(self, waiter):
211211
except (SystemExit, KeyboardInterrupt):
212212
raise
213213
except BaseException as exc:
214+
# Close any pipes that were already connected before the
215+
# error/cancellation to avoid leaking file descriptors.
216+
for proto in self._pipes.values():
217+
if proto is not None:
218+
proto.pipe.close()
219+
for raw_pipe in (proc.stdin, proc.stdout, proc.stderr):
220+
if raw_pipe is not None:
221+
raw_pipe.close()
214222
if waiter is not None and not waiter.cancelled():
215223
waiter.set_exception(exc)
216224
else:
@@ -260,8 +268,12 @@ async def _wait(self):
260268
return self._returncode
261269

262270
waiter = self._loop.create_future()
263-
self._exit_waiters.append(waiter)
264-
return await waiter
271+
self._exit_waiters.add(waiter)
272+
try:
273+
return await waiter
274+
finally:
275+
if self._exit_waiters is not None:
276+
self._exit_waiters.discard(waiter)
265277

266278
def _try_finish(self):
267279
assert not self._finished

Lib/asyncio/unix_events.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ async def _make_subprocess_transport(self, protocol, args, shell,
213213
raise
214214
except BaseException:
215215
transp.close()
216-
await transp._wait()
216+
await tasks.shield(transp._wait())
217217
raise
218218

219219
return transp

Lib/asyncio/windows_events.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ async def _make_subprocess_transport(self, protocol, args, shell,
406406
raise
407407
except BaseException:
408408
transp.close()
409-
await transp._wait()
409+
await tasks.shield(transp._wait())
410410
raise
411411

412412
return transp

Lib/test/test_asyncio/test_subprocess.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from asyncio import subprocess
1313
from test.test_asyncio import utils as test_utils
1414
from test import support
15-
from test.support import os_helper, warnings_helper, gc_collect
15+
from test.support import os_helper, gc_collect
1616

1717
if not support.has_subprocess_support:
1818
raise unittest.SkipTest("test module requires subprocess")
@@ -1028,7 +1028,6 @@ async def main():
10281028

10291029
self.loop.run_until_complete(main())
10301030

1031-
@warnings_helper.ignore_warnings(category=ResourceWarning)
10321031
def test_subprocess_read_pipe_cancelled(self):
10331032
async def main():
10341033
loop = asyncio.get_running_loop()
@@ -1039,7 +1038,6 @@ async def main():
10391038
asyncio.run(main())
10401039
gc_collect()
10411040

1042-
@warnings_helper.ignore_warnings(category=ResourceWarning)
10431041
def test_subprocess_write_pipe_cancelled(self):
10441042
async def main():
10451043
loop = asyncio.get_running_loop()
@@ -1050,7 +1048,6 @@ async def main():
10501048
asyncio.run(main())
10511049
gc_collect()
10521050

1053-
@warnings_helper.ignore_warnings(category=ResourceWarning)
10541051
def test_subprocess_read_write_pipe_cancelled(self):
10551052
async def main():
10561053
loop = asyncio.get_running_loop()

0 commit comments

Comments
 (0)