diff --git a/src/common/cluster-frames.ts b/src/common/cluster-frames.ts index 07d1451e8f..5519c6ea76 100644 --- a/src/common/cluster-frames.ts +++ b/src/common/cluster-frames.ts @@ -19,6 +19,7 @@ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +import AwaitLock from "await-lock"; import { action, observable } from "mobx"; import type { ClusterId } from "./cluster-store"; import { iter, Singleton } from "./utils"; @@ -30,14 +31,26 @@ export interface ClusterFrameInfo { } export class ClusterFrames extends Singleton { - private mapping = observable.map(); + /** + * The current set of frame info for each cluster + */ + private frames = observable.map(); + + /** + * The current mapping of clusters to the window that hope to create an iframe + * + * Used to make sure that if two windows try and open the same cluster, the one + * locks and submits a claim first is the only one. + */ + private claims = observable.map(); + private claimsLock = new AwaitLock(); public getAllFrameInfo(): ClusterFrameInfo[] { - return [...this.mapping.values()]; + return [...this.frames.values()]; } public getClusterIdFromFrameInfo(query: ClusterFrameInfo): ClusterId | undefined { - for (const [clusterId, info] of this.mapping) { + for (const [clusterId, info] of this.frames) { if ( info.frameId === query.frameId && info.processId === query.processId @@ -50,27 +63,74 @@ export class ClusterFrames extends Singleton { return undefined; } + @action public set(clusterId: ClusterId, info: ClusterFrameInfo): void { - this.mapping.set(clusterId, info); + if (!this.claims.has(clusterId)) { + throw new Error("Cannot set a cluster's FrameInfo if no claim exists"); + } + + if (this.claims.get(clusterId) !== info.windowId) { + throw new Error("Cannot set a cluster's FrameInfo for a window that didn't previously claim the cluster"); + } + + this.frames.set(clusterId, info); + this.claims.delete(clusterId); + } + + /** + * Attempts to claim cluster for window. Will succeed if previously claimed by the same window + * @param clusterId The cluster to claim for a particular window + * @param windowId The ID of the window trying to claim it + * @returns `true` if that window now has a claim, otherwise `false` + */ + public async claimCluster(clusterId: ClusterId, windowId: number): Promise { + try { + await this.claimsLock.acquireAsync(); + + if (this.claims.get(clusterId) === windowId) { + return true; + } + + if (this.frames.get(clusterId)?.windowId === windowId) { + return true; + } + + if (this.claims.has(clusterId) || this.frames.has(clusterId)) { + return false; + } + + this.claims.set(clusterId, windowId); + + return true; + } finally { + this.claimsLock.release(); + } } public getFrameInfoByClusterId(clusterId: ClusterId): ClusterFrameInfo | undefined { - return this.mapping.get(clusterId); + return this.frames.get(clusterId); } public getFrameInfoByFrameId(frameId: number): ClusterFrameInfo | undefined { - return iter.find(this.mapping.values(), frameInfo => frameInfo.frameId === frameId); + return iter.find(this.frames.values(), frameInfo => frameInfo.frameId === frameId); } public clearInfoForCluster(clusterId: ClusterId): void { - this.mapping.delete(clusterId); + this.frames.delete(clusterId); + this.claims.delete(clusterId); } @action public clearInfoForWindow(windowId: number): void { - for (const [clusterId, frameInfo] of this.mapping) { + for (const [clusterId, frameInfo] of this.frames) { if (frameInfo.windowId === windowId) { - this.mapping.delete(clusterId); + this.frames.delete(clusterId); + } + } + + for (const [clusterId, windowIdClaim] of this.claims) { + if (windowIdClaim === windowId) { + this.claims.delete(clusterId); } } } diff --git a/src/common/cluster-ipc.ts b/src/common/cluster-ipc.ts index 18330db0b7..9cd4149e0b 100644 --- a/src/common/cluster-ipc.ts +++ b/src/common/cluster-ipc.ts @@ -22,6 +22,7 @@ export const navigateToClusterHandler = "navigate:to-cluster"; export const clusterActivateHandler = "cluster:activate"; export const clusterSetFrameIdHandler = "cluster:set-frame-id"; +export const claimClusterFrameHandler = "cluster:claim-frame"; export const clusterVisibilityHandler = "cluster:visibility"; export const clusterRefreshHandler = "cluster:refresh"; export const clusterDisconnectHandler = "cluster:disconnect"; diff --git a/src/main/initializers/ipc.ts b/src/main/initializers/ipc.ts index 119ee683af..1ff1b03944 100644 --- a/src/main/initializers/ipc.ts +++ b/src/main/initializers/ipc.ts @@ -24,7 +24,7 @@ import type { IpcMainInvokeEvent } from "electron"; import { when } from "mobx"; import { KubernetesCluster } from "../../common/catalog-entities"; import { ClusterFrames } from "../../common/cluster-frames"; -import { clusterActivateHandler, clusterSetFrameIdHandler, clusterVisibilityHandler, clusterRefreshHandler, clusterDisconnectHandler, clusterKubectlApplyAllHandler, clusterKubectlDeleteAllHandler, clusterDeleteHandler, navigateToClusterHandler } from "../../common/cluster-ipc"; +import { clusterActivateHandler, clusterSetFrameIdHandler, clusterVisibilityHandler, clusterRefreshHandler, clusterDisconnectHandler, clusterKubectlApplyAllHandler, clusterKubectlDeleteAllHandler, clusterDeleteHandler, navigateToClusterHandler, claimClusterFrameHandler } from "../../common/cluster-ipc"; import { ClusterId, ClusterStore } from "../../common/cluster-store"; import { appEventBus } from "../../common/event-bus"; import { ipcMainHandle } from "../../common/ipc"; @@ -146,6 +146,10 @@ export function initIpcMainHandlers() { } }); + ipcMainHandle(claimClusterFrameHandler, async (event, clusterId: ClusterId) => { + return ClusterFrames.getInstance().claimCluster(clusterId, event.sender.getProcessId()); + }); + const navigateLocks = new Map(); ipcMainHandle(navigateToClusterHandler, async (event, clusterId: ClusterId, newWindow?: boolean) => { @@ -159,7 +163,9 @@ export function initIpcMainHandlers() { const lock = getOrInsert(navigateLocks, clusterId, new AwaitLock()); try { + console.log("trying to acquire lock for", clusterId, "in", navigateToClusterHandler); await lock.acquireAsync(); + console.log("acquired lock for", clusterId, "in", navigateToClusterHandler); const specifics: NavigateFrameInfoSpecifier[] = [{ clusterId }]; if (newWindow) { diff --git a/src/renderer/components/cluster-manager/cluster-view.tsx b/src/renderer/components/cluster-manager/cluster-view.tsx index 6f4d22dbf1..6042db6f41 100644 --- a/src/renderer/components/cluster-manager/cluster-view.tsx +++ b/src/renderer/components/cluster-manager/cluster-view.tsx @@ -29,7 +29,7 @@ import { hasLoadedView, initView, refreshViews } from "./lens-views"; import type { Cluster } from "../../../main/cluster"; import { ClusterStore } from "../../../common/cluster-store"; import { requestMain } from "../../../common/ipc"; -import { clusterActivateHandler } from "../../../common/cluster-ipc"; +import { clusterActivateHandler, navigateToClusterHandler } from "../../../common/cluster-ipc"; import { catalogEntityRegistry } from "../../api/catalog-entity-registry"; import { navigate } from "../../navigation"; import { catalogURL, ClusterViewRouteParams } from "../../../common/routes"; @@ -65,10 +65,27 @@ export class ClusterView extends React.Component { componentDidMount() { disposeOnUnmount(this, [ reaction(() => this.clusterId, async (clusterId) => { - refreshViews(clusterId); // refresh visibility of active cluster - initView(clusterId); // init cluster-view (iframe), requires parent container #lens-views to be in DOM - requestMain(clusterActivateHandler, clusterId, false); // activate and fetch cluster's state from main - catalogEntityRegistry.activeEntity = catalogEntityRegistry.getById(clusterId); + // refresh visibility of active cluster + refreshViews(clusterId); + + // activate and fetch cluster's state from main, don't force. Do this before starting to init + requestMain(clusterActivateHandler, clusterId, false) + .catch(error => console.warn("[CLUSTER-VIEW]: failed to activate cluster", error)); + + console.log("start initView", { clusterId }); + + if (await initView(clusterId)) { + console.log("success initView", { clusterId }); + // init cluster-view (iframe), requires parent container #lens-views to be in DOM + catalogEntityRegistry.activeEntity = catalogEntityRegistry.getById(clusterId); + } else { + console.log("initView: need to navigate", { clusterId }); + // if it fails then navigate to the new window + await requestMain( + navigateToClusterHandler, + clusterId, + ); + } }, { fireImmediately: true, }), diff --git a/src/renderer/components/cluster-manager/lens-views.ts b/src/renderer/components/cluster-manager/lens-views.ts index dc10d55a6f..b468953e06 100644 --- a/src/renderer/components/cluster-manager/lens-views.ts +++ b/src/renderer/components/cluster-manager/lens-views.ts @@ -23,7 +23,7 @@ import { observable, observe, when } from "mobx"; import { ClusterId, ClusterStore, getClusterFrameUrl } from "../../../common/cluster-store"; import logger from "../../../main/logger"; import { requestMain } from "../../../common/ipc"; -import { clusterVisibilityHandler } from "../../../common/cluster-ipc"; +import { claimClusterFrameHandler, clusterVisibilityHandler } from "../../../common/cluster-ipc"; import { toJS } from "../../utils"; export interface LensView { @@ -32,8 +32,15 @@ export interface LensView { view: HTMLIFrameElement } -export const lensViews = observable.map(); -export const visibleCluster = observable.box(); +/** + * These shouldn't be exported so that this file can ensure consistency + */ +const lensViews = observable.map(); +const visibleCluster = observable.box(); + +export function getVisibleCluster() { + return visibleCluster.get(); +} observe(lensViews, change => { console.info(`lensViews change: type=${change.type} name=${change.name}`, toJS((change as any).newValue)); @@ -43,15 +50,34 @@ export function hasLoadedView(clusterId: ClusterId): boolean { return !!lensViews.get(clusterId)?.isLoaded; } -export async function initView(clusterId: ClusterId) { +/** + * + * @param clusterId The cluster to initialize the frame of + * @resolves to `true` if the frame has been initialized (or the ID is invalid) or `false` if + * a different window has submitted a claim for the cluster + */ +export async function initView(clusterId: ClusterId): Promise { const cluster = ClusterStore.getInstance().getById(clusterId); + // If the cluster is unknown or this window already has an iframe active + // then do nothing as the cluster has already been initialized if (!cluster || lensViews.has(clusterId)) { - return; + return true; + } + + // If we have not successfull claimed this cluster that means that a different + // window has, return false so that the called can navigate to it + if (!await requestMain(claimClusterFrameHandler, clusterId)) { + return false; } logger.info(`[LENS-VIEW]: init dashboard, clusterId=${clusterId}`); const parentElem = document.getElementById("lens-views"); + + if (!parentElem) { + throw new Error(`Failed to initialize view for clusterId=${clusterId}: DOM missing #lens-views`); + } + const iframe = document.createElement("iframe"); iframe.name = cluster.contextName; @@ -69,26 +95,32 @@ export async function initView(clusterId: ClusterId) { await when(() => cluster.ready, { timeout: 5_000 }); // we cannot wait forever because cleanup would be blocked for broken cluster connections logger.info(`[LENS-VIEW]: cluster is ready, clusterId=${clusterId}`); } finally { - await autoCleanOnRemove(clusterId, iframe); + autoCleanOnRemove(clusterId, iframe); } + + return true; } -export async function autoCleanOnRemove(clusterId: ClusterId, iframe: HTMLIFrameElement) { - await when(() => { - const cluster = ClusterStore.getInstance().getById(clusterId); +function autoCleanOnRemove(clusterId: ClusterId, iframe: HTMLIFrameElement) { + return when( + () => { + const cluster = ClusterStore.getInstance().getById(clusterId); - return !cluster || (cluster.disconnected && lensViews.get(clusterId)?.isLoaded); - }); - logger.info(`[LENS-VIEW]: remove dashboard, clusterId=${clusterId}`); - lensViews.delete(clusterId); + return !cluster || (cluster.disconnected && lensViews.get(clusterId)?.isLoaded); + }, + () => { + logger.info(`[LENS-VIEW]: remove dashboard, clusterId=${clusterId}`); + lensViews.delete(clusterId); - // Keep frame in DOM to avoid possible bugs when same cluster re-created after being removed. - // In that case for some reasons `webFrame.routingId` returns some previous frameId (usage in app.tsx) - // Issue: https://github.com/lensapp/lens/issues/811 - iframe.style.display = "none"; - iframe.dataset.meta = `${iframe.name} was removed at ${new Date().toLocaleString()}`; - iframe.removeAttribute("name"); - iframe.contentWindow.postMessage("teardown", "*"); + // Keep frame in DOM to avoid possible bugs when same cluster re-created after being removed. + // In that case for some reasons `webFrame.routingId` returns some previous frameId (usage in app.tsx) + // Issue: https://github.com/lensapp/lens/issues/811 + iframe.style.display = "none"; + iframe.dataset.meta = `${iframe.name} was removed at ${new Date().toLocaleString()}`; + iframe.removeAttribute("name"); + iframe.contentWindow.postMessage("teardown", "*"); + } + ); } export function refreshViews(visibleClusterId?: string) { diff --git a/src/renderer/components/hotbar/hotbar-entity-icon.tsx b/src/renderer/components/hotbar/hotbar-entity-icon.tsx index cd716eae78..90044727bf 100644 --- a/src/renderer/components/hotbar/hotbar-entity-icon.tsx +++ b/src/renderer/components/hotbar/hotbar-entity-icon.tsx @@ -31,7 +31,7 @@ import { cssNames, IClassName } from "../../utils"; import { Icon } from "../icon"; import { HotbarIcon } from "./hotbar-icon"; import { HotbarStore } from "../../../common/hotbar-store"; -import { visibleCluster } from "../cluster-manager/lens-views"; +import { getVisibleCluster } from "../cluster-manager/lens-views"; interface Props extends DOMAttributes { entity: CatalogEntity; @@ -88,7 +88,7 @@ export class HotbarEntityIcon extends React.Component { } isActive(item: CatalogEntity) { - return visibleCluster.get() === item.getId(); + return getVisibleCluster() === item.getId(); } isPersisted(entity: CatalogEntity) { diff --git a/src/renderer/ipc/index.tsx b/src/renderer/ipc/index.tsx index 5eff7373d3..9b3f2c289e 100644 --- a/src/renderer/ipc/index.tsx +++ b/src/renderer/ipc/index.tsx @@ -28,7 +28,7 @@ import { isMac } from "../../common/vars"; import { ClusterStore } from "../../common/cluster-store"; import { navigate } from "../navigation"; import { entitySettingsURL } from "../../common/routes"; -import { visibleCluster } from "../components/cluster-manager/lens-views"; +import { getVisibleCluster } from "../components/cluster-manager/lens-views"; function sendToBackchannel(backchannel: string, notificationId: string, data: BackchannelArg): void { notificationsStore.remove(notificationId); @@ -87,7 +87,7 @@ function ListNamespacesForbiddenHandler(event: IpcRendererEvent, ...[clusterId]: return void console.warn("[IPC]: ListNamespacesForbiddenHandler was called with unknown clusterId", { clusterId }); } - const visibileClusterId = visibleCluster.get(); + const visibileClusterId = getVisibleCluster(); if (visibileClusterId && visibileClusterId !== clusterId) { return void console.debug("[IPC]: ListNamespacesForbiddenHandler not displaying notification that is not about the currently active cluster");