Skip to content

Commit

Permalink
[ios] fix memory leak in Border (#15946)
Browse files Browse the repository at this point in the history
Context: #14664
Context: https://github.com/nacompllo/MemoryLeakEverywhere/tree/bugfix/memoryLeakItemsSource

While looking at the customer sample, we found an issue with
`BorderHandler`:

* `ContentView` -> via `LayoutSubviewsChanged`
* `BorderHandler` ->
* `ContentView`

Creating a cycle & memory leak on iOS and Catalyst. We could reproduce
this in a device test.

It appears the event only did this:

    void OnLayoutSubviewsChanged(object? sender, EventArgs e)
    {
        PlatformView?.UpdateMauiCALayer();
    }

And so instead, we can just call the extension method directly:

    this.UpdateMauiCALayer();

And the leak is gone!

Co-authored-by: Haritha Mohan <harithamohan@microsoft.com>
  • Loading branch information
jonathanpeppers and haritha-mohan authored Jun 30, 2023
1 parent 01fe19f commit d211b85
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 13 deletions.
53 changes: 52 additions & 1 deletion src/Controls/tests/DeviceTests/Elements/Border/BorderTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Threading.Tasks;
using System;
using System.Threading.Tasks;
using Microsoft.Maui.Controls;
using Microsoft.Maui.Graphics;
using Microsoft.Maui.Handlers;
Expand All @@ -11,6 +12,18 @@ namespace Microsoft.Maui.DeviceTests
[Category(TestCategory.Border)]
public partial class BorderTests : ControlsHandlerTestBase
{
void SetupBuilder()
{
EnsureHandlerCreated(builder =>
{
builder.ConfigureMauiHandlers(handlers =>
{
handlers.AddHandler<Label, LabelHandler>();
handlers.AddHandler<Border, BorderHandler>();
});
});
}

[Fact(DisplayName = "Rounded Rectangle Border occupies correct space")]
public async Task RoundedRectangleBorderLayoutIsCorrect()
{
Expand All @@ -31,5 +44,43 @@ public async Task RoundedRectangleBorderLayoutIsCorrect()

await AssertColorAtPoint(border, expected, typeof(BorderHandler), 10, 10);
}


[Fact("Border Does Not Leak")]
public async Task DoesNotLeak()
{
SetupBuilder();
WeakReference platformViewReference = null;
WeakReference handlerReference = null;

await InvokeOnMainThreadAsync(() =>
{
var layout = new Grid();
var border = new Border();
var label = new Label();
border.Content = label;
layout.Add(border);

var handler = CreateHandler<LayoutHandler>(layout);
handlerReference = new WeakReference(label.Handler);
platformViewReference = new WeakReference(label.Handler.PlatformView);
});

Assert.NotNull(handlerReference);
Assert.NotNull(platformViewReference);

// Several GCs required on iOS
for (int i = 0; i < 5; i++)
{
if (!handlerReference.IsAlive && !platformViewReference.IsAlive)
break;
await Task.Yield();
GC.Collect();
GC.WaitForPendingFinalizers();
}

Assert.False(handlerReference.IsAlive, "Handler should not be alive!");
Assert.False(platformViewReference.IsAlive, "PlatformView should not be alive!");
}
}
}
9 changes: 0 additions & 9 deletions src/Core/src/Handlers/Border/BorderHandler.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,11 @@ protected override ContentView CreatePlatformView()
protected override void ConnectHandler(ContentView platformView)
{
base.ConnectHandler(platformView);

platformView.LayoutSubviewsChanged += OnLayoutSubviewsChanged;
}

protected override void DisconnectHandler(ContentView platformView)
{
base.DisconnectHandler(platformView);

platformView.LayoutSubviewsChanged -= OnLayoutSubviewsChanged;
}

public override void SetVirtualView(IView view)
Expand Down Expand Up @@ -63,10 +59,5 @@ public static void MapContent(IBorderHandler handler, IBorderView border)
{
UpdateContent(handler);
}

void OnLayoutSubviewsChanged(object? sender, EventArgs e)
{
PlatformView?.UpdateMauiCALayer();
}
}
}
4 changes: 1 addition & 3 deletions src/Core/src/Platform/iOS/ContentView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ public class ContentView : MauiView
{
WeakReference<IBorderStroke>? _clip;
CAShapeLayer? _childMaskLayer;
internal event EventHandler? LayoutSubviewsChanged;

public ContentView()
{
Expand All @@ -27,8 +26,7 @@ public override void LayoutSubviews()
ChildMaskLayer.Frame = bounds;

SetClip();

LayoutSubviewsChanged?.Invoke(this, EventArgs.Empty);
this.UpdateMauiCALayer();
}

internal IBorderStroke? Clip
Expand Down

0 comments on commit d211b85

Please sign in to comment.