From 735f03f209085d1ac40625d3bc347aba72f68d53 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 10 Nov 2023 10:31:15 -0600 Subject: [PATCH] [ios] fix memory leak in `Switch` Context: https://github.com/dotnet/maui/issues/18365 Adding a parameter to the test: [Theory("Handler Does Not Leak")] [InlineData(typeof(Switch))] public async Task HandlerDoesNotLeak(Type type) Shows a memory leak in `Switch`, caused by the cycle * `SwitchHandler` -> * `UISwitch.ValueChanged` event -> * `SwitchHandler` I could solve this problem by creating a `SwitchProxy` class -- the same pattern I've used in other PRs to avoid cycles. This makes an intermediate type to handle the events and breaks the cycle. Still thinking if the analyzer could have caught this, issue filed at: https://github.com/jonathanpeppers/memory-analyzers/issues/12 --- .../tests/DeviceTests/Memory/MemoryTests.cs | 2 + .../src/Handlers/Switch/SwitchHandler.iOS.cs | 38 +++++++++++++------ 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/Controls/tests/DeviceTests/Memory/MemoryTests.cs b/src/Controls/tests/DeviceTests/Memory/MemoryTests.cs index 13aea7d39d48..eaeebb45da93 100644 --- a/src/Controls/tests/DeviceTests/Memory/MemoryTests.cs +++ b/src/Controls/tests/DeviceTests/Memory/MemoryTests.cs @@ -43,6 +43,7 @@ void SetupBuilder() handlers.AddHandler(); handlers.AddHandler(); handlers.AddHandler(); + handlers.AddHandler(); handlers.AddHandler(); handlers.AddHandler(); }); @@ -71,6 +72,7 @@ void SetupBuilder() [InlineData(typeof(ScrollView))] [InlineData(typeof(Stepper))] [InlineData(typeof(SwipeView))] + [InlineData(typeof(Switch))] [InlineData(typeof(TimePicker))] [InlineData(typeof(WebView))] public async Task HandlerDoesNotLeak(Type type) diff --git a/src/Core/src/Handlers/Switch/SwitchHandler.iOS.cs b/src/Core/src/Handlers/Switch/SwitchHandler.iOS.cs index 20f2ab719e6c..26f043ffe788 100644 --- a/src/Core/src/Handlers/Switch/SwitchHandler.iOS.cs +++ b/src/Core/src/Handlers/Switch/SwitchHandler.iOS.cs @@ -8,6 +8,8 @@ namespace Microsoft.Maui.Handlers { public partial class SwitchHandler : ViewHandler { + readonly SwitchProxy _proxy = new(); + // the UISwitch control becomes inaccessible if it grows to a width > 101 // An issue has been logged with Apple // This ensures that the UISwitch remains the natural size that iOS expects @@ -22,15 +24,13 @@ protected override UISwitch CreatePlatformView() protected override void ConnectHandler(UISwitch platformView) { base.ConnectHandler(platformView); - - platformView.ValueChanged += OnControlValueChanged; + _proxy.Connect(VirtualView, platformView); } protected override void DisconnectHandler(UISwitch platformView) { base.DisconnectHandler(platformView); - - platformView.ValueChanged -= OnControlValueChanged; + _proxy.Disconnect(platformView); } public static void MapIsOn(ISwitchHandler handler, ISwitch view) @@ -49,17 +49,33 @@ public static void MapThumbColor(ISwitchHandler handler, ISwitch view) handler.PlatformView?.UpdateThumbColor(view); } - void OnControlValueChanged(object? sender, EventArgs e) + static void UpdateIsOn(ISwitchHandler handler) { - if (VirtualView is null || PlatformView is null || VirtualView.IsOn == PlatformView.On) - return; - - VirtualView.IsOn = PlatformView.On; + handler.UpdateValue(nameof(ISwitch.TrackColor)); } - static void UpdateIsOn(ISwitchHandler handler) + class SwitchProxy { - handler.UpdateValue(nameof(ISwitch.TrackColor)); + WeakReference? _virtualView; + + ISwitch? VirtualView => _virtualView is not null && _virtualView.TryGetTarget(out var v) ? v : null; + + public void Connect(ISwitch virtualView, UISwitch platformView) + { + _virtualView = new(virtualView); + platformView.ValueChanged += OnControlValueChanged; + } + + public void Disconnect(UISwitch platformView) + { + platformView.ValueChanged -= OnControlValueChanged; + } + + void OnControlValueChanged(object? sender, EventArgs e) + { + if (VirtualView is ISwitch virtualView && sender is UISwitch platformView && virtualView.IsOn != platformView.On) + virtualView.IsOn = platformView.On; + } } } } \ No newline at end of file