fix(simplismart): close the streams the STT collects - #7012
Merged
tinalenguyen merged 1 commit intoAug 31, 2026
Conversation
|
|
Contributor
Author
|
recheck |
Rehansanjay
force-pushed
the
fix/stt-aclose-tracked-streams
branch
from
August 27, 2026 17:35
9572437 to
6838386
Compare
Contributor
Author
|
Same class of leak as #7023, which was merged over the weekend: the sarvam and simplismart STTs append every stream to cc @davidzhao — same shape as #7023. @dhruvladia may want to sanity-check the sarvam side. |
Contributor
|
Thanks for the detailed issue @Rehansanjay |
`STT.stream()` gives every `SpeechStream` its own `aiohttp.ClientSession`
("a fresh session for this stream to avoid conflicts"), and only
`SpeechStream.aclose()` closes it. The STT collects its streams but
defines no `aclose()`, so `STT.aclose()` resolves to the base class
no-op: the streams are never closed, their sessions stay open, and
aiohttp reports unclosed client sessions once the streams are dropped.
The base contract asks for this — `STT.aclose` is documented as "Close
the STT, and every stream/requests associated with it", and
`__aexit__` delegates to it, so `async with stt:` leaks today.
Streams are now owned strongly rather than through a `WeakSet`: a stream
that finishes and is garbage collected would otherwise take its
still-open session with it, before `aclose()` ever sees it. Closed
streams discard themselves from the STT, so the set does not grow
without bound, and `aclose()` removes only what it actually closed so a
concurrently created stream stays tracked.
Regression tests are hermetic — `_run` is stubbed, no network. Three of
the four fail with the fix reverted, with aiohttp's "Unclosed client
session" appearing exactly as the leak predicts.
The sarvam half of this change is superseded by livekit#7058, which fixes the
identical bug in that plugin; this keeps the simplismart half, which
livekit#7058 does not cover. The strong-ownership and self-discard approach here
follows livekit#7058 so the two plugins stay consistent.
Rehansanjay
force-pushed
the
fix/stt-aclose-tracked-streams
branch
from
August 31, 2026 13:12
6838386 to
7e79bc1
Compare
Rehansanjay
changed the title
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+309
to
+320
| async def aclose(self) -> None: | ||
| """Close every stream this instance created. | ||
|
|
||
| ``stream()`` gives each ``SpeechStream`` its own ``aiohttp.ClientSession``, | ||
| and only ``SpeechStream.aclose()`` closes it. Streams are owned strongly | ||
| (see ``self._streams``) so a finished, garbage-collected stream cannot take | ||
| its open session with it; closed streams discard themselves, so the set | ||
| never grows without bound. | ||
| """ | ||
| closed = list(self._streams) | ||
| for stream in closed: | ||
| await stream.aclose() |
Contributor
There was a problem hiding this comment.
🔴 Sarvam stream sessions still leak
The cleanup omits STT.aclose for Sarvam's stream, which creates a client session per stream. Closing that recognizer leaves collected sessions open.
Prompt for agents
Implement the same ownership and lifecycle cleanup for livekit-plugins/livekit-plugins-sarvam/livekit/plugins/sarvam/stt.py. Its STT.stream creates a fresh aiohttp.ClientSession and records the SpeechStream in a WeakSet, but STT has no aclose override. Ensure STT.aclose closes every tracked stream, ensure individually closed streams stop being retained if strong ownership is used, and add Sarvam regression coverage equivalent to the new SimpliSmart tests.
Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The leak
STT.stream()gives everySpeechStreamits ownaiohttp.ClientSession("a fresh session for this stream to avoid conflicts"), and onlySpeechStream.aclose()closes it. TheSTTcollects its streams intoself._streams, but nothing ever reads that set and the class defines noaclose()— soSTT.aclose()resolves to the no-op on the base class. The streams are never closed, their sessions stay open, and aiohttp reports unclosed client sessions once the streams are dropped.The base contract asks for this:
STT.acloseis documented as "Close the STT, and every stream/requests associated with it", andSTT.__aexit__delegates to it, soasync with stt:currently leaks the streams.The change
An
aclose()onSTTthat closes what it collected, plus the ownership fix from #7058:self._streamsbecomes a strongsetinstead of aweakref.WeakSet. A stream that finishes and is garbage collected would otherwise take its still-open session with it, beforeaclose()could ever reach it — which is the case most likely to leak in practice.SpeechStream.aclose()discards itself from the STT once its session is actually closed, so the set does not grow without bound.aclose()removes only the streams it closed, so one created concurrently (or one whose cleanup was cancelled) stays tracked for a later close.Validation
New hermetic tests in
tests/test_plugin_simplismart_stt.py—_runis stubbed, no network:aclose()closes every per-stream session and empties the setasync withpath closes them via__aexit__aclose()tolerates an already-closed streamWeakSetcaseWith the fix reverted, three of the four fail and aiohttp's
Unclosed client sessionappears exactly as the leak predicts. With the fix, all four pass.ruff checkandruff format --checkpass on both changed files.