fix(sarvam, simplismart): close the streams the STT collects - #7012
Open
Rehansanjay wants to merge 1 commit into
Open
fix(sarvam, simplismart): close the streams the STT collects#7012Rehansanjay wants to merge 1 commit into
Rehansanjay wants to merge 1 commit into
Conversation
Author
|
recheck |
Both plugins give every stream its own aiohttp session:
# Create a fresh session for this stream to avoid conflicts
stream_session = aiohttp.ClientSession()
and only SpeechStream.aclose() ever closes it. Both also collect their
streams in a weakref.WeakSet, but neither STT class defines aclose(), so
STT.aclose() resolves to the abstract no-op on the base class and those
sessions are never closed. aiohttp then reports unclosed client sessions and
the sockets stay open until the objects are collected.
The WeakSet is the tell: in both files it is initialised and added to and then
never read again, which is the shape of cleanup that was intended and not
written. Every other plugin that keeps a _streams set reads it - cartesia,
palabra and telnyx close through it, rime does the same for TTS, and
sarvam's own stt_streaming.py already has exactly this method a few hundred
lines away.
The base contract asks for it too: STT.aclose is documented as "Close the STT,
and every stream/requests associated with it".
Found by reading the plugins rather than from a report, so there is no issue
to close.
Written with AI assistance (Claude Code); reviewed and verified before opening.
Rehansanjay
force-pushed
the
fix/stt-aclose-tracked-streams
branch
from
August 27, 2026 17:35
9572437 to
6838386
Compare
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
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.
Found by reading the plugins rather than from a report, so there is no issue to close.
The leak
Both
sarvamandsimplismartgive every stream its own aiohttp session:and only
SpeechStream.aclose()closes it (simplismartat line 428,sarvamat 1100).Both also collect their streams:
But neither
STTclass definesaclose(). Insarvam/stt.pythe onlyaclosein the file belongs toSpeechStream(line 1056, withclass SpeechStreamat 868);simplismartis the same shape. SoSTT.aclose()resolves to the abstract no-op on the base class, those streams are never closed, and their sessions stay open — aiohttp reports unclosed client sessions and the sockets survive until the objects happen to be collected.Why this reads as an omission rather than a decision
The
WeakSetis the tell. In both files it is initialised, added to, and then never read again — the shape of cleanup that was intended and not written. I checked every plugin that keeps a_streamsset, and these two are the only ones where nothing reads it.Everywhere else it is read:
cartesia,palabra,telnyx— close through it inacloserime(TTS) —for s in list(self._streams): await s.aclose()sarvam's ownstt_streaming.py— already has this exact method at line 466, a few hundred lines from the file this PR touchesupdate_optionsThe base contract asks for it as well —
STT.acloseis documented as "Close the STT, and every stream/requests associated with it".The change
Eleven lines in each plugin, mirroring
stt_streaming.py:Neither STT owns a session of its own to close —
sarvamresolves throughutils.http_context.http_session()— so the streams are the whole of it.Verification, and its limits
ruff checkandruff format --checkpass on both files at the repo'sline-length = 100/target-version = py310, and both compile.Being straight about what I did not do: I have not run the test suite locally, and there is no test in this PR. The correctness argument here is that
stream()creates a session, onlySpeechStream.aclose()closes it, and nothing was calling it — plus five existing implementations of the same method in this repo to match. If you would like a regression test, tell me where it belongs and I will add one.