| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 2ac1611 commit 44d3d9e
3 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -26,7 +26,6 @@ def __init__(self, loop, protocol, args, shell, | |||
| 26 | 26 | self._pending_calls = collections.deque() | |
| 27 | 27 | self._pipes = {} | |
| 28 | 28 | self._finished = False | |
| 29 | - self._pipes_connected = False | ||
| 30 | 29 | ||
| 31 | 30 | if stdin == subprocess.PIPE: | |
| 32 | 31 | self._pipes[0] = None | |
@@ -214,7 +213,6 @@ async def _connect_pipes(self, waiter): | |||
| 214 | 213 | else: | |
| 215 | 214 | if waiter is not None and not waiter.cancelled(): | |
| 216 | 215 | waiter.set_result(None) | |
| 217 | - self._pipes_connected = True | ||
| 218 | 216 | ||
| 219 | 217 | def _call(self, cb, *data): | |
| 220 | 218 | if self._pending_calls is not None: | |
@@ -235,6 +233,16 @@ def _process_exited(self, returncode): | |||
| 235 | 233 | if self._loop.get_debug(): | |
| 236 | 234 | logger.info('%r exited with return code %r', self, returncode) | |
| 237 | 235 | self._returncode = returncode | |
| 236 | + | ||
| 237 | + # gh-119710: Wake up futures waiting for wait() as soon as the process | ||
| 238 | + # exits. The pipe transports now check for the loop being closed before | ||
| 239 | + # scheduling a callback preventing gh-114177. This is consistent with | ||
| 240 | + # the behavior prior to 3.11 and the documented semantics in _wait(). | ||
| 241 | + for waiter in self._exit_waiters: | ||
| 242 | + if not waiter.done(): | ||
| 243 | + waiter.set_result(returncode) | ||
| 244 | + self._exit_waiters = None | ||
| 245 | + | ||
| 238 | 246 | if self._proc.returncode is None: | |
| 239 | 247 | # asyncio uses a child watcher: copy the status into the Popen | |
| 240 | 248 | # object. On Python 3.6, it is required to avoid a ResourceWarning. | |
@@ -258,15 +266,7 @@ def _try_finish(self): | |||
| 258 | 266 | assert not self._finished | |
| 259 | 267 | if self._returncode is None: | |
| 260 | 268 | return | |
| 261 | - if not self._pipes_connected: | ||
| 262 | - # self._pipes_connected can be False if not all pipes were connected | ||
| 263 | - # because either the process failed to start or the self._connect_pipes task | ||
| 264 | - # got cancelled. In this broken state we consider all pipes disconnected and | ||
| 265 | - # to avoid hanging forever in self._wait as otherwise _exit_waiters | ||
| 266 | - # would never be woken up, we wake them up here. | ||
| 267 | - for waiter in self._exit_waiters: | ||
| 268 | - if not waiter.done(): | ||
| 269 | - waiter.set_result(self._returncode) | ||
| 269 | + | ||
| 270 | 270 | if all(p is not None and p.disconnected | |
| 271 | 271 | for p in self._pipes.values()): | |
| 272 | 272 | self._finished = True | |
@@ -276,11 +276,6 @@ def _call_connection_lost(self, exc): | |||
| 276 | 276 | try: | |
| 277 | 277 | self._protocol.connection_lost(exc) | |
| 278 | 278 | finally: | |
| 279 | - # wake up futures waiting for wait() | ||
| 280 | - for waiter in self._exit_waiters: | ||
| 281 | - if not waiter.done(): | ||
| 282 | - waiter.set_result(self._returncode) | ||
| 283 | - self._exit_waiters = None | ||
| 284 | 279 | self._loop = None | |
| 285 | 280 | self._proc = None | |
| 286 | 281 | self._protocol = None | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -128,9 +128,6 @@ def test_proc_exited_no_invalid_state_error_on_exit_waiters(self): | |||
| 128 | 128 | exit_waiter = self.loop.create_future() | |
| 129 | 129 | transport._exit_waiters.append(exit_waiter) | |
| 130 | 130 | ||
| 131 | - # _connect_pipes hasn't completed, so _pipes_connected is False. | ||
| 132 | - self.assertFalse(transport._pipes_connected) | ||
| 133 | - | ||
| 134 | 131 | # Simulate process exit. _try_finish() will set the result on | |
| 135 | 132 | # exit_waiter because _pipes_connected is False, and then schedule | |
| 136 | 133 | # _call_connection_lost() because _pipes is empty (vacuously all | |
@@ -143,6 +140,32 @@ def test_proc_exited_no_invalid_state_error_on_exit_waiters(self): | |||
| 143 | 140 | ||
| 144 | 141 | transport.close() | |
| 145 | 142 | ||
| 143 | + def test_wait_returns_on_exit_with_open_pipe(self): | ||
| 144 | + # gh-119710: wait() must resolve when the process exits even if a | ||
| 145 | + # pipe is still open and never reaches EOF (e.g. inherited by a | ||
| 146 | + # grandchild). Otherwise _call_connection_lost() never runs and | ||
| 147 | + # _wait() would hang forever despite the returncode being known. | ||
| 148 | + transport, protocol = self.create_transport() | ||
| 149 | + | ||
| 150 | + # Pipes are fully connected, but fd 1 stays open (never disconnects). | ||
| 151 | + pipe = mock.Mock() | ||
| 152 | + pipe.disconnected = False | ||
| 153 | + transport._pipes[1] = pipe | ||
| 154 | + | ||
| 155 | + # A waiter registered via _wait() before the process exits. | ||
| 156 | + exit_waiter = self.loop.create_future() | ||
| 157 | + transport._exit_waiters.append(exit_waiter) | ||
| 158 | + | ||
| 159 | + # _process_exited() must resolve exit_waiter even though the pipe | ||
| 160 | + # never disconnects (so _call_connection_lost() never runs). Without | ||
| 161 | + # the fix, exit_waiter stays pending forever and this hangs. | ||
| 162 | + transport._process_exited(7) | ||
| 163 | + self.loop.run_until_complete(exit_waiter) | ||
| 164 | + | ||
| 165 | + self.assertEqual(exit_waiter.result(), 7) | ||
| 166 | + | ||
| 167 | + transport.close() | ||
| 168 | + | ||
| 146 | 169 | ||
| 147 | 170 | class SubprocessMixin: | |
| 148 | 171 | ||
@@ -436,6 +459,47 @@ async def len_message(message): | |||
| 436 | 459 | self.assertEqual(output.rstrip(), b'3') | |
| 437 | 460 | self.assertEqual(exitcode, 0) | |
| 438 | 461 | ||
| 462 | + def test_wait_even_if_pipe_is_open(self): | ||
| 463 | + # gh-119710: Process.wait() must return once the process exits even | ||
| 464 | + # if its stdout pipe is inherited by a grandchild that keeps it open, | ||
| 465 | + # so the pipe never reaches EOF. Otherwise wait() hangs forever | ||
| 466 | + # despite the returncode being known. | ||
| 467 | + | ||
| 468 | + async def run(): | ||
| 469 | + # Just setup a pipe to pass to the grandchild for reading to ensure it dies. | ||
| 470 | + # Inheritable is to allow it to be passed on windows | ||
| 471 | + r, w = os.pipe() | ||
| 472 | + os.set_inheritable(r, True) | ||
| 473 | + | ||
| 474 | + code = textwrap.dedent(f"""\ | ||
| 475 | + import subprocess, sys | ||
| 476 | + subprocess.run([sys.executable, "-c", "import sys;sys.stdin.read()"]) | ||
| 477 | + """) | ||
| 478 | + | ||
| 479 | + proc = await asyncio.create_subprocess_exec( | ||
| 480 | + sys.executable, "-c", code, | ||
| 481 | + # This will be inherited by granchild and should not prevent | ||
| 482 | + # *this* process from firing .wait(). | ||
| 483 | + stdout=subprocess.PIPE, | ||
| 484 | + stdin=r, | ||
| 485 | + pass_fds=(r,) if sys.platform != "win32" else (), | ||
| 486 | + close_fds=False if sys.platform == "win32" else True, | ||
| 487 | + ) | ||
| 488 | + os.close(r) | ||
| 489 | + | ||
| 490 | + try: | ||
| 491 | + # Ensure we start waiting before the process is killed. | ||
| 492 | + wait_proc = asyncio.create_task(proc.wait()) | ||
| 493 | + await asyncio.sleep(0.1) | ||
| 494 | + proc.kill() | ||
| 495 | + await asyncio.wait_for(wait_proc, timeout=2.0) | ||
| 496 | + finally: | ||
| 497 | + os.close(w) # Allows the grandchild to exit | ||
| 498 | + if proc.stdout is not None: | ||
| 499 | + await proc.stdout.read() | ||
| 500 | + | ||
| 501 | + self.loop.run_until_complete(run()) | ||
| 502 | + | ||
| 439 | 503 | def test_empty_input(self): | |
| 440 | 504 | ||
| 441 | 505 | async def empty_input(): | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,4 @@ | |||
| 1 | + Fix :mod:`asyncio` subprocess :meth:`~asyncio.subprocess.Process.wait` | ||
| 2 | + hanging when the process has exited but one of its pipes is kept open by an | ||
| 3 | + inherited child process (so the pipe never reaches EOF). ``wait()`` now | ||
| 4 | + returns as soon as the process exits, regardless of the pipes' state. | ||
| Back | FazBrowse Home | New Git URL |
0 commit comments