Skip to content

Commit

Permalink
[ios] fix memory leak in Button
Browse files Browse the repository at this point in the history
Fixes: dotnet#12048
Context: https://github.com/jonathanpeppers/MemoryLeaksOniOS
Context: https://stackoverflow.com/a/13059140

Issue dotnet#12048 has four scenarios that are currently fixed in `main` for
Android and Windows. Unfortunately, one of the four scenarios fails on
iOS due to its usage of `Button`. I could reproduce the issue by
simply adding a `Button` to existing memory leak tests.

`ButtonHandler.iOS.cs` has a "cycle":

* `ButtonHandler` subscribes to events like `UIButton.TouchUpInside`
  making `UIButton` hold a strong reference to `ButtonHandler`.

* `ButtonHandler` (like all handlers) has a strong reference to the
  `Microsoft.Maui.Button`.

* `ButtonHandler` has a strong reference to `UIButton`.

* `Microsoft.Maui.Button` has a strong reference to `ButtonHandler`.

To solve this issue, we can make a nested classed named `ButtonProxy`:

* Its purpose is subscribe to `UIButton.TouchUpInside` and invoke
  `Microsoft.Maui.Button.Clicked()`.

* `ButtonProxy` has a weak reference to the `Microsoft.Maui.Button`.

Now the `UIButton`, `ButtonProxy`, `ButtonHandler`, and
`Microsoft.Maui.Button` can all be collected.
  • Loading branch information
jonathanpeppers committed Mar 29, 2023
1 parent cbce20c commit 08be7ab
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ void SetupBuilder()
handlers.AddHandler<Window, WindowHandlerStub>();
handlers.AddHandler<Frame, FrameRenderer>();
handlers.AddHandler<Label, LabelHandler>();
handlers.AddHandler<Button, ButtonHandler>();
});
});
}
Expand Down Expand Up @@ -298,7 +299,7 @@ public async Task DoesNotLeak()

await CreateHandlerAndAddToWindow<WindowHandlerStub>(new Window(navPage), async (handler) =>
{
var page = new ContentPage { Title = "Page 2", Content = new VerticalStackLayout { new Label() } };
var page = new ContentPage { Title = "Page 2", Content = new VerticalStackLayout { new Label(), new Button() } };
pageReference = new WeakReference(page);
await navPage.Navigation.PushAsync(page);
await navPage.Navigation.PopAsync();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -968,7 +968,7 @@ await CreateHandlerAndAddToWindow<ShellHandler>(shell, async (handler) =>
{
await OnLoadedAsync(shell.CurrentPage);
var page = new ContentPage { Title = "Page 2", Content = new VerticalStackLayout { new Label() } };
var page = new ContentPage { Title = "Page 2", Content = new VerticalStackLayout { new Label(), new Button() } };
pageReference = new WeakReference(page);
await shell.Navigation.PushAsync(page);
Expand Down
1 change: 1 addition & 0 deletions src/Controls/tests/DeviceTests/Extensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public static void SetupShellHandlers(this IMauiHandlersCollection handlers)
handlers.TryAddHandler<Layout, LayoutHandler>();
handlers.TryAddHandler<Image, ImageHandler>();
handlers.TryAddHandler<Label, LabelHandler>();
handlers.TryAddHandler<Button, ButtonHandler>();
handlers.TryAddHandler<Page, PageHandler>();
handlers.TryAddHandler(typeof(Toolbar), typeof(ToolbarHandler));
handlers.TryAddHandler(typeof(MenuBar), typeof(MenuBarHandler));
Expand Down
64 changes: 47 additions & 17 deletions src/Core/src/Handlers/Button/ButtonHandler.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,18 @@ protected override UIButton CreatePlatformView()
return button;
}

readonly ButtonEventProxy _proxy = new ButtonEventProxy();

protected override void ConnectHandler(UIButton platformView)
{
platformView.TouchUpInside += OnButtonTouchUpInside;
platformView.TouchUpOutside += OnButtonTouchUpOutside;
platformView.TouchDown += OnButtonTouchDown;
_proxy.Connect(VirtualView, platformView);

base.ConnectHandler(platformView);
}

protected override void DisconnectHandler(UIButton platformView)
{
platformView.TouchUpInside -= OnButtonTouchUpInside;
platformView.TouchUpOutside -= OnButtonTouchUpOutside;
platformView.TouchDown -= OnButtonTouchDown;
_proxy.Disconnect(platformView);

base.DisconnectHandler(platformView);
}
Expand Down Expand Up @@ -166,20 +164,52 @@ static void SetControlPropertiesFromProxy(UIButton platformView)
}
}

void OnButtonTouchUpInside(object? sender, EventArgs e)
class ButtonEventProxy
{
VirtualView?.Released();
VirtualView?.Clicked();
}
WeakReference<IButton>? _virtualView;

void OnButtonTouchUpOutside(object? sender, EventArgs e)
{
VirtualView?.Released();
}
public void Connect(IButton virtualView, UIButton platformView)
{
_virtualView = new(virtualView);

void OnButtonTouchDown(object? sender, EventArgs e)
{
VirtualView?.Pressed();
platformView.TouchUpInside += OnButtonTouchUpInside;
platformView.TouchUpOutside += OnButtonTouchUpOutside;
platformView.TouchDown += OnButtonTouchDown;
}

public void Disconnect(UIButton platformView)
{
_virtualView = null;

platformView.TouchUpInside -= OnButtonTouchUpInside;
platformView.TouchUpOutside -= OnButtonTouchUpOutside;
platformView.TouchDown -= OnButtonTouchDown;
}

void OnButtonTouchUpInside(object? sender, EventArgs e)
{
if (_virtualView != null && _virtualView.TryGetTarget(out var virtualView))
{
virtualView.Released();
virtualView.Clicked();
}
}

void OnButtonTouchUpOutside(object? sender, EventArgs e)
{
if (_virtualView != null && _virtualView.TryGetTarget(out var virtualView))
{
virtualView.Released();
}
}

void OnButtonTouchDown(object? sender, EventArgs e)
{
if (_virtualView != null && _virtualView.TryGetTarget(out var virtualView))
{
virtualView?.Pressed();
}
}
}
}
}

0 comments on commit 08be7ab

Please sign in to comment.