Ensure windows are not retained after being destroyed #1008

src/ElectronNET.API/API/WindowManager.cs

@@ -19,6 +19,7 @@ namespace ElectronNET.API
 
         internal WindowManager()
         {
+            EnsureBrowserWindowClosedSubscription();
         }
 
         internal static WindowManager Instance
@@ -106,17 +107,6 @@ namespace ElectronNET.API
                 tcs.SetResult(browserWindow);
             });
 
-            BridgeConnector.Socket.Once<int[]>("BrowserWindowClosed", (ids) =>
-            {
-                for (int index = 0; index < _browserWindows.Count; index++)
-                {
-                    if (!ids.Contains(_browserWindows[index].Id))
-                    {
-                        _browserWindows.RemoveAt(index);
-                    }
-                }
-            });
-
             if (loadUrl.Equals("http://localhost", StringComparison.OrdinalIgnoreCase) && ElectronNetRuntime.AspNetWebPort.HasValue)
             {
                 loadUrl = $"{loadUrl}:{ElectronNetRuntime.AspNetWebPort}";
@@ -149,6 +139,40 @@ namespace ElectronNET.API
             return await tcs.Task.ConfigureAwait(false);
         }
 
+        private readonly object _browserWindowSubscriptionSync = new();
+        private bool _browserWindowClosedSubscribed;
+
+        private void EnsureBrowserWindowClosedSubscription()
+        {
+            if (_browserWindowClosedSubscribed)
+            {
+                return;
+            }
+
+            lock (_browserWindowSubscriptionSync)
+            {
+                if (_browserWindowClosedSubscribed)
+                {
+                    return;
+                }
+
+                BridgeConnector.Socket.On<int[]>("BrowserWindowClosed", HandleBrowserWindowClosed);
+                _browserWindowClosedSubscribed = true;
+            }
+        }
+
+        private void HandleBrowserWindowClosed(int[] ids)
+        {
+            if (ids == null || ids.Length == 0)
+            {
+                _browserWindows.Clear();
+                return;
+            }
+
+            var existingIds = ids.ToHashSet();
+            _browserWindows.RemoveAll(window => !existingIds.Contains(window.Id));
+        }
+
         private bool IsWindows10()
         {
             return RuntimeInformation.OSDescription.Contains("Windows 10");

src/ElectronNET.Host/api/browserWindows.js

@@ -230,7 +230,10 @@ module.exports = (socket, app) => {
             window = app["mainWindow"];
             if (window) {
                 window.reload();
-                windows.push(window);
+                synchronizeWindowRegistry();
+                if (!windows.some((entry) => tryGetWindowId(entry) === window.id)) {
+                    windows.push(window);
+                }
                 electronSocket.emit("BrowserWindowCreated", window.id);
                 return;
             }
@@ -253,21 +256,9 @@ module.exports = (socket, app) => {
             }
         });
         lastOptions = options;
