Skip to content

gh-119710: Let asyncio Process.wait() finish on only process exit.#151983

Open
tapetersen wants to merge 2 commits into
python:mainfrom
tapetersen:gh-119710-subprocess-wait-hang
Open

gh-119710: Let asyncio Process.wait() finish on only process exit.#151983
tapetersen wants to merge 2 commits into
python:mainfrom
tapetersen:gh-119710-subprocess-wait-hang

Conversation

@tapetersen

Copy link
Copy Markdown

gh-119710: Let asyncio Process.wait() finish on only process exit

Letting Process.wait() only wait on actual process return is closer to how it's documented and consistent with Popen.wait(). This also reduces complexity for waking waiters which was inconsistend depending on ordering of wait/exit.

@python-cla-bot

python-cla-bot Bot commented Jun 23, 2026

Copy link
Copy Markdown

The following commit authors need to sign the Contributor License Agreement:

CLA not signed

Letting Process.wait() only wait on actual process return is closer to
how it's documented and consistent with Popen.wait(). This also reduces
complexity for waking waiters which was inconsistend depending on
ordering of wait/exit.
@tapetersen tapetersen force-pushed the gh-119710-subprocess-wait-hang branch from aa43850 to 44d3d9e Compare June 23, 2026 11:36
@tapetersen tapetersen force-pushed the gh-119710-subprocess-wait-hang branch from e774576 to f6df3f5 Compare June 23, 2026 12:07
exit_waiter = self.loop.create_future()
transport._exit_waiters.append(exit_waiter)

# _connect_pipes hasn't completed, so _pipes_connected is False.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My inclination would be to remove this test fully since it should be covered by test_wait_even_if_pipe_is_open in a less implementation dependent way.
With a single place for notifying waiters (instead of 3) this feels too specific.

Comment on lines +241 to +243
for waiter in self._exit_waiters:
if not waiter.done():
waiter.set_result(returncode)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could be changed back to not waiter.cancelled() (reverting that part of #145554) since this should now be the only place they are otherwise resolved.

@tapetersen tapetersen marked this pull request as ready for review June 23, 2026 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant