Skip to content

Response not closed when timeout/cancel reading response stream, which cause RuntimeError: The connection pool was closed while 1 HTTP requests/responses were still in-flight.  #2139

Closed

Description

@guyskk

Response not closed when timeout/cancel reading response stream, which cause RuntimeError: The connection pool was closed while 1 HTTP requests/responses were still in-flight.

Reproduce:

import asyncio
import httpx

async def main():
    url = "https://httpbin.org/drip?delay=0&duration=5"
    # Or use local httpbin server:
    #     docker run -ti -p 8088:80 kennethreitz/httpbin:latest
    url = "http://127.0.0.1:8088/drip?delay=0&duration=5"
    async with httpx.AsyncClient(timeout=10, trust_env=False) as client:
        try:
            coro = client.get(url)
            response = await asyncio.wait_for(coro, 3)
        except Exception as ex:
            print(type(ex), repr(ex))
        else:
            print(response)

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

Output:

<class 'asyncio.exceptions.TimeoutError'> TimeoutError()
Traceback (most recent call last):
  File "/Users/kk/dev/zhuwen/httpx/http_error2.py", line 22, in <module>
    asyncio.run(main())
  File "/Users/kk/.pyenv/versions/3.9.7/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/Users/kk/.pyenv/versions/3.9.7/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/kk/dev/zhuwen/httpx/http_error2.py", line 18, in main
    print(response)
  File "/Users/kk/dev/zhuwen/httpx/httpx/_client.py", line 1978, in __aexit__
    await self._transport.__aexit__(exc_type, exc_value, traceback)
  File "/Users/kk/dev/zhuwen/httpx/httpx/_transports/default.py", line 332, in __aexit__
    await self._pool.__aexit__(exc_type, exc_value, traceback)
  File "/Users/kk/.pyenv/versions/3.9.7/envs/httpx/lib/python3.9/site-packages/httpcore/_async/connection_pool.py", line 326, in __aexit__
    await self.aclose()
  File "/Users/kk/.pyenv/versions/3.9.7/envs/httpx/lib/python3.9/site-packages/httpcore/_async/connection_pool.py", line 312, in aclose
    raise RuntimeError(
RuntimeError: The connection pool was closed while 1 HTTP requests/responses were still in-flight.

Root cause:

It's because use except Exception in _client.py which will not catch asyncio CancelledError, so except branch will not executed. https://github.com/encode/httpx/blob/master/httpx/_client.py#L1604

        try:
            if not stream:
                await response.aread()  # will raise CancelledError

            return response

        except Exception as exc:  # pragma: no cover
            await response.aclose()  # will not executed
            raise exc

Change all except Exception in _client.py to except BaseException can resolve the issue.

Activity

  1. remi-dupre commented on Mar 22, 2022

    @remi-dupre

    Hi!

    I had just investigated an issue which is probably the same bug. I didn't get to your level of investigation / getting into the root cause, but it might still help others 🤷

    In my case the canceled tasks were holding slots in the connection pool. I'll also add that the requests that we try to cancel never complete : increasing the size of the connection pool just results in a memory leak and once it is full the client can never be used anymore for the whole runtime.

    This is a minimal example canceling a request and trying to send another right away

    import asyncio
    import httpx
    
    url = "http://perdu.com"
    client = httpx.AsyncClient(limits=httpx.Limits(max_connections=1))
    
    async def get():
        await client.get(url)
    
    async def get_cancel():
        task = asyncio.create_task(get())
        # the bug does not reproduce if we don't give time for the request to be sent
        await asyncio.sleep(0.001)
        task.cancel()
    
    async def main():
        await get_cancel()
        await asyncio.sleep(2) # the request should have finished
        await get() # !!! httpx.PoolTimeout !!!
    
    asyncio.run(main())

    EDIT: It might also help to note that the bug appears to have been introduced somewhere between version 0.18 and 0.21 since applying this upgrade appears to have triggered the bug for me.

  2. guyskk commented on Mar 23, 2022

    @guyskk
    ContributorAuthor

    @remi-dupre I can reproduce your case, but it seems multiple reason cause your issue.

    1. response not closed in _client.py because not catch BaseException: https://github.com/encode/httpx/blob/master/httpx/_client.py#L1604
    2. connection is in "connecting" state, so it is still not closed or expired in connection pool if response closed (the logic is ok here) : https://github.com/encode/httpcore/blob/master/httpcore/_async/connection_pool.py#L282
    3. when cancelled in connection acquire request_lock, it is not catched, so connection state incorrect: https://github.com/encode/httpcore/blob/master/httpcore/_async/connection.py#L60
    4. when cancelled in connection connect, connect_failed flag not set because not catch BaseException: https://github.com/encode/httpcore/blob/master/httpcore/_async/connection.py#L84
    5. I see many except Exception in httpcore which probably bugs too, which may cause other issues.

    We need help from httpx maintainer, it's a big problem.

  3. guyskk commented on Mar 28, 2022

    @guyskk
    ContributorAuthor

    Hi @florimondmanca, I submit a PR #2148 to resolve issue, could you help review?

    The PR not fix case of @remi-dupre, which seems another issue and need a bigger PR to fix.

  4. florimondmanca commented on Mar 28, 2022

    @florimondmanca
    Contributor

    Hi all,

    This seems like something that's affecting several people so let's catch on with this. FYI I am able to reproduce from the script provided in issue description - thanks so much for providing it.

    As we can see from the traceback, this actually seems to be an issue that should be resolved in https://github.com/encode/httpcore, which hosts the core networking / connection pool implementation, for multiple async libraries (asyncio, trio).

    I'd suggest as a next step we find a reproduction script that only uses HTTPCore, see async usage here: https://www.encode.io/httpcore/async/. Then we can pinpoint what needs fixing in HTTPCore, and after that if there's anything left to fix in HTTPX.

    EDIT: It might also help to note that the bug appears to have been introduced somewhere between version 0.18 and 0.21 since applying this upgrade appears to have triggered the bug for me.

    IIRC HTTPX 0.20 came with a complete overhaul of HTTPCore (version 0.14.0), which might explain this. We could verify this by bisecting on httpcore==0.14.0

  5. lovelydinosaur commented on Mar 28, 2022

    @lovelydinosaur
    Member

    I'd suggest as a next step we find a reproduction script that only uses HTTPCore, see async usage here: https://www.encode.io/httpcore/async/. Then we can pinpoint what needs fixing in HTTPCore, and after that if there's anything left to fix in HTTPX.

    Fantastic point @florimondmanca, yup.

    Following on from your suggestion, I took a look at that just now...

    import asyncio
    import httpcore
    
    async def main():
        url = "https://httpbin.org/drip?delay=0&duration=5"
        async with httpcore.AsyncConnectionPool() as pool:
            try:
                coro = pool.request("GET", url)
                response = await asyncio.wait_for(coro, 3)
            except Exception as ex:
                print(type(ex), repr(ex))
            else:
                print(response)
    
    if __name__ == "__main__":
        asyncio.run(main())

    Which results in...

    $ python ./example.py 
    <class 'asyncio.exceptions.TimeoutError'> TimeoutError()

    In constrast to running essentially the same thing with httpx...

    import asyncio
    import httpx
    
    async def main():
        url = "https://httpbin.org/drip?delay=0&duration=5"
        async with httpx.AsyncClient() as client:
            try:
                coro = client.request("GET", url)
                response = await asyncio.wait_for(coro, 3)
            except Exception as ex:
                print(type(ex), repr(ex))
            else:
                print(response)
    
    if __name__ == "__main__":
        asyncio.run(main())

    Which replicates the issue as described...

    $ python ./example.py 
    <class 'asyncio.exceptions.TimeoutError'> TimeoutError()
    Traceback (most recent call last):
      File "/Users/tomchristie/Temp/./example.py", line 34, in <module>
        asyncio.run(main())
      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/./example.py", line 31, in main
        print(response)
      File "/Users/tomchristie/Temp/venv/lib/python3.9/site-packages/httpx/_client.py", line 1977, in __aexit__
        await self._transport.__aexit__(exc_type, exc_value, traceback)
      File "/Users/tomchristie/Temp/venv/lib/python3.9/site-packages/httpx/_transports/default.py", line 332, in __aexit__
        await self._pool.__aexit__(exc_type, exc_value, traceback)
      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 1 HTTP requests/responses were still in-flight.

    I haven't properly understood for myself exactly why that happens, but I assume it's because in the second case the cancellation exception is occurring within httpx, not httpcore.

    I believe that we already have this fixed within httpcore, and that the proposed fix here is also necessary to ensure correct response closing at the httpx level too.

  6. florimondmanca commented on Mar 28, 2022

    @florimondmanca
    Contributor

    @tomchristie We're investigating this concurrently :-)

    I came to the same conclusion that the default usage with HTTPCore's .request() does not reproduce the error.

    This is because it is a shorthand that calls all of aread/aclose as necessary: https://github.com/encode/httpcore/blob/44b8eedfe3bc3c4b15d54810c36f05e7c5fd600d/httpcore/_async/interfaces.py#L17-L46

    This snippet is most relevant:

            response = await self.handle_async_request(request)
            try:
                await response.aread()
            finally:
                await response.aclose()

    So before we aclose() the pool when exiting the context manager, the response gets properly closed thanks to the finally: await response.aclose().

    On the other hand, HTTPX does not call .request() directly, instead it calls into .handle_async_request() and manages aread/aclose via the Response class, which does leave it in an "open" state when inside a simple timeout as we have here:

    httpx/httpx/_client.py

    Lines 1697 to 1698 in d07c4b4

    with request_context(request=request):
    response = await transport.handle_async_request(request)

    So the issue is indeed on the HTTPX side.

    Essentially, we need to move await response.aclose() into a finally block, which mimicks what HTTPCore does in its .request() helper method.

  7. florimondmanca commented on Mar 28, 2022

    @florimondmanca
    Contributor

    Actually, there might be some changes needed on the HTTPCore side too.

    I think about usages such as this one:

    https://github.com/encode/httpcore/blob/44b8eedfe3bc3c4b15d54810c36f05e7c5fd600d/httpcore/_async/connection.py#L84-L86

    Which might hide a similar bug at connect time (here we only reproduce against reads).

    I'm still not sure if we should prefer BaseException, enclosing both cancellations as well as SystemExit etc, or a narrower except (backend.cancellation_exc_class, Exception).

  8. lovelydinosaur commented on Mar 28, 2022

    @lovelydinosaur
    Member

    Actually, there might be some changes needed on the HTTPCore side too.

    Agreed. I can see there's a number of places where we clean up resources within an except ... close ... re-raise construct.

    I think they essentially all ought to be BaseException.

  9. rafaelfelix commented on Sep 8, 2022

    @rafaelfelix

    even though this issue is marked as closed, I still suffer from it. while trying to track it down, I noticed that the issue happens when asyncio.CancelledError is raised before the try/except statement here: https://github.com/encode/httpcore/blob/master/httpcore/_async/connection.py#L62-L86. On my local tests, I managed to fix this issue by surrounding the whole content under async with self._request_lock with the try block, catching BaseException instead of Exception. I can open a PR if anyone is interested, but I'll need help writing a proper test to it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions