wip

Author: daandemeyer

Lib/asyncio/base_subprocess.py

@@ -265,7 +265,7 @@ def _try_finish(self):
             # to avoid hanging forever in self._wait as otherwise _exit_waiters
             # would never be woken up, we wake them up here.
             for waiter in self._exit_waiters:
-                if not waiter.cancelled():
+                if not waiter.done():
                     waiter.set_result(self._returncode)
         if all(p is not None and p.disconnected
                for p in self._pipes.values()):
@@ -278,7 +278,7 @@ def _call_connection_lost(self, exc):
         finally:
             # wake up futures waiting for wait()
             for waiter in self._exit_waiters:
-                if not waiter.cancelled():
+                if not waiter.done():
                     waiter.set_result(self._returncode)
             self._exit_waiters = None
             self._loop = None

Lib/test/test_asyncio/test_subprocess.py

@@ -111,6 +111,31 @@ def test_subprocess_repr(self):
         )
         transport.close()
 
+    def test_proc_exited_no_invalid_state_error_on_exit_waiters(self):
+        # gh-145541: when _connect_pipes hasn't completed (so
+        # _pipes_connected is False) and the process exits, _try_finish()
+        # sets the result on exit waiters. Then _call_connection_lost() must
+        # not call set_result() again on the same waiters.
+        waiter = self.loop.create_future()
+        transport, protocol = self.create_transport(waiter)
+
+        # Simulate a waiter registered via _wait() before the process exits.
+        exit_waiter = self.loop.create_future()
+        transport._exit_waiters.append(exit_waiter)
+
+        # _connect_pipes hasn't completed, so _pipes_connected is False.
+        self.assertFalse(transport._pipes_connected)
+
+        # Simulate process exit. _try_finish() will set the result on
+        # exit_waiter because _pipes_connected is False, and then schedule
+        # _call_connection_lost() because _pipes is empty (vacuously all
+        # disconnected). _call_connection_lost() must skip exit_waiter
+        # because it's already done.
+        transport._process_exited(6)
+        self.loop.run_until_complete(waiter)
+
+        self.assertEqual(exit_waiter.result(), 6)
+
 
 class SubprocessMixin:
 
@@ -918,6 +943,36 @@ async def main():
         asyncio.run(main())
         gc_collect()
 
+    @warnings_helper.ignore_warnings(category=ResourceWarning)
+    def test_subprocess_pipe_cancelled_no_invalid_state_error(self):
+        # gh-145541: when _connect_pipes is cancelled and the process
+        # subsequently exits, _call_connection_lost() must not raise
+        # InvalidStateError by calling set_result() on exit waiters that
+        # were already resolved by _try_finish().
+        async def main():
+            loop = asyncio.get_running_loop()
+            loop.connect_read_pipe = mock.AsyncMock(
+                side_effect=asyncio.CancelledError,
+            )
+
+            proc = None
+            with self.assertRaises(asyncio.CancelledError):
+                proc = await asyncio.create_subprocess_exec(
+                    *PROGRAM_BLOCKED,
+                    stdout=asyncio.subprocess.PIPE,
+                )
+
+            if proc is not None:
+                proc.kill()
+                # wait() adds an exit waiter. Before the fix,
+                # _call_connection_lost() would call set_result() on it
+                # after _try_finish() already did, raising
+                # InvalidStateError.
+                await proc.wait()
+
+        asyncio.run(main())
+        gc_collect()
+
 if sys.platform != 'win32':
     # Unix
     class SubprocessWatcherMixin(SubprocessMixin):