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

Fix async cancellation behaviour #580

Merged
merged 3 commits into from Nov 7, 2022

Conversation

tomchristie
Copy link
Member

Closes #564.

I haven't come up with a good test case for this yet, but it resolves the example provided in that issue, by ensuring the request queue modifications happen outside of async context switches.

@tomchristie tomchristie added the bug Something isn't working label Sep 29, 2022
@tomchristie tomchristie mentioned this pull request Oct 11, 2022
@Kludex Kludex requested a review from graingert October 11, 2022 19:54
@Kludex
Copy link
Sponsor Member

Kludex commented Nov 2, 2022

Should we wait for a test to be added, or may I ask instructions on how to test it?

@tomchristie
Copy link
Member Author

tomchristie commented Nov 3, 2022

Within a clean directory...

example.py

import trio
import httpcore


async def worker(client, nurse):
    r = await client.request('HEAD', 'https://google.com')
    print(r)
    nurse.cancel_scope.cancel()


async def main():
    async with httpcore.AsyncConnectionPool() as client, trio.open_nursery() as nurse:
        for _ in range(3):
            nurse.start_soon(worker, client, nurse)


if __name__ == "__main__":
    trio.run(main)

Using trio

Now...

$ python3 -m venv venv
$ venv/bin/pip install trio
$ venv/bin/pip install httpcore
$ venv/bin/python ./example.py 
<Response [301]>
Traceback (most recent call last):
  File "/Users/tomchristie/Temp/./example.py", line 18, in <module>
    trio.run(main)
  File "/Users/tomchristie/Temp/venv/lib/python3.9/site-packages/trio/_core/_run.py", line 1946, in run
    raise runner.main_task_outcome.error
  File "/Users/tomchristie/Temp/./example.py", line 14, in main
    nurse.start_soon(worker, client, nurse)
  File "/Users/tomchristie/Temp/venv/lib/python3.9/site-packages/httpcore/_async/connection_pool.py", line 326, in __aexit__
    await self.aclose()
  File "/Users/tomchristie/Temp/venv/lib/python3.9/site-packages/httpcore/_async/connection_pool.py", line 312, in aclose
    raise RuntimeError(
RuntimeError: The connection pool was closed while 2 HTTP requests/responses were still in-flight.

Or with this branch...

$ python3 -m venv venv
$ venv/bin/pip install trio
$ venv/bin/pip install git+https://github.com/encode/httpcore.git@fix-async-cancellation-behaviour
$ venv/bin/python ./example.py 
/Users/tomchristie/Temp/venv/lib/python3.9/site-packages/anyio/_backends/_trio.py:164: TrioDeprecationWarning: trio.MultiError is deprecated since Trio 0.22.0; use BaseExceptionGroup (on Python 3.11 and later) or exceptiongroup.BaseExceptionGroup (earlier versions) instead (https://github.com/python-trio/trio/issues/2211)
  class ExceptionGroup(BaseExceptionGroup, trio.MultiError):
<Response [301]>

Ooops, we've run into agronholm/anyio#470...

Downgrading trio should avoid that (the upcoming anyio 4.0 should resolve it too).

$ python3 -m venv venv
$ venv/bin/pip install trio==0.21.0
$ venv/bin/pip install git+https://github.com/encode/httpcore.git@fix-async-cancellation-behaviour
$ venv/bin/python ./example.py 
<Response [301]>

Using asyncio, with anyio for TaskGroups

example.py

import anyio
import httpcore


async def worker(client, nurse):
    r = await client.request('HEAD', 'https://google.com')
    print(r)
    nurse.cancel_scope.cancel()


async def main():
    async with httpcore.AsyncConnectionPool() as client, anyio.create_task_group() as tg:
        for _ in range(3):
            tg.start_soon(worker, client, nurse)


if __name__ == "__main__":
    anyio.run(main)

Before...

$ python3 -m venv venv
$ venv/bin/pip install httpcore
$ venv/bin/python ./example.py 
<Response [301]>
Traceback (most recent call last):
  File "/Users/tomchristie/Temp/./example.py", line 18, in <module>
    anyio.run(main)
  File "/Users/tomchristie/Temp/venv/lib/python3.9/site-packages/anyio/_core/_eventloop.py", line 70, in run
    return asynclib.run(func, *args, **backend_options)
  File "/Users/tomchristie/Temp/venv/lib/python3.9/site-packages/anyio/_backends/_asyncio.py", line 292, in run
    return native_run(wrapper(), debug=debug)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/base_events.py", line 642, in run_until_complete
    return future.result()
  File "/Users/tomchristie/Temp/venv/lib/python3.9/site-packages/anyio/_backends/_asyncio.py", line 287, in wrapper
    return await func(*args)
  File "/Users/tomchristie/Temp/./example.py", line 14, in main
    nurse.start_soon(worker, client, nurse)
  File "/Users/tomchristie/Temp/venv/lib/python3.9/site-packages/httpcore/_async/connection_pool.py", line 326, in __aexit__
    await self.aclose()
  File "/Users/tomchristie/Temp/venv/lib/python3.9/site-packages/httpcore/_async/connection_pool.py", line 312, in aclose
    raise RuntimeError(
RuntimeError: The connection pool was closed while 2 HTTP requests/responses were still in-flight.

After...

$ python3 -m venv venv
$ venv/bin/pip install git+https://github.com/encode/httpcore.git@fix-async-cancellation-behaviour
$ venv/bin/python ./example.py 
<Response [301]>

@tomchristie
Copy link
Member Author

Using Python 3.11 TaskGroups, looks like the scheduling is a little different, and I can't reproduce the issue.

import asyncio
import httpcore


async def worker(client, tasks):
    r = await client.request('HEAD', 'https://google.com')
    print(r)
    [task.cancel() for task in tasks]


async def main():
    async with httpcore.AsyncConnectionPool() as client, asyncio.TaskGroup() as tg:
        tasks = []
        for _ in range(3):
            task = tg.create_task(worker(client, tasks))
            tasks.append(task)


if __name__ == "__main__":
    asyncio.run(main())

Both httpcore 0.15.0 and httpcore master result in the same...

$ venv/bin/python ./example.py 
<Response [301]>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug
2 participants