From 111e923d50a5df322f031ef72a56163f6facceb7 Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Wed, 10 Jun 2026 11:08:15 +0200 Subject: [PATCH] fix(player): kill reused MPV/VLC processes on app quit (#1039) When the reuse-instance setting is enabled, the spawned MPV/VLC process is stored globally and kept alive (non-detached, piped stdio) so follow-up streams can be loaded into it. Nothing killed that process on app quit, so every app restart left an orphaned player running in the background. Register an explicit shutdown in the before-quit hook that kills the stored process and stops position polling, mirroring the existing embedded-MPV shutdown path. Co-authored-by: 4gray Co-authored-by: Claude Fable 5 --- .../app/events/mpv-session.service.spec.ts | 150 ++++++++++++++++++ .../src/app/events/mpv-session.service.ts | 28 +++- .../src/app/events/vlc-session.service.ts | 28 +++- apps/electron-backend/src/main.ts | 4 + 4 files changed, 198 insertions(+), 12 deletions(-) create mode 100644 apps/electron-backend/src/app/events/mpv-session.service.spec.ts diff --git a/apps/electron-backend/src/app/events/mpv-session.service.spec.ts b/apps/electron-backend/src/app/events/mpv-session.service.spec.ts new file mode 100644 index 000000000..4d4424e0e --- /dev/null +++ b/apps/electron-backend/src/app/events/mpv-session.service.spec.ts @@ -0,0 +1,150 @@ +jest.mock('electron', () => ({ + ipcMain: { + handle: jest.fn(), + }, +})); + +jest.mock('child_process', () => ({ + spawn: jest.fn(), +})); + +jest.mock('../app', () => ({ + __esModule: true, + default: { + mainWindow: null, + }, +})); + +jest.mock('../services/store.service', () => ({ + MPV_PLAYER_ARGUMENTS: 'MPV_PLAYER_ARGUMENTS', + MPV_PLAYER_PATH: 'MPV_PLAYER_PATH', + MPV_REUSE_INSTANCE: 'MPV_REUSE_INSTANCE', + VLC_PLAYER_ARGUMENTS: 'VLC_PLAYER_ARGUMENTS', + VLC_PLAYER_PATH: 'VLC_PLAYER_PATH', + VLC_REUSE_INSTANCE: 'VLC_REUSE_INSTANCE', + store: { + get: jest.fn(), + set: jest.fn(), + }, +})); + +import { spawn, type ChildProcess } from 'child_process'; +import { EventEmitter } from 'events'; +import { + MPV_PLAYER_PATH, + MPV_REUSE_INSTANCE, + VLC_PLAYER_PATH, + VLC_REUSE_INSTANCE, + store, +} from '../services/store.service'; +import { openMpvPlayer, shutdownMpvSession } from './mpv-session.service'; +import { openVlcPlayer, shutdownVlcSession } from './vlc-session.service'; + +function createMockChildProcess(): ChildProcess { + return Object.assign(new EventEmitter(), { + killed: false, + kill: jest.fn(() => true), + stderr: null, + stdout: null, + unref: jest.fn(), + }) as unknown as ChildProcess; +} + +async function waitForSpawnCallCount(count: number): Promise { + for (let attempt = 0; attempt < 20; attempt += 1) { + if ((spawn as unknown as jest.Mock).mock.calls.length >= count) { + return; + } + + await new Promise((resolve) => { + setImmediate(resolve); + }); + } + + throw new Error(`Expected ${count} player spawn calls`); +} + +function mockStoreValues(values: Record): void { + (store.get as unknown as jest.Mock).mockImplementation( + (key: string, fallback?: unknown) => + key in values ? values[key] : fallback + ); +} + +describe('external player shutdown on app quit', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('kills the stored reusable MPV process on shutdown', async () => { + const proc = createMockChildProcess(); + (spawn as unknown as jest.Mock).mockReturnValue(proc); + mockStoreValues({ + [MPV_PLAYER_PATH]: '/usr/bin/mpv', + [MPV_REUSE_INSTANCE]: true, + }); + + await openMpvPlayer({ + title: 'Reusable MPV stream', + url: 'https://example.com/live.m3u8', + }); + + expect(proc.kill).not.toHaveBeenCalled(); + + shutdownMpvSession(); + + expect(proc.kill).toHaveBeenCalledTimes(1); + + // The stored process reference is cleared, so a second shutdown + // must not attempt another kill. + shutdownMpvSession(); + expect(proc.kill).toHaveBeenCalledTimes(1); + }); + + it('does not track non-reusable MPV processes for shutdown', async () => { + const proc = createMockChildProcess(); + (spawn as unknown as jest.Mock).mockReturnValue(proc); + mockStoreValues({ + [MPV_PLAYER_PATH]: '/usr/bin/mpv', + [MPV_REUSE_INSTANCE]: false, + }); + + await openMpvPlayer({ + title: 'Detached MPV stream', + url: 'https://example.com/live.m3u8', + }); + + expect(proc.unref).toHaveBeenCalled(); + + shutdownMpvSession(); + + expect(proc.kill).not.toHaveBeenCalled(); + }); + + it('kills the stored reusable VLC process on shutdown', async () => { + const proc = createMockChildProcess(); + (spawn as unknown as jest.Mock).mockReturnValue(proc); + mockStoreValues({ + [VLC_PLAYER_PATH]: '/usr/bin/vlc', + [VLC_REUSE_INSTANCE]: true, + }); + + const openPromise = openVlcPlayer({ + title: 'Reusable VLC stream', + url: 'https://example.com/live.m3u8', + }); + + await waitForSpawnCallCount(1); + proc.emit('spawn'); + await openPromise; + + expect(proc.kill).not.toHaveBeenCalled(); + + shutdownVlcSession(); + + expect(proc.kill).toHaveBeenCalledTimes(1); + + shutdownVlcSession(); + expect(proc.kill).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/electron-backend/src/app/events/mpv-session.service.ts b/apps/electron-backend/src/app/events/mpv-session.service.ts index adc15746a..fb6052f45 100644 --- a/apps/electron-backend/src/app/events/mpv-session.service.ts +++ b/apps/electron-backend/src/app/events/mpv-session.service.ts @@ -181,19 +181,35 @@ function sendMpvCommand( }); } +function killStoredMpvProcess(reason: string): void { + if (!mpvProcess || mpvProcess.killed) { + return; + } + traceExternalPlayer(reason); + mpvProcess.kill(); + mpvProcess = null; + mpvSocketPath = null; + stopPositionPolling(); +} + export function setMpvReuseInstance(reuseInstance: boolean): void { traceExternalPlayer('set mpv reuse instance', { reuseInstance }); store.set(MPV_REUSE_INSTANCE, reuseInstance); - if (!reuseInstance && mpvProcess && !mpvProcess.killed) { - traceExternalPlayer('clean up mpv process after disabling reuse'); - mpvProcess.kill(); - mpvProcess = null; - mpvSocketPath = null; - stopPositionPolling(); + if (!reuseInstance) { + killStoredMpvProcess('clean up mpv process after disabling reuse'); } } +/** + * Kill the MPV instance kept alive for reuse. The reused process is spawned + * non-detached with piped stdio, so without an explicit kill it outlives the + * app and keeps playing after quit. + */ +export function shutdownMpvSession(): void { + killStoredMpvProcess('kill reused mpv process on app shutdown'); +} + export async function openMpvPlayer({ url, title, diff --git a/apps/electron-backend/src/app/events/vlc-session.service.ts b/apps/electron-backend/src/app/events/vlc-session.service.ts index 4d7f5bdcf..a8ad22e6a 100644 --- a/apps/electron-backend/src/app/events/vlc-session.service.ts +++ b/apps/electron-backend/src/app/events/vlc-session.service.ts @@ -270,19 +270,35 @@ function getFreePort(): Promise { }); } +function killStoredVlcProcess(reason: string): void { + if (!vlcProcess || vlcProcess.killed) { + return; + } + traceExternalPlayer(reason); + vlcProcess.kill(); + vlcProcess = null; + vlcRcPort = null; + stopVlcPositionPolling(); +} + export function setVlcReuseInstance(reuseInstance: boolean): void { traceExternalPlayer('set vlc reuse instance', { reuseInstance }); store.set(VLC_REUSE_INSTANCE, reuseInstance); - if (!reuseInstance && vlcProcess && !vlcProcess.killed) { - traceExternalPlayer('clean up vlc process after disabling reuse'); - vlcProcess.kill(); - vlcProcess = null; - vlcRcPort = null; - stopVlcPositionPolling(); + if (!reuseInstance) { + killStoredVlcProcess('clean up vlc process after disabling reuse'); } } +/** + * Kill the VLC instance kept alive for reuse. The reused process is spawned + * non-detached, so without an explicit kill it outlives the app and keeps + * playing after quit. + */ +export function shutdownVlcSession(): void { + killStoredVlcProcess('kill reused vlc process on app shutdown'); +} + export async function openVlcPlayer({ url, title, diff --git a/apps/electron-backend/src/main.ts b/apps/electron-backend/src/main.ts index 18fb7b51c..4d616cbde 100644 --- a/apps/electron-backend/src/main.ts +++ b/apps/electron-backend/src/main.ts @@ -13,7 +13,9 @@ import EmbeddedMpvEvents, { shutdownEmbeddedMpv, } from './app/events/embedded-mpv.events'; import EpgEvents from './app/events/epg.events'; +import { shutdownMpvSession } from './app/events/mpv-session.service'; import PlayerEvents from './app/events/player.events'; +import { shutdownVlcSession } from './app/events/vlc-session.service'; import PlaylistEvents from './app/events/playlist.events'; import RemoteControlEvents from './app/events/remote-control.events'; import SettingsEvents from './app/events/settings.events'; @@ -145,5 +147,7 @@ app.whenReady().then(async () => { app.on('before-quit', () => { shutdownEmbeddedMpv(); + shutdownMpvSession(); + shutdownVlcSession(); void databaseWorkerClient.shutdown(); });