Skip to content

Commit b851ab4

Browse files
committed
gh-145541: Fix InvalidStateError in BaseSubprocessTransport._call_connection_lost()
Change `if not waiter.cancelled()` to `if not waiter.done()` in both _try_finish() and _call_connection_lost() so that waiters whose result was already set by _try_finish() are not set again by _call_connection_lost(), which would raise InvalidStateError. When _connect_pipes is cancelled (e.g. by SIGINT during subprocess creation), _pipes_connected stays False. If the process then exits, _try_finish() sets the result on exit waiters because _pipes_connected is False, and then schedules _call_connection_lost() because all pipes are disconnected. _call_connection_lost() must skip waiters that are already done.
1 parent 0c29f83 commit b851ab4

3 files changed

Lines changed: 30 additions & 2 deletions

File tree

Lib/asyncio/base_subprocess.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ def _try_finish(self):
265265
# to avoid hanging forever in self._wait as otherwise _exit_waiters
266266
# would never be woken up, we wake them up here.
267267
for waiter in self._exit_waiters:
268-
if not waiter.cancelled():
268+
if not waiter.done():
269269
waiter.set_result(self._returncode)
270270
if all(p is not None and p.disconnected
271271
for p in self._pipes.values()):
@@ -278,7 +278,7 @@ def _call_connection_lost(self, exc):
278278
finally:
279279
# wake up futures waiting for wait()
280280
for waiter in self._exit_waiters:
281-
if not waiter.cancelled():
281+
if not waiter.done():
282282
waiter.set_result(self._returncode)
283283
self._exit_waiters = None
284284
self._loop = None

Lib/test/test_asyncio/test_subprocess.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,33 @@ def test_subprocess_repr(self):
111111
)
112112
transport.close()
113113

114+
def test_proc_exited_no_invalid_state_error_on_exit_waiters(self):
115+
# gh-145541: when _connect_pipes hasn't completed (so
116+
# _pipes_connected is False) and the process exits, _try_finish()
117+
# sets the result on exit waiters. Then _call_connection_lost() must
118+
# not call set_result() again on the same waiters.
119+
waiter = self.loop.create_future()
120+
transport, protocol = self.create_transport(waiter)
121+
122+
# Simulate a waiter registered via _wait() before the process exits.
123+
exit_waiter = self.loop.create_future()
124+
transport._exit_waiters.append(exit_waiter)
125+
126+
# _connect_pipes hasn't completed, so _pipes_connected is False.
127+
self.assertFalse(transport._pipes_connected)
128+
129+
# Simulate process exit. _try_finish() will set the result on
130+
# exit_waiter because _pipes_connected is False, and then schedule
131+
# _call_connection_lost() because _pipes is empty (vacuously all
132+
# disconnected). _call_connection_lost() must skip exit_waiter
133+
# because it's already done.
134+
transport._process_exited(6)
135+
self.loop.run_until_complete(waiter)
136+
137+
self.assertEqual(exit_waiter.result(), 6)
138+
139+
transport.close()
140+
114141

115142
class SubprocessMixin:
116143

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix InvalidStateError when cancelling process created by :func:`asyncio.create_subprocess_exec` or :func:`asyncio.create_subprocess_shell`. Patch by Daan De Meyer.

0 commit comments

Comments
 (0)