-        window.on("closed", (sender) => {
-            for (let index = 0; index < windows.length; index++) {
-                const windowItem = windows[index];
-                try {
-                    windowItem.id;
-                }
-                catch (error) {
-                    if (error.message === "Object has been destroyed") {
-                        windows.splice(index, 1);
-                        const ids = [];
-                        windows.forEach((x) => ids.push(x.id));
-                        electronSocket.emit("BrowserWindowClosed", ids);
-                    }
-                }
-            }
+        window.on("closed", () => {
+            synchronizeWindowRegistry();
+            emitBrowserWindowClosed();
         });
         app.on("activate", () => {
             // On macOS it's common to re-create a window in the app when the
@@ -703,12 +694,47 @@ module.exports = (socket, app) => {
         getWindowById(id).setBrowserView((0, browserView_1.browserViewMediateService)(browserViewId));
     });
     function getWindowById(id) {
+        const runtimeWindow = electron_1.BrowserWindow.fromId(id);
+        if (runtimeWindow) {
+            return runtimeWindow;
+        }
+        synchronizeWindowRegistry();
         for (let index = 0; index < windows.length; index++) {
             const element = windows[index];
-            if (element.id === id) {
+            if (tryGetWindowId(element) === id) {
                 return element;
             }
         }
+        throw new Error(`BrowserWindow with id '${id}' was not found.`);
+    }
+    function tryGetWindowId(element) {
+        try {
+            return element.id;
+        }
+        catch {
+            return null;
+        }
+    }
+    function synchronizeWindowRegistry() {
+        const runtimeWindows = electron_1.BrowserWindow.getAllWindows();
+        const runtimeWindowIds = new Set(runtimeWindows.map((entry) => entry.id));
+        for (let index = windows.length - 1; index >= 0; index--) {
+            const windowId = tryGetWindowId(windows[index]);
+            if (windowId === null || !runtimeWindowIds.has(windowId)) {
+                windows.splice(index, 1);
+            }
+        }
+        readyToShowWindowsIds = readyToShowWindowsIds.filter((entryId) => runtimeWindowIds.has(entryId));
+    }
+    function emitBrowserWindowClosed() {
+        const ids = [];
+        for (const entry of windows) {
+            const windowId = tryGetWindowId(entry);
+            if (windowId !== null) {
+                ids.push(windowId);
+            }
+        }
+        electronSocket.emit("BrowserWindowClosed", ids);
     }
 };
 //# sourceMappingURL=browserWindows.js.map

src/ElectronNET.Host/api/browserWindows.ts

@@ -255,7 +255,10 @@ export = (socket: Socket, app: Electron.App) => {
       window = app["mainWindow"];
       if (window) {
         window.reload();
-        windows.push(window);
+        synchronizeWindowRegistry();
+        if (!windows.some((entry) => tryGetWindowId(entry) === window.id)) {
+          windows.push(window);
+        }
         electronSocket.emit("BrowserWindowCreated", window.id);
         return;
       }
@@ -283,21 +286,9 @@ export = (socket: Socket, app: Electron.App) => {
 
     lastOptions = options;
 
-    window.on("closed", (sender) => {
-      for (let index = 0; index < windows.length; index++) {
-        const windowItem = windows[index];
-        try {
-          windowItem.id;
-        } catch (error) {
-          if (error.message === "Object has been destroyed") {
-            windows.splice(index, 1);
-
-            const ids = [];
-            windows.forEach((x) => ids.push(x.id));
-            electronSocket.emit("BrowserWindowClosed", ids);
-          }
-        }
-      }
+    window.on("closed", () => {
+      synchronizeWindowRegistry();
+      emitBrowserWindowClosed();
     });
 
     app.on("activate", () => {
@@ -907,11 +898,57 @@ export = (socket: Socket, app: Electron.App) => {
   });
 
   function getWindowById(id: number): Electron.BrowserWindow {
+    const runtimeWindow = BrowserWindow.fromId(id);
+    if (runtimeWindow) {
+      return runtimeWindow;
+    }
+
+    synchronizeWindowRegistry();
+
     for (let index = 0; index < windows.length; index++) {
       const element = windows[index];
-      if (element.id === id) {
+      if (tryGetWindowId(element) === id) {
         return element;
       }
     }
+
+    throw new Error(`BrowserWindow with id '${id}' was not found.`);
+  }
+
+  function tryGetWindowId(element: Electron.BrowserWindow): number | null {
+    try {
+      return element.id;
+    } catch {
+      return null;
+    }
+  }
+
+  function synchronizeWindowRegistry(): void {
+    const runtimeWindows = BrowserWindow.getAllWindows();
+    const runtimeWindowIds = new Set(runtimeWindows.map((entry) => entry.id));
+
+    for (let index = windows.length - 1; index >= 0; index--) {
+      const windowId = tryGetWindowId(windows[index]);
+      if (windowId === null || !runtimeWindowIds.has(windowId)) {
+        windows.splice(index, 1);
+      }
+    }
+
+    readyToShowWindowsIds = readyToShowWindowsIds.filter((entryId) =>
+      runtimeWindowIds.has(entryId),
+    );
+  }
+
+  function emitBrowserWindowClosed(): void {
+    const ids: number[] = [];
+
+    for (const entry of windows) {
+      const windowId = tryGetWindowId(entry);
+      if (windowId !== null) {
+        ids.push(windowId);
+      }
+    }
+
+    electronSocket.emit("BrowserWindowClosed", ids);
   }
 };

src/ElectronNET.IntegrationTests/Tests/WindowManagerLifecycleTests.cs

@@ -0,0 +1,55 @@
+namespace ElectronNET.IntegrationTests.Tests;
+
+/// <summary>
+/// Regression checks for BrowserWindow lifecycle cleanup in WindowManager.
+/// Covers GitHub issue #1008.
+/// </summary>
+public class WindowManagerLifecycleTests
+{
+    private static readonly string WindowManagerFilePath = FindWindowManagerFile();
+
+    [Fact]
+    public void WindowManager_ShouldUsePersistentBrowserWindowClosedSubscription()
+    {
+        File.Exists(WindowManagerFilePath).Should().BeTrue(
+            $"WindowManager source must exist at '{WindowManagerFilePath}'.");
+
+        var content = File.ReadAllText(WindowManagerFilePath);
+
+        content.Should().Contain(
+            "Socket.On<int[]>(\"BrowserWindowClosed\"",
+            "closed-window cleanup must be wired for every close event, not just the first one.");
+
+        content.Should().NotContain(
+            "Socket.Once<int[]>(\"BrowserWindowClosed\"",
+            "a one-shot subscription causes stale BrowserWindow references after subsequent closes (issue #1008).");
+    }
+
+    private static string FindWindowManagerFile()
+    {
+        const string RelativeFromRepoRoot = "src/ElectronNET.API/API/WindowManager.cs";
+        const string RelativeFromSrc = "ElectronNET.API/API/WindowManager.cs";
+
+        var dir = new DirectoryInfo(AppContext.BaseDirectory);
+        while (dir != null)
+        {
+            var fromRepoRoot = Path.Combine(dir.FullName, RelativeFromRepoRoot);
+            if (File.Exists(fromRepoRoot))
+            {
+                return Path.GetFullPath(fromRepoRoot);
+            }
+
+            var fromSrc = Path.Combine(dir.FullName, RelativeFromSrc);
+            if (File.Exists(fromSrc))
+            {
+                return Path.GetFullPath(fromSrc);
+            }
+
+            dir = dir.Parent;
+        }
+
+        throw new FileNotFoundException(
+            "Could not locate WindowManager.cs by walking up from " +
+            $"'{AppContext.BaseDirectory}'.");
+    }
+}