mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-08 09:01:03 -08:00
fix(data-service): track and actually remove window message listeners
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) <noreply@anthropic.com>
Entire-Checkpoint: 3f453cc0085a
This commit is contained in:
1 parent
6303986c72
commit
2b3c65932d
2 files changed
+52
-17
No files matched your search
@@ -42,6 +42,7 @@ interface ErrorStatus {
|
||||
})
|
||||
export class ElectronService extends DataService {
|
||||
private eventListeners: { [key: string]: () => void } = {};
|
||||
private messageListeners = new Map<string, EventListener>();
|
||||
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 {
|
||||
|
||||
@@ -52,6 +52,7 @@ interface ErrorStatus {
|
||||
providedIn: 'root',
|
||||
})
|
||||
export class PwaService extends DataService {
|
||||
private messageListeners = new Map<string, EventListener>();
|
||||
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 {
|
||||
|
||||
Reference in new issue
Block a user