httpcore icon indicating copy to clipboard operation
httpcore copied to clipboard

Bug

Open AmericanY opened this issue 2 years ago • 5 comments

import trio
import httpx


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


async def main():
    async with httpx.AsyncClient(timeout=None) as client, trio.open_nursery() as nurse:
        for _ in range(3):
            nurse.start_soon(worker, client, nurse)


if __name__ == "__main__":
    trio.run(main)
Traceback (most recent call last):
  File "c:\Users\AmericaN\Desktop\Lab\test.py", line 17, in <module>
    trio.run(main)
  File "C:\Users\AmericaN\Desktop\Lab\MyEnv\lib\site-packages\trio\_core\_run.py", line 1946, in 
run
    raise runner.main_task_outcome.error
  File "c:\Users\AmericaN\Desktop\Lab\test.py", line 11, in main
    async with httpx.AsyncClient(timeout=None) as client, trio.open_nursery() as nurse:
  File "C:\Users\AmericaN\Desktop\Lab\MyEnv\lib\site-packages\httpx\_client.py", line 1997, in __aexit__
    await self._transport.__aexit__(exc_type, exc_value, traceback)
  File "C:\Users\AmericaN\Desktop\Lab\MyEnv\lib\site-packages\httpx\_transports\default.py", line 332, in __aexit__
    await self._pool.__aexit__(exc_type, exc_value, traceback)
  File "C:\Users\AmericaN\Desktop\Lab\MyEnv\lib\site-packages\httpcore\_async\connection_pool.py", line 326, in __aexit__
    await self.aclose()
  File "C:\Users\AmericaN\Desktop\Lab\MyEnv\lib\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.

AmericanY avatar Jul 02 '22 03:07 AmericanY

https://github.com/encode/httpcore/blob/master/httpcore/_async/connection_pool.py#L277

AmericanY avatar Jul 02 '22 03:07 AmericanY

This is a bug in httpcore where it tries to acquire a lock while in a cancelled scope. This should be refactored so no awaits happen inside the lock and so the lock can be removed.

graingert avatar Jul 12 '22 12:07 graingert

@graingert - Can you be more specific?

tomchristie avatar Jul 12 '22 13:07 tomchristie

This line raises a CancelledError when a request is cancelled, so the lock cannot be acquired and the connection cannot be released https://github.com/encode/httpcore/blob/master/httpcore/_async/connection_pool.py#L277

graingert avatar Jul 12 '22 13:07 graingert

@tomchristie this is a design flaw in httpcore and needs to be re-done with an assumption of level cancellation

graingert avatar Aug 14 '22 02:08 graingert

Reopened by #627.

tomchristie avatar Nov 25 '22 15:11 tomchristie

Looking into this, the problem is that except BaseException: means that you might be cancelled, so you can't checkpoint (await or async for or async with) unless in a shielded cancel-scope. (see: anyio docs)

https://github.com/encode/httpcore/blob/d3ca620e8ffd1e18fe2a1aa5369f2df8b4de1662/httpcore/_async/connection_pool.py#L228-L234

Since we presumably don't want to hang forever if we can't get the lock (?), I'd wrap this in with anyio.fail_after(delay, shield=True):, with delay set to e.g. 65, or None if hanging is better than raising an error.


This is tricky enough to get right consistently at scale that flake8-trio warns about it (error 102), and if you'd use it I'm open to adding anyio support https://github.com/Zac-HD/flake8-trio/issues/61 🙂

Zac-HD avatar Nov 25 '22 20:11 Zac-HD

Okay, so my assessment of this has changed...

Actually the simplest way to fix this error is simply to not raise a RuntimeError in this case. The pool becomes closed, the connections become closed, and the example given in the opening post works absolutely fine. Everything is okay.

We only get an error if we later attempt to read from a connection which has been closed.

tomchristie avatar Nov 30 '22 15:11 tomchristie