Cancellation during HTTP/1.1 response close can retain an ACTIVE connection and pool request

[d36choi]

Summary

In httpcore2 2.12.0, cancellation during HTTP/1.1 response cleanup can leave the connection pool in an inconsistent state:

• the pool request remains registered without a live caller;
• the established HTTP/1.1 connection remains ACTIVE; and
• retrying response.aclose() does not resume the interrupted cleanup.

The retained connection continues to count toward max_connections. With max_connections=1, the next request fails with PoolTimeout.

This appears distinct from [#982](#982) and [#983](#983). PR #983 handles cancellation before connection establishment, where an unreferenced NEW connection remains in the pool. This reproducer cancels cleanup after an HTTP/1.1 connection has been established and a response has been returned.

Reproducer

The following standalone script uses AsyncMockBackend, so it does not require a web framework, external server, or network access.

python -m pip install httpcore2==2.12.0
python repro_httpcore2_active_leak.py

"""Reproducer for cancellation during HTTP/1.1 response cleanup."""

import asyncio

import httpcore2


class BlockingAsyncLock:
    def __init__(self) -> None:
        self.entered = asyncio.Event()
        self.release = asyncio.Event()

    async def __aenter__(self) -> "BlockingAsyncLock":
        self.entered.set()
        await self.release.wait()
        return self

    async def __aexit__(self, exc_type, exc_value, traceback) -> None:
        return None


def make_request() -> httpcore2.Request:
    return httpcore2.Request(
        "GET",
        "http://example.com/",
        headers={"Host": "example.com"},
        extensions={"timeout": {"pool": 0.1}},
    )


async def main() -> None:
    assert httpcore2.__version__ == "2.12.0"

    backend = httpcore2.AsyncMockBackend(
        [
            b"HTTP/1.1 200 OK\r\n",
            b"Content-Length: 2\r\n",
            b"\r\n",
            b"{}",
        ]
    )

    async with httpcore2.AsyncConnectionPool(
        max_connections=1,
        network_backend=backend,
    ) as pool:
        response = await pool.handle_async_request(make_request())
        pool_connection = pool.connections[0]
        http11_connection = pool_connection._connection

        assert http11_connection is not None
        assert "HTTP/1.1, ACTIVE" in repr(pool_connection)

        # Deliver cancellation while _response_closed() is waiting to
        # acquire the HTTP/1.1 state lock.
        state_lock = BlockingAsyncLock()
        http11_connection._state_lock = state_lock

        close_task = asyncio.create_task(response.aclose())
        await asyncio.wait_for(state_lock.entered.wait(), timeout=1)
        close_task.cancel()

        try:
            await close_task
        except asyncio.CancelledError:
            pass

        print(f"pool after cancelled close: {pool!r}")
        print(f"connection:                 {pool_connection!r}")
        print(f"pool requests retained:    {len(pool._requests)}")
        print(f"pool stream closed:         {response.stream._closed}")

        assert response.stream._closed
        assert len(pool._requests) == 1
        assert pool._requests[0].connection is pool_connection
        assert pool.connections == [pool_connection]

        assert not pool_connection.is_available()
        assert not pool_connection.is_idle()
        assert not pool_connection.is_closed()
        assert not pool_connection.has_expired()
        assert "HTTP/1.1, ACTIVE" in repr(pool_connection)

        # Retrying close does not resume cleanup because PoolByteStream
        # has already marked itself closed.
        await response.aclose()

        assert len(pool._requests) == 1
        assert pool.connections == [pool_connection]

        try:
            await pool.handle_async_request(make_request())
        except httpcore2.PoolTimeout:
            print("follow-up request:          PoolTimeout (reproduced)")
        else:
            raise AssertionError("Expected the retained connection to exhaust the pool")


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

Actual output

pool after cancelled close: <AsyncConnectionPool [Requests: 1 active, 0 queued | Connections: 1 active, 0 idle]>
connection:                 <AsyncHTTPConnection ['http://example.com:80', HTTP/1.1, ACTIVE, Request Count: 1]>
pool requests retained:    1
pool stream closed:         True
follow-up request:          PoolTimeout (reproduced)

Expected behavior

After response cleanup completes or cancellation propagates:

1. the associated pool request should no longer be retained without a live response; and
2. the HTTP/1.1 connection should become either reusable or terminal:

ACTIVE -> IDLE    # response completed cleanly and can be reused
ACTIVE -> CLOSED  # response processing was interrupted

A cancelled response close should not leave an unusable ACTIVE connection occupying a pool slot indefinitely.

Observed control flow

The failure appears to occur in this sequence:

1. PoolByteStream.aclose() marks the outer pool stream as closed.
2. It awaits HTTP11ConnectionByteStream.aclose().
3. The inner stream also marks itself closed.
4. AsyncHTTP11Connection._response_closed() waits for _state_lock.
5. Cancellation interrupts the wait before the connection transitions to IDLE or CLOSED.
6. Control does not return to the remaining PoolByteStream.aclose() bookkeeping, so the pool request is not removed.
7. A subsequent response.aclose() is a no-op because the outer stream is already marked closed.

This leaves the following retained state:

pool request retained
    -> established HTTP/1.1 connection
    -> connected=True
    -> state=ACTIVE
    -> available=False
    -> idle=False
    -> closed=False
    -> expired=False

Difference from #982 / #983

PR #983 handles a connection cancelled before establishment:

request removed
connection establishment incomplete
connection.is_connected() == False
unreferenced NEW connection remains in pool

This reproducer reaches a later lifecycle stage before cancellation:

HTTP/1.1 connection established
connection.is_connected() == True
response returned to the caller
connection state == ACTIVE
response cleanup started

After cancellation, the connection is still referenced by the retained pool request. Therefore, the garbage-connection condition introduced by #983 does not remove it:

elif not (connection.is_connected() or reserved):
    continue

In this case:

connection.is_connected() == True
reserved == True

Requested behavior

Please make established HTTP/1.1 response cleanup cancellation-safe and ensure that pool bookkeeping completes even if underlying stream cleanup is cancelled.

Potentially relevant paths include:

• PoolByteStream.aclose()
• HTTP11ConnectionByteStream.aclose()
• AsyncHTTP11Connection._response_closed()
• AsyncHTTP11Connection.aclose()

The desired invariant is:

Once a response has ended or its cancellation has propagated, its pool request must not remain retained without a live response, and the associated connection must be reusable (IDLE) or terminal (CLOSED), never permanently ACTIVE.

I can prepare a PR with a regression test and proposed fix if this diagnosis and expected behavior look correct.

Environment

• Python: 3.11.4
• httpcore2: 2.12.0
• Protocol: HTTP/1.1
• Async runtime: asyncio
• Network backend: AsyncMockBackend
• External network access: none

[rksharma-owg]

This looks like a real lifecycle bug, not just a flaky pool symptom. The sequence in your reproducer makes sense: the outer pool stream gets marked closed, cancellation interrupts the HTTP/1.1 connection cleanup while it is waiting for the state lock, and the pool-request removal never runs. A second aclose() then cannot repair it because the outer stream already considers itself closed.

One thing I would check before preparing a PR is the exact current baseline. main now wraps the inner stream close in AsyncShieldCancellation(), so it is worth rerunning the reproducer against the latest 2.14.x/main and recording whether the leak is still present there.

If it still reproduces, a focused regression test around PoolByteStream.aclose() would be useful: cancel cleanup after the connection is established and assert that the pool request is removed and the connection ends up either IDLE or CLOSED, never permanently ACTIVE. That would complement the existing cancellation-before-establishment coverage.

[agammann]

Thanks for pointing me at the newer baseline. I reran the cancellation-after-establishment case against the published httpcore2==2.13.1 and main at fc32b34daa5e070917979ae36b8dd91913897379 (editable install). I made one change to the reproducer: after close_task.cancel(), I yielded one event-loop turn and then released the injected lock so cleanup could settle. Both runs ended with response.aclose() cancelled, one pool request retained, the established connection still HTTP/1.1, ACTIVE, and the next request with max_connections=1 raising PoolTimeout. Main wraps the inner close in AsyncShieldCancellation(), but this direct asyncio.Task.cancel() case still reproduces. I have not prepared a fix yet.

[dfbustosus]

I independently reproduced this against main at fc32b34daa5e070917979ae36b8dd91913897379. The diagnosis is correct.

There are two coupled failure points:

• HTTP11ConnectionByteStream.aclose() sets its _closed flag before awaiting _response_closed().
• PoolByteStream.aclose() does the same and removes the pool request only after the inner close returns.

AsyncShieldCancellation uses an AnyIO shielded cancel scope, but it does not prevent this direct asyncio.Task.cancel() path. Cancellation while HTTP/1.1 waits for the state lock therefore leaves both wrappers marked closed, skips pool bookkeeping, and makes a retry a no-op.

A pool-only fix is insufficient. If the request is merely removed, _assign_requests_to_connections() still retains the unreferenced connection because it is connected and remains ACTIVE; that follows the current retention logic.

The fix needs two guarantees:

1. If HTTP/1.1 _response_closed() is interrupted, force that connection into the terminal CLOSED state before propagating cancellation.
2. Put pool-request removal and reassignment in guaranteed cleanup around the inner close.

A local prototype with that ordering propagated CancelledError, left zero pool requests and zero retained connections, and allowed the next request with max_connections=1 to complete.

The regression test should use native asyncio.Task.cancel() after an established response, then assert that cancellation propagates, the old connection is closed and removed, the request list is empty, and a follow-up request does not raise PoolTimeout. This complements the existing AnyIO cancellation tests.