[PR #6727/adeece3c backport][3.8] Fix asyncio.CancelledError again (#6719) #6741
+1
−2
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a backport of PR #6727 as merged into master (adeece3).
asyncio.CancelledError()
on peer disconnection have been removed by #4080,but #4415 re-introduced it silently.
self._waiter.cancel()
andself._task_handler.cancel()
were added by #4415,but #4415 in fact only needed
self._waiter.cancel()
(proof below).So I propose to remove
self._task_handler.cancel()
, both #4080 and #4415 willbe fixed.
To test that I re-resolved #4080 I used:
curl -X POST -H "Content-Type: text/plain" --data "bouh" localhost:8080
I kill the curl request before the 5 seconds of sleep.
Before this PR I have the following error right after killing the curl:
After this commit I have the following error, but only after the 5 seconds
sleep:
To test that I didn't re-introduce #4415 I use a basic
handle
and 30curl localhost:8080
:Before this commit no issue
If I remove
self._task_handler.cancel()
no issueIf I remove both
self._task_handler.cancel()
andself._waiter.cancel()
:So it's OK to remove only
self._task_handler.cancel()
.There is no documentation or tests to be updated because #4080 already did that
part.
However I guess that for a few people it might be seen as a breaking change,
I'm not sure.
Tested on master and on v3.8.1.
fixes #6719