Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PR #6727/adeece3c backport][3.9] Fix asyncio.CancelledError again (#6719) #6742

Commits on May 8, 2022

  1. Fix asyncio.CancelledError again (#6719) (#6727)

    * Fix asyncio.CancelledError again (#6719)
    
    `asyncio.CancelledError()` on peer disconnection have been removed by #4080,
    but #4415 re-introduced it silently.
    
    `self._waiter.cancel()` and `self._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 will
    be fixed.
    
    To test that I re-resolved #4080 I used:
    ```py
    async def handle(request):
        try:
            await asyncio.sleep(5)
            await request.text()
        except BaseException as e:
            print(f'base exception {type(e).__name__}')
        return web.Response(text='toto')
    ```
    ```console
    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:
    ```console
    $ python -Wall -X dev test.py
    ======== Running on http://0.0.0.0:8080 ========
    (Press CTRL+C to quit)
    base exception CancelledError
    ```
    
    After this commit I have the following error, but only after the 5 seconds
    sleep:
    ```console
    $ python -Wall -X dev test.py
    ======== Running on http://0.0.0.0:8080 ========
    (Press CTRL+C to quit)
    base exception ConnectionResetError
    ```
    
    To test that I didn't re-introduce #4415 I use a basic `handle` and 30
    `curl localhost:8080`:
    
    - Before this commit no issue
    - If I remove `self._task_handler.cancel()` no issue
    - If I remove both `self._task_handler.cancel()` and `self._waiter.cancel()`:
    
      ```console
      $ python -Wall -X dev test.py
      ======== Running on http://0.0.0.0:8080 ========
      (Press CTRL+C to quit)
      Task was destroyed but it is pending!
      task: <Task pending name='Task-4' coro=<RequestHandler.start() done, defined at ...
      Task was destroyed but it is pending!
      ...
      ```
    
    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
    
    * Update 6719.bugfix
    
    Co-authored-by: Sam Bull <aa6bs0@sambull.org>
    (cherry picked from commit adeece3)
    H--o-l authored and patchback[bot] committed May 8, 2022
    Configuration menu
    Copy the full SHA
    ea15256 View commit details
    Browse the repository at this point in the history