Skip to content

Fix async cancellation behaviour - #580

Merged
lovelydinosaur merged 3 commits into
masterfrom
fix-async-cancellation-behaviour
Nov 7, 2022
Merged

Fix async cancellation behaviour#580
lovelydinosaur merged 3 commits into
masterfrom
fix-async-cancellation-behaviour

Conversation

@lovelydinosaur

Copy link
Copy Markdown
Contributor

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.

@lovelydinosaurlovelydinosaur added the bug Something isn't working label Sep 29, 2022
@lovelydinosaurlovelydinosaur mentioned this pull request Oct 11, 2022
@Kludex
Kludex requested a review from graingertOctober 11, 2022 19:54
@Kludex

Copy link
Copy Markdown
Contributor

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

@lovelydinosaur

lovelydinosaur commented Nov 3, 2022

Copy link
Copy Markdown
ContributorAuthor

Within a clean directory...

example.py

importtrioimporthttpcoreasyncdefworker(client, nurse):
r=awaitclient.request('HEAD', 'https://google.com')
print(r)
nurse.cancel_scope.cancel()
asyncdefmain():
asyncwithhttpcore.AsyncConnectionPool() asclient, trio.open_nursery() asnurse:
for_inrange(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

importanyioimporthttpcoreasyncdefworker(client, nurse):
r=awaitclient.request('HEAD', 'https://google.com')
print(r)
nurse.cancel_scope.cancel()
asyncdefmain():
asyncwithhttpcore.AsyncConnectionPool() asclient, anyio.create_task_group() astg:
for_inrange(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
returnnative_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
returnfuture.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-mvenvvenv
$ venv/bin/pipinstallgit+https://github.com/encode/httpcore.git@fix-async-cancellation-behaviour
$ venv/bin/python ./example.py<Response [301]>

@lovelydinosaur

Copy link
Copy Markdown
ContributorAuthor

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

importasyncioimporthttpcoreasyncdefworker(client, tasks):
r=awaitclient.request('HEAD', 'https://google.com')
print(r)
[task.cancel() fortaskintasks]
asyncdefmain():
asyncwithhttpcore.AsyncConnectionPool() asclient, asyncio.TaskGroup() astg:
tasks= []
for_inrange(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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugSomething isn't working

Development

Successfully merging this pull request may close these issues.

Bug

2 participants

@lovelydinosaur@Kludex