Skip to content

Fix orphaned zms processes accumulating on Montage page tab hide/show #4706

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Simpler1 wants to merge 2 commits into
base: master
Choose a base branch
from
+9 −3
Open

Fix orphaned zms processes accumulating on Montage page tab hide/show #4706

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44849e0
Fix orphaned zms processes accumulating on Montage page tab hide/show
Simpler1 Mar 12, 2026
6a6daa7
Update watch.js and zone.js with same change from stop() to kill()
Simpler1 Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion web/skins/classic/views/js/montage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ document.onvisibilitychange = () => {
//closing should kill, hiding should stop/pause
for (let i = 0, length = monitors.length; i < length; i++) {
// Stop instead of pause because we don't want buffering in zms
monitors[i].stop();
monitors[i].kill();
Comment on lines 1046 to +1049
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline comment says “closing should kill, hiding should stop/pause”, but the hidden-tab timeout now calls kill() (not stop/pause). Please update the comment to match the current behavior (and rationale) so future changes don’t reintroduce the issue.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching to kill() here has the same cleanup concern as in other views: MonitorStream.kill() sets started=false before calling stop(), causing stop() to return early and skip clearing timers/resetting activePlayer. Because Montage restarts streams on visibility (via startVisibleMonitors()), this can leave stale state and interfere with a clean restart. Consider adjusting kill()/stop() so kill() both sends CMD_QUIT and still executes the full local teardown path.

Suggested change
monitors[i].kill();
monitors[i].stop();

Copilot uses AI. Check for mistakes.
}
}, 15*1000);
} else {
Expand Down
5 changes: 4 additions & 1 deletion web/skins/classic/views/js/watch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1402,7 +1402,10 @@ document.onvisibilitychange = () => {
prevStateStarted = 'played';
//Stop only if playing (not paused).
// We might want to continue status updates so that alarm sounds etc still happen
monitorStream.stop();
// Use kill() instead of stop() to send CMD_QUIT and terminate the zms
// process. stop() only sends CMD_STOP which leaves zms running, causing
// orphaned processes to accumulate each time the tab is hidden/shown.
Comment on lines +1405 to +1407
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using kill() here relies on MonitorStream.kill() performing a full teardown so monitorStream.start() works correctly when the tab becomes visible again. Currently kill() sets started=false before calling stop(), and stop() returns early when started is false, which skips parts of the normal cleanup (timers/activePlayer). Consider fixing kill()/stop() so the QUIT path still runs the complete local cleanup needed for reliable hide/show restart.

Suggested change
// Use kill() instead of stop() to send CMD_QUIT and terminate the zms
// process. stop() only sends CMD_STOP which leaves zms running, causing
// orphaned processes to accumulate each time the tab is hidden/shown.
// First run the normal stop() cleanup while started is true, then use kill()
// to send CMD_QUIT and terminate the zms process. stop() only sends CMD_STOP
// which leaves zms running, causing orphaned processes to accumulate each time
// the tab is hidden/shown if we don't also call kill().
monitorStream.stop();

Copilot uses AI. Check for mistakes.
monitorStream.kill();
Copy link
Copy Markdown
Member

@connortechnology connortechnology Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You didn't read my comment above. hide/show shouldn't kill zms. If you kill it then everything stops. No audio alerts, etc. We just want to stop/pause video streaming.

Alternatively we could switch to monitor status polling instead of working through zms..

}
} else {
prevStateStarted = 'stopped';
Expand Down
5 changes: 4 additions & 1 deletion web/skins/classic/views/js/zone.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,10 @@ document.onvisibilitychange = () => {
TimerHideShow = setTimeout(function() {
//Stop monitors when closing or hiding page
for (let i = 0, length = monitorData.length; i < length; i++) {
monitors[i].stop();
// Use kill() instead of stop() to send CMD_QUIT and terminate the zms
// process. stop() only sends CMD_STOP which leaves zms running, causing
// orphaned processes to accumulate each time the tab is hidden/shown.
Comment on lines +796 to +798
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This visibility handler now calls kill(), but MonitorStream.kill() currently sets started=false before calling stop(), which makes stop() return early and skip its normal cleanup (e.g., clearing intervals/resetting activePlayer). Since this code relies on restarting monitors after the tab becomes visible, consider updating kill()/stop() so kill() performs full local cleanup while still avoiding sending CMD_STOP after CMD_QUIT.

Suggested change
// Use kill() instead of stop() to send CMD_QUIT and terminate the zms
// process. stop() only sends CMD_STOP which leaves zms running, causing
// orphaned processes to accumulate each time the tab is hidden/shown.
// First perform a normal stop to run local cleanup (clear intervals,
// reset activePlayer, etc.) and send CMD_STOP, then call kill() to
// send CMD_QUIT and terminate the zms process. This avoids sending
// CMD_STOP after CMD_QUIT while still ensuring full cleanup.
if (monitors[i].started) {
monitors[i].stop();
}

Copilot uses AI. Check for mistakes.
monitors[i].kill();
}
}, 15*1000);
} else {
Expand Down
Loading