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

Bug #564

Closed
AmericanY opened this issue Jul 2, 2022 · 8 comments · Fixed by #580 or #631
Closed

Bug #564

AmericanY opened this issue Jul 2, 2022 · 8 comments · Fixed by #580 or #631

Comments

@AmericanY
Copy link

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
Copy link
Author

@graingert
Copy link
Member

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.

@tomchristie
Copy link
Member

@graingert - Can you be more specific?

@graingert
Copy link
Member

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
Copy link
Member

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

@tomchristie
Copy link
Member

Reopened by #627.

@tomchristie tomchristie reopened this Nov 25, 2022
@Zac-HD
Copy link

Zac-HD commented Nov 25, 2022

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)

except BaseException as exc:
# If we timeout here, or if the task is cancelled, then make
# sure to remove the request from the queue before bubbling
# up the exception.
async with self._pool_lock:
self._requests.remove(status)
raise exc

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 python-trio/flake8-async#61 🙂

@tomchristie
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants