Uh oh!
There was an error while loading. Please reload this page.
Fix leaked anyio streams in streamable_http - #1991
Conversation
aad9e69 to
aff1b13Compareaff1b13 to
42891f4Compare42891f4 to
de5d624Compare| finally: | ||
| await sse_stream_reader.aclose() |
There was a problem hiding this comment.
Why does the reader moves to the finally but not the writer?
There was a problem hiding this comment.
On L569 it gets stored
# Store writer reference so close_sse_stream() can close itself._sse_stream_writers[request_id] =sse_stream_writerShould I be closing it here too, or does it need to outlive this function?
| except Exception: | ||
| except Exception: # pragma: lax no cover | ||
| logger.exception("Error in standalone SSE response") | ||
| await self._clean_up_memory_streams(GET_STREAM_KEY) |
There was a problem hiding this comment.
This stays in the except block same as before. Should this happen in finally regardless of exception?
Uh oh!
There was an error while loading. Please reload this page.
acf0bbf to
b11e04fCompareHead branch was pushed to by a user without write access
aabmass
commented
Feb 9, 2026
Tests are fixed now |
| @pytest.fixture | ||
| def reset_sse_app_status(): | ||
| # Needed for tests with sse-starlette < 3 | ||
| # https://github.com/sysid/sse-starlette/issues/59 | ||
| # https://github.com/sysid/sse-starlette/blob/v3.2.0/README.md#testing | ||
| from sse_starlette.sse import AppStatus | ||
| AppStatus.should_exit_event = None # pyright: ignore[reportAttributeAccessIssue] | ||
| yield | ||
| AppStatus.should_exit_event = None # pyright: ignore[reportAttributeAccessIssue] |
There was a problem hiding this comment.
We can bump sse-starlette btw, can you do it, please?
There was a problem hiding this comment.
That works for me. Done and removed this fixture
Kludex
commented
Feb 11, 2026
Thanks @aabmass |
Motivation and Context
Fixes a resource leak. I was seeing this in tests I am writing for a different issue (#421 (comment)). Before:
How Has This Been Tested?
Added a new test as first commit in this PR which fails. It passes with the fix, and I was able to remove a bunch of
pragma: no coverstatements.Breaking Changes
No
Types of changes
Checklist
Additional context