Reclaim connections orphaned by a cancelled request

[wwmanidumaneesha]

Reclaim connections orphaned by a cancelled request

Closes #1093. Also covers the earlier reports in #658 and #830, which were closed but describe the same failure.

The problem

A request cancelled after the pool has assigned it a newly created connection, but before that connection has been established, leaves the connection behind in the pool.

That connection is then stuck:

• It cannot be reaped. With self._connection is None and _connect_failed still False, is_closed(), has_expired() and is_idle() all return False, so no branch of the cleanup loop matches it.
• It cannot be reused. is_available() returns False for HTTP/1.1 while connecting.

So it holds a pool slot that is never reclaimed. Once max_connections of them accumulate, the pool is permanently full and every subsequent request fails with PoolTimeout against a perfectly healthy server. Only recreating the pool recovers.

The pool's own repr shows the inconsistency clearly — no requests, yet a connection still occupying the only slot:

<AsyncConnectionPool [Requests: 0 active, 0 queued | Connections: 1 active, 0 idle]>
pool.connections == [<AsyncHTTPConnection [CONNECTING]>]

Scope

Worth noting for reviewers: this needs cancellation delivered while the task is suspended in wait_for_connection(), which is what asyncio.Task.cancel() and asyncio.wait_for() do.

Scope-based cancellation (anyio.move_on_after, trio) is not affected — it is delivered at the next checkpoint, which lands inside AsyncHTTPConnection.handle_async_request(), where the existing except BaseException sets _connect_failed = True and the connection is correctly reaped. I probed that path across several pool sizes and timings and saw no leak.

The fix

Rather than patching the single cancellation path, this restores an invariant the pool already relies on. As the comment on AsyncHTTP11Connection.is_available() puts it, a connection in this state "will not be acquired from the connection pool for any other request" — it belongs to exactly one request.

So: a connection that is neither available nor idle is reserved for the one request it was assigned to. If no request references it any more, nothing can ever drive it forward, and it is closed and removed. This is self-healing regardless of how the connection came to be orphaned.

Tests

test_connection_pool_does_not_leak_slot_on_cancelled_request asserts both that no connection is left behind and that the pool still works afterwards.

Verified to fail on master and pass with the fix, 8 runs each way.

[mbeijen]

Hi, unfortunately, apparently httpx does no longer get new releases. Please note how these older PRs fix the same issue and have been open for a long time.

• Fix ConnectionPool state after async cancellation #986
• Fix 2 bugs and improve performance from O(N²+NxM) to O(N+M) in _assign_requests_to_connections #1035

Pydantic decided to fork this package to httpx2, because of accruing issues like this, and I fixed this very issue there --> pydantic/httpx2#983

[avyukd]

We hit this in production and reproduced it independently. Your rule holds for a second shape of the same defect, which the current test does not cover.

If the cancellation arrives after the socket is established but before AsyncHTTP11Connection marks the connection ACTIVE, the orphan is an HTTP/1.1 NEW entry holding an open socket, rather than a CONNECTING entry with no socket. The delivery point is the _state_lock acquire at _async/http11.py:72, which is outside that method's try at line 80 and outside AsyncHTTPConnection.handle_async_request's try at _async/connection.py:75. Neither _response_closed() nor _connect_failed = True runs, so the entry reports closed=False expired=False idle=False available=False exactly like the CONNECTING one. It holds the pool slot permanently, and it also holds the file descriptor.

Reproduction against master (10a6582):

import asyncio, anyio, httpcore

RESP = [b"HTTP/1.1 200 OK\r\n", b"Content-Length: 2\r\n", b"\r\n", b"{}"]

class ParkedStream(httpcore.AsyncNetworkStream):
    async def write(self, buffer, timeout=None): await anyio.sleep(999)
    async def aclose(self): pass

class OneParkedBackend(httpcore.AsyncMockBackend):
    def __init__(self, buf): super().__init__(list(buf)); self.connects = 0
    async def connect_tcp(self, *a, **k):
        self.connects += 1
        return ParkedStream() if self.connects == 1 else await super().connect_tcp(*a, **k)

async def settle(pred, steps=200):
    for _ in range(steps):
        if pred(): return True
        await asyncio.sleep(0)
    return pred()

async def main():
    backend = OneParkedBackend(RESP)
    async with httpcore.AsyncConnectionPool(max_connections=1, network_backend=backend) as pool:
        first = asyncio.ensure_future(pool.request("GET", "http://example.com/"))
        await settle(lambda: len(pool._connections) == 1)
        second = asyncio.ensure_future(pool.request("GET", "http://example.com/"))
        await settle(lambda: any(r.is_queued() for r in pool._requests))
        first.cancel(); second.cancel()
        await asyncio.gather(first, second, return_exceptions=True)

        print(repr(pool))
        for c in pool.connections:
            print(f"  {c!r}")
            print(f"    socket={'OPEN' if c._connection else 'none'} "
                  f"connect_failed={c._connect_failed} closed={c.is_closed()} "
                  f"expired={c.has_expired()} idle={c.is_idle()} available={c.is_available()}")
        try:
            await pool.request("GET", "http://example.com/", extensions={"timeout": {"pool": 0.2}})
            print("  follow-up: OK")
        except Exception as e:
            print(f"  follow-up: {type(e).__name__}")
        print(f"  sockets opened: {backend.connects}")

asyncio.run(main())

On master:

<AsyncConnectionPool [Requests: 0 active, 0 queued | Connections: 1 active, 0 idle]>
  <AsyncHTTPConnection ['http://example.com:80', HTTP/1.1, NEW, Request Count: 0]>
    socket=OPEN connect_failed=False closed=False expired=False idle=False available=False
  follow-up: PoolTimeout
  sockets opened: 1

With this PR applied, the pool is empty and the follow-up succeeds. So the patch already handles it and needs no change. A second test case for this shape may be worth adding, since it reaches the abandoned state at a different point in the connection lifecycle than the one the current test drives.