Fix unclosed 'MemoryObjectReceiveStream' upon exception in 'BaseHTTPMiddleware' children #2813
Conversation
…iddleware' children Co-authored-by: Thomas Grainger <413772+graingert@users.noreply.github.com> Co-authored-by: Nikita Gashkov <8746283+nikitagashkov@users.noreply.github.com>
| async def close_recv_stream_on_response_sent() -> None: | ||
| await response_sent.wait() | ||
| recv_stream.close() | ||
|
|
There was a problem hiding this comment.
|
Well... I'm very confused as to why the current test is failing. 😆 |
|
hmm, 3.8 is EOL so probably getting an old anyio? edit 1: I ran the test suite locally and got the failure once, running the suite again 3 more times did not reproduce it edit 2: I deleted my pyc files, ran and it passed. I deleted my pyc files again and recreated my venv and it passed |
|
The flakiness worries me. Do you know why? Even if it's an old version of anyio. |
|
I'm going to try running it 1000x |
|
for future reference in case the CI logs expire, in python3.8 CI the failure is: =================================== FAILURES ===================================
_______________ test_websocket_endpoint_on_receive_text[asyncio] _______________
test_client_factory = functools.partial(<class 'starlette.testclient.TestClient'>, backend='asyncio', backend_options={})
def test_websocket_endpoint_on_receive_text(
test_client_factory: TestClientFactory,
) -> None:
class WebSocketApp(WebSocketEndpoint):
encoding = "text"
async def on_receive(self, websocket: WebSocket, data: str) -> None:
await websocket.send_text(f"Message text was: {data}")
client = test_client_factory(WebSocketApp)
with client.websocket_connect("/ws") as websocket:
websocket.send_text("Hello, world!")
_text = websocket.receive_text()
assert _text == "Message text was: Hello, world!"
with pytest.raises(RuntimeError):
with client.websocket_connect("/ws") as websocket:
> websocket.send_bytes(b"Hello world")
E Failed: DID NOT RAISE <class 'RuntimeError'>
tests/test_endpoints.py:134: Failed
================== 1 failed, 878 passed, 4 xfailed in 13.38s =================== |
|
ah I think it's a race condition here: starlette/starlette/testclient.py Lines 132 to 136 in 29b9c42 the other thread could be busy doing stuff moments before trying to put a message on the queue, so the queue appears empty when there's a message pending being put You need to use queue.shutdown() 3.13+ or send an explicit EOF object when the queue is done with |
|
I think this is a pre-existing issue, just VERY rare |
Co-authored-by: Thomas Grainger <tagrain@gmail.com>
Co-authored-by: Thomas Grainger <tagrain@gmail.com>
starlette/testclient.py
Outdated
| class _Eof(enum.Enum): | ||
| EOF = enum.auto() | ||
|
|
||
|
|
||
| EOF: typing.Final = _Eof.EOF | ||
| Eof = typing.Literal[_Eof.EOF] |
There was a problem hiding this comment.
The problem this solves is not related to this PR then?
There was a problem hiding this comment.
yes probably best to split into two PRs and add a changelog entry for the lost shutdown exception issue
|
omg it's still possible to fail!? |
|
Hmmm... Then it's not the queue shutdown thingy? |
|
rather than continue to hijack your PR I've opened a new one #2814 |
|
truly bizarre |
|
I'll make a release when I get home. |
I open a different PR to remove the
close_recv_stream_on_response_sent. I'm not sure that's why the test is failing...