Skip to content

Commit

Permalink
[ios] fix memory leak in Switch (#18682)
Browse files Browse the repository at this point in the history
Context: #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:

jonathanpeppers/memory-analyzers#12
  • Loading branch information
jonathanpeppers authored Nov 13, 2023
1 parent 275dd6d commit 2e286bf
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 11 deletions.
2 changes: 2 additions & 0 deletions src/Controls/tests/DeviceTests/Memory/MemoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ void SetupBuilder()
handlers.AddHandler<IScrollView, ScrollViewHandler>();
handlers.AddHandler<Stepper, StepperHandler>();
handlers.AddHandler<SwipeView, SwipeViewHandler>();
handlers.AddHandler<Switch, SwitchHandler>();
handlers.AddHandler<TimePicker, TimePickerHandler>();
handlers.AddHandler<WebView, WebViewHandler>();
});
Expand Down Expand Up @@ -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)
Expand Down
38 changes: 27 additions & 11 deletions src/Core/src/Handlers/Switch/SwitchHandler.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ namespace Microsoft.Maui.Handlers
{
public partial class SwitchHandler : ViewHandler<ISwitch, UISwitch>
{
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
Expand All @@ -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)
Expand All @@ -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<ISwitch>? _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;
}
}
}
}

0 comments on commit 2e286bf

Please sign in to comment.