fix: don't let kill() short-circuit stop() - #5033
Merged
connortechnology merged 1 commit intoAug 3, 2026
Merged
Conversation
kill() cleared this.started before calling this.stop(), but stop() returns early when !started. For the zms path that meant clearInterval() on statusCmdTimer and streamCmdTimer never ran, activePlayer was never reset and mediaStream/audioTrack/videoTrack were never released. Every kill() leaked a pair of intervals, which adds up over a montage or watch page that cycles monitors every few seconds. Keep started set until stop() has done its work, and pass skipStreamCommand so stop() doesn't follow CMD_QUIT with a CMD_STOP against a socket zms is already tearing down. Clear connkey afterwards. stop() already sets started=false and activePlayer='' at the end, so kill() doesn't need to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 3, 2026
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.
Split out of the investigation in #5029 — independent of the
mode=singlequestion there,.kill()clearedthis.startedbefore callingthis.stop(), butstop()returns early when!started:For the zms path that meant the
clearInterval()calls forstatusCmdTimerandstreamCmdTimernever ran,activePlayerwas never reset, andmediaStream/audioTrack/videoTrackwere never released. Everykill()leaked a pair of live intervals, which adds up on a montage or Watch page cycling monitors every few seconds.This keeps
startedset untilstop()has done its work, and passesskipStreamCommandsostop()doesn't followCMD_QUITwith aCMD_STOPaimed at a socket zms is already tearing down. The connkey is cleared afterwards instead of before.stop()already setsstarted = falseandactivePlayer = ''at the end, sokill()no longer needs to.The new parameter is optional and every existing caller invokes
stop()with no arguments, so behaviour is unchanged for them.Testing:
npx eslint web/js/MonitorStream.jsclean. Not exercised against a live stream — worth a sanity check on a montage page.