From 2b3c65932d39dc1bdc53146c5795c6f9458d171d Mon Sep 17 00:00:00 2001 From: 4gray Date: Sat, 2 May 2026 00:29:49 +0200 Subject: [PATCH] fix(data-service): track and actually remove window message listeners MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both ElectronService.listenOn() and PwaService.listenOn() called window.addEventListener('message', callback) but neither could remove the listener afterwards: - ElectronService.removeAllListeners() called a placeholder getListenerForCommand() that returned a fresh () => undefined function on every invocation — never matching the registered listener (with a comment confessing as much). - PwaService.removeAllListeners() was a literal `// not implemented` no-op. The result was a latent memory leak: every listenOn() call accumulated a global window listener that nothing could remove. Calling listenOn() twice for the same command also stacked duplicate listeners. Track callbacks in a messageListeners Map keyed by command name. On listenOn(), drop any existing listener for that command before adding the new one. On removeAllListeners(type), remove either the named listener or all of them (when type === 'all'). The fix lands proactively: grep finds no current callers in the renderer, so today's leak is theoretical. But the API is exposed via the abstract DataService and the placeholder comment explicitly invited the bug. Closing the footgun is cheaper than discovering it later. Inspired by matracey/iptvnator@df7e1dc — extends the fix to PwaService, which had the same bug in even more obvious form. Co-Authored-By: Claude Opus 4.7 (1M context) Entire-Checkpoint: 3f453cc0085a --- apps/web/src/app/services/electron.service.ts | 39 ++++++++++++------- apps/web/src/app/services/pwa.service.ts | 30 ++++++++++++-- 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/apps/web/src/app/services/electron.service.ts b/apps/web/src/app/services/electron.service.ts index d2af6bea4..5052a925c 100644 --- a/apps/web/src/app/services/electron.service.ts +++ b/apps/web/src/app/services/electron.service.ts @@ -42,6 +42,7 @@ interface ErrorStatus { }) export class ElectronService extends DataService { private eventListeners: { [key: string]: () => void } = {}; + private messageListeners = new Map(); private readonly snackBar = inject(MatSnackBar); private readonly store = inject(Store); private readonly translateService = inject(TranslateService); @@ -556,27 +557,39 @@ export class ElectronService extends DataService { unsubscribe() ); this.eventListeners = {}; - } else if (this.eventListeners[type]) { + // Remove all tracked window message listeners + this.messageListeners.forEach((listener) => + window.removeEventListener('message', listener) + ); + this.messageListeners.clear(); + return; + } + + if (this.eventListeners[type]) { // Unsubscribe from a specific event this.eventListeners[type](); delete this.eventListeners[type]; } - // Also remove any window message listeners - window.removeEventListener('message', this.getListenerForCommand(type)); - } - - private getListenerForCommand(_command: string): EventListener { - void _command; - // This is a placeholder. In a real implementation, you would need to - // store the actual listener functions to be able to remove them - return () => undefined; + // Remove the window message listener registered for this command + const messageListener = this.messageListeners.get(type); + if (messageListener) { + window.removeEventListener('message', messageListener); + this.messageListeners.delete(type); + } } listenOn(command: string, callback: (...args: unknown[]) => void): void { - // For Electron, use window message events - void command; - window.addEventListener('message', callback); + // Drop any existing listener for this command so calling listenOn() + // again rebinds rather than accumulating duplicates. + const existing = this.messageListeners.get(command); + if (existing) { + window.removeEventListener('message', existing); + } + + const listener = callback as EventListener; + window.addEventListener('message', listener); + this.messageListeners.set(command, listener); } getAppEnvironment(): string { diff --git a/apps/web/src/app/services/pwa.service.ts b/apps/web/src/app/services/pwa.service.ts index 3f7a26c4a..e82872587 100644 --- a/apps/web/src/app/services/pwa.service.ts +++ b/apps/web/src/app/services/pwa.service.ts @@ -52,6 +52,7 @@ interface ErrorStatus { providedIn: 'root', }) export class PwaService extends DataService { + private messageListeners = new Map(); private readonly http = inject(HttpClient); private readonly snackBar = inject(MatSnackBar); private readonly store = inject(Store); @@ -500,12 +501,33 @@ export class PwaService extends DataService { }); } - removeAllListeners(): void { - // not implemented + removeAllListeners(type: string): void { + if (type === 'all') { + this.messageListeners.forEach((listener) => + window.removeEventListener('message', listener) + ); + this.messageListeners.clear(); + return; + } + + const messageListener = this.messageListeners.get(type); + if (messageListener) { + window.removeEventListener('message', messageListener); + this.messageListeners.delete(type); + } } - listenOn(_command: string, callback: (...args: unknown[]) => void): void { - window.addEventListener('message', callback); + listenOn(command: string, callback: (...args: unknown[]) => void): void { + // Drop any existing listener for this command so calling listenOn() + // again rebinds rather than accumulating duplicates. + const existing = this.messageListeners.get(command); + if (existing) { + window.removeEventListener('message', existing); + } + + const listener = callback as EventListener; + window.addEventListener('message', listener); + this.messageListeners.set(command, listener); } getAppEnvironment(): string {