cpython icon indicating copy to clipboard operation
cpython copied to clipboard

GH-96764: rewrite `asyncio.wait_for` to use `asyncio.timeout`

Open kumaraditya303 opened this issue 3 years ago • 3 comments

This PR changes asyncio.wait_for to use asyncio.timeout as its underlying implementation. It simplifies the code and makes it easy to understand the cancellation semantics as both asyncio.timeout and asyncio.wait_for behaves similarly.

Fixes https://github.com/python/cpython/issues/86296 Fixes https://github.com/python/cpython/issues/81839 Fixes #96764

  • Issue: gh-96764

kumaraditya303 avatar Oct 21 '22 12:10 kumaraditya303

This is more of a POC at this point. I think it is worth changing wait_for to use timeout in 3.12+. This seems to fix some of the open bugs of wait_for, I'll add those tests to make sure.

cc @gvanrossum

kumaraditya303 avatar Oct 21 '22 18:10 kumaraditya303

I had hoped to first merge one of the alternatives that can be backported to 3.10, so we can declare this fixed in 3.10. Then we would merge this one on top of that, but only in 3.11 and main.

But we could also just do a custom fix for 3.10 based on one of @twisteroidambassador's PRs. (I wish you had linked this PR to the same issue rather than creating a new issue, since it's all related.)

gvanrossum avatar Oct 25 '22 18:10 gvanrossum

In particular, @twisteroidambassador has this PR: https://github.com/python/cpython/pull/98607

gvanrossum avatar Oct 25 '22 18:10 gvanrossum

Revisiting this, tests for the bug reports linked are added and existing tests pass without any change except one. The only remaining thing is the removed test_wait_for_self_cancellation test which tests that wait_for creates a future and awaits on it and tests its cancellation. See https://github.com/python/cpython/pull/98518#discussion_r1005269554

kumaraditya303 avatar Jan 05 '23 10:01 kumaraditya303

@gvanrossum Thanks for the review, this was a tough nut to crack.

kumaraditya303 avatar Feb 16 '23 18:02 kumaraditya303