Skip to content

Commit

Permalink
Merge pull request #7238 from Incanta/fix/fs-watch-memory-leak
Browse files Browse the repository at this point in the history
Fix fs.watch leak with a timestamp to prevent repetitive buildFromConfig
  • Loading branch information
mook-as authored Jul 25, 2024
2 parents e313649 + 3866dec commit e2706a5
Showing 1 changed file with 32 additions and 15 deletions.
47 changes: 32 additions & 15 deletions pkg/rancher-desktop/main/tray.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,11 @@ export class Tray {
private settings: Settings;
private currentNetworkStatus: networkStatus = networkStatus.CHECKING;
private static instance: Tray;
private abortController: AbortController | undefined;
private networkState: boolean | undefined;
private networkInterval: NodeJS.Timeout;
private runBuildFromConfigTimer: NodeJS.Timeout | null = null;
private kubeConfigWatchers: fs.FSWatcher[] = [];
private fsWatcherInterval: NodeJS.Timeout;

protected contextMenuItems: Electron.MenuItemConstructorOptions[] = [
{
Expand Down Expand Up @@ -130,18 +132,19 @@ export class Tray {
* triggered, close the watcher and restart after a duration (one second).
*/
private async watchForChanges() {
const abortController = new AbortController();
for (const watcher of this.kubeConfigWatchers) {
watcher.close();
}
this.kubeConfigWatchers = [];

this.abortController = abortController;
const paths = await kubeconfig.getKubeConfigPaths();
const options: fs.WatchOptions = {
persistent: false,
recursive: !this.isLinux(), // Recursive not implemented in Linux
encoding: 'utf-8',
signal: abortController.signal,
};

paths.map(filepath => fs.watch(filepath, options, async(eventType) => {
this.kubeConfigWatchers = paths.map(filepath => fs.watch(filepath, options, async(eventType) => {
if (eventType === 'rename') {
try {
await fs.promises.access(filepath);
Expand All @@ -151,10 +154,12 @@ export class Tray {
}
}

this.abortController?.abort();
this.buildFromConfig();

setTimeout(this.watchForChanges.bind(this), 1_000);
// This prevents calling buildFromConfig multiple times in quick succession
// while making sure that the last file change within the period is processed.
this.runBuildFromConfigTimer ||= setTimeout(() => {
this.runBuildFromConfigTimer = null;
this.buildFromConfig();
}, 1_000);
}));
}

Expand Down Expand Up @@ -183,16 +188,20 @@ export class Tray {
this.buildFromConfig();
this.watchForChanges();

// We reset the watchers on an interval in the event that `fs.watch` silently
// fails to keep watching. This original issue is documented at
// https://github.com/rancher-sandbox/rancher-desktop/pull/2038 and further discussed at
// https://github.com/rancher-sandbox/rancher-desktop/pull/7238#discussion_r1690128729
this.fsWatcherInterval = setInterval(() => this.watchForChanges(), 5 * 60_000);

mainEvents.on('backend-locked-update', this.backendStateEvent);
mainEvents.emit('backend-locked-check');
mainEvents.on('k8s-check-state', this.k8sStateChangedEvent);
mainEvents.on('settings-update', this.settingsUpdateEvent);

/**
* This triggers the CONNECTED_TO_INTERNET diagnostic at a set interval and
* updates the network status in the tray if there's a change in the network
* state.
*/
// This triggers the CONNECTED_TO_INTERNET diagnostic at a set interval and
// updates the network status in the tray if there's a change in the network
// state.
this.networkInterval = setInterval(async() => {
const networkDiagnostic = await mainEvents.invoke('diagnostics-trigger', 'CONNECTED_TO_INTERNET');

Expand Down Expand Up @@ -242,11 +251,19 @@ export class Tray {
*/
public hide() {
this.trayMenu.destroy();
this.abortController?.abort();
mainEvents.off('k8s-check-state', this.k8sStateChangedEvent);
mainEvents.off('settings-update', this.settingsUpdateEvent);
ipcMainProxy.removeListener('update-network-status', this.updateNetworkStatusEvent);
clearInterval(this.fsWatcherInterval);
clearInterval(this.networkInterval);
if (this.runBuildFromConfigTimer) {
clearTimeout(this.runBuildFromConfigTimer);
this.runBuildFromConfigTimer = null;
}
for (const watcher of this.kubeConfigWatchers) {
watcher.close();
}
this.kubeConfigWatchers = [];
}

/**
Expand Down

0 comments on commit e2706a5

Please sign in to comment.