diff --git a/src/Controls/src/Core/HandlerImpl/Application/Application.Impl.cs b/src/Controls/src/Core/HandlerImpl/Application/Application.Impl.cs index f7edda731a9d..cc623971d061 100644 --- a/src/Controls/src/Core/HandlerImpl/Application/Application.Impl.cs +++ b/src/Controls/src/Core/HandlerImpl/Application/Application.Impl.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Runtime.CompilerServices; using Microsoft.Extensions.Logging; using Microsoft.Maui.ApplicationModel; using Microsoft.Maui.Devices; @@ -9,10 +10,10 @@ namespace Microsoft.Maui.Controls { public partial class Application : IApplication { - const string MauiWindowIdKey = "__MAUI_WINDOW_ID__"; + internal const string MauiWindowIdKey = "__MAUI_WINDOW_ID__"; readonly List _windows = new(); - readonly Dictionary _requestedWindows = new(); + readonly Dictionary> _requestedWindows = new(); ILogger? _logger; ILogger? Logger => @@ -29,8 +30,14 @@ IWindow IApplication.CreateWindow(IActivationState? activationState) // try get the window that is pending if (activationState?.State?.TryGetValue(MauiWindowIdKey, out var requestedWindowId) ?? false) { - if (requestedWindowId != null && _requestedWindows.TryGetValue(requestedWindowId, out var w)) - window = w; + if (requestedWindowId != null && _requestedWindows.TryGetValue(requestedWindowId, out var r)) + { + if (r.TryGetTarget(out var w)) + { + window = w; + } + _requestedWindows.Remove(requestedWindowId); + } } // create a new one if there is no pending windows @@ -86,9 +93,8 @@ internal void RemoveWindow(Window window) public virtual void OpenWindow(Window window) { - var id = Guid.NewGuid().ToString(); - - _requestedWindows[id] = window; + var id = Guid.NewGuid().ToString("n"); + _requestedWindows.Add(id, new WeakReference(window)); var state = new PersistedState { diff --git a/src/Controls/src/Core/HandlerImpl/Window/Window.Impl.cs b/src/Controls/src/Core/HandlerImpl/Window/Window.Impl.cs index dd6fcb365d55..eb0e28e5a2d5 100644 --- a/src/Controls/src/Core/HandlerImpl/Window/Window.Impl.cs +++ b/src/Controls/src/Core/HandlerImpl/Window/Window.Impl.cs @@ -504,6 +504,7 @@ void IWindow.Destroying() OnDestroying(); Application?.RemoveWindow(this); + Handler?.DisconnectHandler(); } void IWindow.Resumed() diff --git a/src/Controls/tests/Core.UnitTests/WindowsTests.cs b/src/Controls/tests/Core.UnitTests/WindowsTests.cs index 6b063e341ae2..5ad99f8da81c 100644 --- a/src/Controls/tests/Core.UnitTests/WindowsTests.cs +++ b/src/Controls/tests/Core.UnitTests/WindowsTests.cs @@ -1,5 +1,8 @@ -using System.Collections.Generic; +using System; +using System.Collections; +using System.Collections.Generic; using System.Linq; +using System.Reflection; using System.Threading.Tasks; using Microsoft.Maui.Graphics; using Xunit; @@ -621,5 +624,63 @@ public void MinDimensionsArePassedToCoreCorrectly(double inW, double inH, double Assert.Equal(outW, coreWindow.MinimumWidth); Assert.Equal(outH, coreWindow.MinimumHeight); } + + [Fact] + public async Task WindowDoesNotLeak() + { + var application = new Application(); + WeakReference reference; + + // Scope for window + { + var window = new Window { Page = new ContentPage() }; + reference = new WeakReference(window); + application.OpenWindow(window); + ((IWindow)window).Destroying(); + } + + // GC + await Task.Yield(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + + Assert.False(reference.IsAlive, "Window should not be alive!"); + } + + // NOTE: this test is here to show `ConditionalWeakTable _requestedWindows` was a bad idea + [Fact] + public async Task TwoKeysSameWindow() + { + var application = new Application(); + var window = new Window { Page = new ContentPage() }; + + application.OpenWindow(window); + + // Access Application._requestedWindows + var flags = BindingFlags.NonPublic | BindingFlags.Instance; + var table = typeof(Application).GetField("_requestedWindows", flags).GetValue(application) as IDictionary; + Assert.NotNull(table); + + // A "cloned" key should still work + string key; + { + var originalKey = table.Keys.OfType().Single(); + key = new string(originalKey); + Assert.NotSame(originalKey, key); + } + + // GC collect the original key + await Task.Yield(); + GC.Collect(); + GC.WaitForPendingFinalizers(); + + // Same window, doesn't create a new one + var actual = ((IApplication)application).CreateWindow(new ActivationState(new MockMauiContext(), new PersistedState + { + { Application.MauiWindowIdKey, key } + })); + Assert.Same(window, actual); + Assert.Empty(table); + } } }