Add automatic cleanup on Singleton removal

- simplify WindowsManager

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/common/utils/singleton.ts

@@ -19,12 +19,16 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
+import { disposer } from "./disposer";
+
 type StaticThis<T, R extends any[]> = { new(...args: R): T };
 
 export class Singleton {
   private static instances = new WeakMap<object, Singleton>();
   private static creating = "";
 
+  protected disposers = disposer();
+
   constructor() {
     if (Singleton.creating.length === 0) {
       throw new TypeError("A singleton class must be created by createInstance()");
@@ -43,7 +47,7 @@ export class Singleton {
    * @param args The constructor arguments for the child class
    * @returns An instance of the child class
    */
-  static createInstance<T, R extends any[]>(this: StaticThis<T, R>, ...args: R): T {
+  static createInstance<T extends Singleton, R extends any[]>(this: StaticThis<T, R>, ...args: R): T {
     if (!Singleton.instances.has(this)) {
       if (Singleton.creating.length > 0) {
         throw new TypeError("Cannot create a second singleton while creating a first");
@@ -64,7 +68,7 @@ export class Singleton {
    * Default: `true`
    * @returns An instance of the child class
    */
-  static getInstance<T, R extends any[]>(this: StaticThis<T, R>, strict = true): T | undefined {
+  static getInstance<T extends Singleton, R extends any[]>(this: StaticThis<T, R>, strict = true): T | undefined {
     if (!Singleton.instances.has(this) && strict) {
       throw new TypeError(`instance of ${this.name} is not created`);
     }
@@ -80,6 +84,7 @@ export class Singleton {
    * There is *no* way in JS or TS to prevent globals like that.
    */
   static resetInstance() {
+    Singleton.instances.get(this)?.disposers();
     Singleton.instances.delete(this);
   }
 }

src/main/catalog-sources/kubeconfig-sync.ts

@@ -42,8 +42,6 @@ const logPrefix = "[KUBECONFIG-SYNC]:";
 export class KubeconfigSyncManager extends Singleton {
   protected sources = observable.map<string, [IComputedValue<CatalogEntity[]>, Disposer]>();
   protected syncing = false;
-  protected syncListDisposer?: Disposer;
-
   protected static readonly syncName = "lens:kube-sync";
 
   constructor() {
@@ -76,28 +74,26 @@ export class KubeconfigSyncManager extends Singleton {
       this.startNewSync(filePath);
     }
 
-    this.syncListDisposer = observe(UserStore.getInstance().syncKubeconfigEntries, change => {
-      switch (change.type) {
-        case "add":
-          this.startNewSync(change.name);
-          break;
-        case "delete":
-          this.stopOldSync(change.name);
-          break;
+    this.disposers.push(
+      observe(UserStore.getInstance().syncKubeconfigEntries, change => {
+        switch (change.type) {
+          case "add":
+            this.startNewSync(change.name);
+            break;
+          case "delete":
+            this.stopOldSync(change.name);
+            break;
+        }
+      }),
+      () => {
+        for (const filePath of this.sources.keys()) {
+          this.stopOldSync(filePath);
+        }
+
+        catalogEntityRegistry.removeSource(KubeconfigSyncManager.syncName);
+        this.syncing = false;
       }
-    });
-  }
-
-  @action
-  stopSync() {
-    this.syncListDisposer?.();
-
-    for (const filePath of this.sources.keys()) {
-      this.stopOldSync(filePath);
-    }
-
-    catalogEntityRegistry.removeSource(KubeconfigSyncManager.syncName);
-    this.syncing = false;
+    );
   }
 
   @action

src/main/cluster-manager.ts

@@ -27,9 +27,9 @@ import { ClusterStore, getClusterIdFromHost } from "../common/cluster-store";
 import type { Cluster } from "./cluster";
 import logger from "./logger";
 import { apiKubePrefix } from "../common/vars";
-import { Singleton } from "../common/utils";
+import { Singleton, toJS } from "../common/utils";
 import { catalogEntityRegistry } from "./catalog";
-import { KubernetesCluster, KubernetesClusterPrometheusMetrics } from "../common/catalog-entities/kubernetes-cluster";
+import { KubernetesCluster } from "../common/catalog-entities/kubernetes-cluster";
 
 export class ClusterManager extends Singleton {
   private store = ClusterStore.getInstance();
@@ -37,35 +37,37 @@ export class ClusterManager extends Singleton {
   constructor() {
     super();
     makeObservable(this);
-    this.bindEvents();
-  }
 
-  private bindEvents() {
-    // reacting to every cluster's state change and total amount of items
-    reaction(
-      () => this.store.clustersList.map(c => c.getState()),
-      () => this.updateCatalog(this.store.clustersList),
-      { fireImmediately: true, }
-    );
+    this.disposers.push(
+      reaction(
+        () => toJS(this.store.clustersList),
+        clusters => this.updateCatalog(clusters),
+        { fireImmediately: true },
+      ),
+      reaction(
+        () => catalogEntityRegistry.getItemsForApiKind<KubernetesCluster>("entity.k8slens.dev/v1alpha1", "KubernetesCluster"),
+        entities => this.syncClustersFromCatalog(entities)
+      ),
+      // auto-stop removed clusters
+      autorun(() => {
+        const removedClusters = Array.from(this.store.removedClusters.values());
 
-    reaction(() => catalogEntityRegistry.getItemsForApiKind<KubernetesCluster>("entity.k8slens.dev/v1alpha1", "KubernetesCluster"), (entities) => {
-      this.syncClustersFromCatalog(entities);
-    });
+        if (removedClusters.length > 0) {
+          const meta = removedClusters.map(cluster => cluster.getMeta());
 
-    // auto-stop removed clusters
-    autorun(() => {
-      const removedClusters = Array.from(this.store.removedClusters.values());
-
-      if (removedClusters.length > 0) {
-        const meta = removedClusters.map(cluster => cluster.getMeta());
-
-        logger.info(`[CLUSTER-MANAGER]: removing clusters`, meta);
-        removedClusters.forEach(cluster => cluster.disconnect());
-        this.store.removedClusters.clear();
+          logger.info(`[CLUSTER-MANAGER]: removing clusters`, meta);
+          removedClusters.forEach(cluster => cluster.disconnect());
+          this.store.removedClusters.clear();
+        }
+      }, {
+        delay: 250
+      }),
+      () => {
+        for (const cluster of this.store.clusters.values()) {
+          cluster.disconnect();
+        }
       }
-    }, {
-      delay: 250
-    });
+    );
 
     ipcMain.on("network:offline", this.onNetworkOffline);
     ipcMain.on("network:online", this.onNetworkOnline);
@@ -77,7 +79,7 @@ export class ClusterManager extends Singleton {
       const index = catalogEntityRegistry.items.findIndex((entity) => entity.metadata.uid === cluster.id);
 
       if (index !== -1) {
-        const entity = catalogEntityRegistry.items[index] as KubernetesCluster;
+        const entity = catalogEntityRegistry.items[index];
 
         entity.status.phase = cluster.disconnected ? "disconnected" : "connected";
         entity.status.active = !cluster.disconnected;
@@ -86,14 +88,12 @@ export class ClusterManager extends Singleton {
           entity.metadata.name = cluster.preferences.clusterName;
         }
 
-        entity.spec.metrics ||= { source: "local" };
+        entity.spec.metrics ??= { source: "local" };
 
         if (entity.spec.metrics.source === "local") {
-          const prometheus: KubernetesClusterPrometheusMetrics = entity.spec?.metrics?.prometheus || {};
-
-          prometheus.type = cluster.preferences.prometheusProvider?.type;
-          prometheus.address = cluster.preferences.prometheus;
-          entity.spec.metrics.prometheus = prometheus;
+          entity.spec.metrics.prometheus ??= {};
+          entity.spec.metrics.prometheus.type ??= cluster.preferences.prometheusProvider?.type;
+          entity.spec.metrics.prometheus.address = cluster.preferences.prometheus;
         }
 
         catalogEntityRegistry.items.splice(index, 1, entity);
@@ -146,12 +146,6 @@ export class ClusterManager extends Singleton {
     });
   };
 
-  stop() {
-    this.store.clusters.forEach((cluster: Cluster) => {
-      cluster.disconnect();
-    });
-  }
-
   getClusterForRequest(req: http.IncomingMessage): Cluster {
     let cluster: Cluster = null;
 

src/main/exit-app.ts

@@ -26,19 +26,11 @@ import { ClusterManager } from "./cluster-manager";
 import logger from "./logger";
 
 export function exitApp() {
-  console.log("before windowManager");
-  const windowManager = WindowManager.getInstance(false);
-
-  console.log("before clusterManager");
-  const clusterManager = ClusterManager.getInstance(false);
-
-  console.log("after clusterManager");
-
   appEventBus.emit({ name: "service", action: "close" });
-  windowManager?.hide();
-  clusterManager?.stop();
+
+  WindowManager.resetInstance();
+  ClusterManager.resetInstance();
   logger.info("SERVICE:QUIT");
-  setTimeout(() => {
-    app.exit();
-  }, 1000);
+
+  setTimeout(() => app.exit(), 1000);
 }

src/main/helm/__tests__/helm-service.test.ts

@@ -40,7 +40,7 @@ describe("Helm Service tests", () => {
           { name: "experiment", url: "experimenturl" },
         ];
       }),
-    });
+    } as any);
 
     const charts = await helmService.listCharts();
 
@@ -91,7 +91,7 @@ describe("Helm Service tests", () => {
           { name: "bitnami", url: "bitnamiurl" },
         ];
       }),
-    });
+    } as any);
 
     const charts = await helmService.listCharts();
 

src/main/index.ts

@@ -148,7 +148,6 @@ app.on("ready", async () => {
   const lensProxy = LensProxy.createInstance(handleWsUpgrade);
 
   ClusterManager.createInstance();
-  KubeconfigSyncManager.createInstance();
 
   try {
     logger.info("🔌 Starting LensProxy");
@@ -194,7 +193,7 @@ app.on("ready", async () => {
 
   ipcMain.on(IpcRendererNavigationEvents.LOADED, () => {
     cleanup.push(pushCatalogToRenderer(catalogEntityRegistry));
-    KubeconfigSyncManager.getInstance().startSync();
+    KubeconfigSyncManager.createInstance().startSync();
     startUpdateChecking();
     LensProtocolRouterMain.getInstance().rendererLoaded = true;
   });
@@ -252,9 +251,9 @@ app.on("will-quit", (event) => {
   // Quit app on Cmd+Q (MacOS)
   logger.info("APP:QUIT");
   appEventBus.emit({name: "app", action: "close"});
-  ClusterManager.getInstance(false)?.stop(); // close cluster connections
-  KubeconfigSyncManager.getInstance(false)?.stopSync();
-  cleanup();
+  WindowManager.resetInstance();
+  ClusterManager.resetInstance();
+  KubeconfigSyncManager.resetInstance();
 
   if (blockQuit) {
     event.preventDefault(); // prevent app's default shutdown (e.g. required for telemetry, etc.)

src/main/window-manager.ts

@@ -34,11 +34,14 @@ import logger from "./logger";
 import { productName } from "../common/vars";
 import { LensProxy } from "./proxy/lens-proxy";
 
+function isHideable(window: BrowserWindow | null): boolean {
+  return Boolean(window && !window.isDestroyed());
+}
+
 export class WindowManager extends Singleton {
-  protected mainWindow: BrowserWindow;
-  protected splashWindow: BrowserWindow;
+  protected mainWindow: BrowserWindow | null = null;
+  protected splashWindow: BrowserWindow | null = null;
   protected windowState: windowStateKeeper.State;
-  protected disposers: Record<string, Function> = {};
 
   @observable activeClusterId: ClusterId;
 
@@ -46,8 +49,9 @@ export class WindowManager extends Singleton {
     super();
     makeObservable(this);
     this.bindEvents();
-    this.initMenu();
-    this.initTray();
+    this.disposers.push(initMenu(this));
+    this.disposers.push(initTray(this));
+    this.disposers.push(() => this.destroy());
   }
 
   get mainUrl() {
@@ -131,14 +135,6 @@ export class WindowManager extends Singleton {
     }
   }
 
-  protected async initMenu() {
-    this.disposers.menuAutoUpdater = initMenu(this);
-  }
-
-  protected initTray() {
-    this.disposers.trayAutoUpdater = initTray(this);
-  }
-
   protected bindEvents() {
     // track visible cluster from ui
     subscribeToBroadcast(IpcRendererNavigationEvents.CLUSTER_VIEW_CURRENT_ID, (event, clusterId: ClusterId) => {
@@ -206,18 +202,19 @@ export class WindowManager extends Singleton {
   }
 
   hide() {
-    if (this.mainWindow && !this.mainWindow.isDestroyed()) this.mainWindow.hide();
-    if (this.splashWindow && !this.splashWindow.isDestroyed()) this.splashWindow.hide();
+    if (isHideable(this.mainWindow)) {
+      this.mainWindow.hide();
+    }
+
+    if (isHideable(this.splashWindow)) {
+      this.splashWindow.hide();
+    }
   }
 
-  destroy() {
+  private destroy() {
     this.mainWindow.destroy();
     this.splashWindow.destroy();
     this.mainWindow = null;
     this.splashWindow = null;
-    Object.entries(this.disposers).forEach(([name, dispose]) => {
-      dispose();
-      delete this.disposers[name];
-    });
   }
 }