Skip to content

fix(asyncio): do not hang when async_playwright() is cancelled while connecting - #3177

Closed
Yury Semikhatsky (yury-s) wants to merge 3 commits into
microsoft:mainfrom
yury-s:fix-42296
Closed

fix(asyncio): do not hang when async_playwright() is cancelled while connecting#3177
Yury Semikhatsky (yury-s) wants to merge 3 commits into
microsoft:mainfrom
yury-s:fix-42296

Conversation

@yury-s

Copy link
Copy Markdown
Member

Summary

  • stop the connection (driver process and background tasks) when __aenter__ fails or is cancelled, reusing __aexit__ like the sync context manager does
  • allow PipeTransport.request_stop() before the driver has spawned
  • release pending protocol callbacks when the Connection.run() task is cancelled at loop shutdown, so their cancellation is not absorbed by Channel._abort() waiting for a reply that can never arrive
  • retrieve callback exceptions in Channel._abort() even when its wait is interrupted by another cancellation

Fixes microsoft/playwright#42296

…connecting

Cancelling __aenter__ left the driver process and the transport tasks
running with no owner. At loop shutdown the init task absorbed its one
cancellation inside Channel._abort() waiting for a reply that could
never arrive, so asyncio.run() never returned.

- stop the connection when __aenter__ is cancelled
- allow PipeTransport.request_stop() before the driver has spawned
- release pending protocol callbacks when the Connection.run() task is
  cancelled at loop shutdown
- retrieve callback exceptions in Channel._abort() even when its wait is
  interrupted by another cancellation

Fixes: microsoft/playwright#42296
- reuse __aexit__ for teardown when __aenter__ fails or is cancelled,
  mirroring the sync context manager
- move _stopped_future and wait_until_stopped() into the Transport base,
  deleting both subclass copies
- reuse request_stop() when connect() finishes after a stop request
- skip sending __abort__ over a closed connection
await self._transport.run()
try:
await self._transport.run()
except asyncio.CancelledError:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why only on CancelledError and not on any error? How is this one special?

async def _abort(
self, object: ChannelOwner, callback: ProtocolCallback, reason: str
) -> None:
if not self._closed_error:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we would benefit from another test, where we cancel a task like click(), while at the same time closing the connection. I don't think existing tests cover this change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: async_playwright() leaves blocking background task if cancelled quickly

2 participants