TestClient fails to close memory stream objects

[loichuder]

EDIT: My original post below was based on my shallow understanding of the problem. See the comments below by @ppena-LiveData and @markwaddle for better test examples.

---

An example is worth a thousand words:

from starlette.testclient import TestClient
from starlette.testclient import TestClient

import contextlib
from typing import AsyncIterator, TypedDict

import httpx
from starlette.applications import Starlette
from starlette.requests import Request
from starlette.responses import PlainTextResponse
from starlette.routing import Route


# From https://www.starlette.io/lifespan/#running-lifespan-in-tests
class State(TypedDict):
    http_client: httpx.AsyncClient


@contextlib.asynccontextmanager
async def lifespan(app: Starlette) -> AsyncIterator[State]:
    async with httpx.AsyncClient() as client:
        yield {"http_client": client}


async def homepage(request: Request) -> PlainTextResponse:
    client = request.state.http_client
    response = await client.get("https://www.example.com")
    return PlainTextResponse(response.text)


app = Starlette(lifespan=lifespan, routes=[Route("/", homepage)])

# Actual testing code
def test_app():
    with TestClient(app) as client:
        response = client.get("/")
        assert response.status_code == 200


def test_app2():
    with TestClient(app) as client:
        response = client.get("/")
        assert response.status_code == 200


def test_app3():
    with TestClient(app) as client:
        response = client.get("/")
        assert response.status_code == 200

When running this with pytest -W error, I get the following error:

E                   Traceback (most recent call last):
E                     File ".../anyio/streams/memory.py", line 177, in __del__
E                       warnings.warn(
E                   ResourceWarning: Unclosed <MemoryObjectReceiveStream>

Note that this warning appears with anyio 4.4.0. Also, the warning does not appear when running a single test (which is why multiple test_app... are needed)

My guess is that the MemoryObjectReceiveStreams created there are not properly closed ?

[ppena-LiveData]

This doesn't require multiple TestClients. Here's a very simple reproduction (in addition to doing a pip install fastapi pytest):

from fastapi import APIRouter, FastAPI
from fastapi.testclient import TestClient

app = FastAPI()
router = APIRouter()


@router.post('/api/test')
def route() -> str:
    return 'OK'


app.include_router(router)


def test_route():
    with TestClient(app) as client:
        client.get('/api/test')

For some reason, pytest -W error doesn't seem to be sufficient to reproduce, so I had to first set PYTHONWARNINGS=error as an environment variable, and then I could just run pytest. After the test passed message, I see these errors:

Exception ignored in: <function MemoryObjectSendStream.__del__ at 0x7fc34ae6fe20>
Traceback (most recent call last):
  File "/tmp/fastapi-testclient-issue/.venv/lib/python3.10/site-packages/anyio/streams/memory.py", line 307, in __del__
    warnings.warn(
ResourceWarning: Unclosed <MemoryObjectSendStream>
Exception ignored in: <function MemoryObjectReceiveStream.__del__ at 0x7fc34ae6f6d0>
Traceback (most recent call last):
  File "/tmp/fastapi-testclient-issue/.venv/lib/python3.10/site-packages/anyio/streams/memory.py", line 177, in __del__
    warnings.warn(
ResourceWarning: Unclosed <MemoryObjectReceiveStream>

[ppena-LiveData]

@loichuder, do you mind if we change the subject of this discussion to something simpler, like "TestClient fails to close memory stream objects"?

[loichuder]

Sure, no problem. Thanks for finding an easier way of reproducing this 🙂

[ppena-LiveData]

I don't seem to have permissions to change the title/subject. Can you do it?

[markwaddle]

a somewhat simpler test that often, but not always, reproduces this warning

from starlette.applications import Starlette
from starlette.testclient import TestClient


def test_close_warning():
    for _ in range(10):
        with TestClient(Starlette()) as client:
            client.get("/")

[markwaddle]

i'm fairly confident this is an issue. i've tracked it down to the TestClient.stream_receive not being closed. i noticed that in wait_shutdown, it uses with to close the TestClient.stream_send.

    async def wait_shutdown(self) -> None:
        async def receive() -> typing.Any:
            message = await self.stream_send.receive()
            if message is None:
                self.task.result()
            return message

        async with self.stream_send:
            await self.stream_receive.send({"type": "lifespan.shutdown"})
            message = await receive()
            assert message["type"] in (
                "lifespan.shutdown.complete",
                "lifespan.shutdown.failed",
            )
            if message["type"] == "lifespan.shutdown.failed":
                await receive()

i recommend this function be changed to the following, to close both streams:

    async def wait_shutdown(self) -> None:
        async def receive() -> typing.Any:
            message = await self.stream_send.receive()
            if message is None:
                self.task.result()
            return message

        async with self.stream_send, self.stream_receive:
            await self.stream_receive.send({"type": "lifespan.shutdown"})
            message = await receive()
            assert message["type"] in (
                "lifespan.shutdown.complete",
                "lifespan.shutdown.failed",
            )
            if message["type"] == "lifespan.shutdown.failed":
                await receive()

i've verified that this change fixes the issue.

[markwaddle]

i'm currently working around this issue by pinning anyio to 4.3.0

[ppena-LiveData]

Yup, changing async with self.stream_send: to async with self.stream_send, self.stream_receive: fixes my simple reproduction as well.

[loichuder]

Thanks ! 👍

[ppena-LiveData]

@loichuder, I don't think you should mark this discussion as answered, since the issue is with starlette's TestClient, not with any of our code. I don't think the discussion should be marked as answered until there's an associated GitHub Issue and/or PR created for this.

[loichuder]

You're right !

[ppena-LiveData]

Now that we have a fix for this, what is the procedure; can we submit a PR with that fix, or do we need an admin to approve something? The Reporting Bugs or Other Issues section fo contributing.md says, "We can then determine if the discussion needs to be escalated into an "Issue" or not, or if we'd consider a pull request." I'm not sure who we're supposed to ask...

[Kludex]

Yeah, PR is welcome. Please remove the anyio line on the requirements.txt.

[ppena-LiveData]

Unfortunately, testclient.py isn't the only thing with problems. I was able to fix a lot of the other problems by adding recv_stream.close() at the top of the except anyio.EndOfStream: block in starlette/middleware/base.py, but that's not sufficient. That same recv_stream is still failing to close in some situations, but Python's eventual deletion call makes it really hard to figure out what situation is causing the problem of recv_stream not closing.

FYI, to debug this, I modified anyio/streams/memory.py to add a traceback.print_stack() and also print id(self) in MemoryObjectReceiveStream.__post_init__() and also printed id(self) in MemoryObjectReceiveStream.__del__(), so that's how I know that the failing MemoryObjectReceiveStream is the recv_stream in starlette/middleware/base.py.

[Kludex]

@jhominal Do you have availability to help here? I don't know how to solve this.

[jhominal]

@Kludex yes I would be happy to help.

i actually had fallen on some issues of that kind while working on my on-hold PR about streaming AsgiClient, I think the issue is that when using async for the aclose method is only called when the iteration ends normally, I think httpx/httpcore needs to be a lot more rigorous about tracking all opened generators and closing them using finally blocks.

[jhominal]

Note that my comment above is after a cursory read of the issue, I have not taken the time to look into it rigorously.