Skip to content

[ios] fix memory leak in RefreshView#16384

Merged
rmarinho merged 1 commit intodotnet:mainfrom
jonathanpeppers:OhNoRefreshViewLeak
Jul 27, 2023
Merged

[ios] fix memory leak in RefreshView#16384
rmarinho merged 1 commit intodotnet:mainfrom
jonathanpeppers:OhNoRefreshViewLeak

Conversation

@jonathanpeppers
Copy link
Copy Markdown
Member

Context: #16346

This addresses the memory leak discovered by:

src/Core/src/Platform/iOS/MauiRefreshView.cs(17,10): error MA0002: Member '_refreshControlParent' could cause memory leaks in an NSObject subclass. Remove the member, store the value as a WeakReference, or add the [UnconditionalSuppressMessage("Memory", "MA0002")] attribute with a justification as to why the member will not leak.
src/Core/src/Platform/iOS/MauiRefreshView.cs(18,11): error MA0002: Member '_contentView' could cause memory leaks in an NSObject subclass. Remove the member, store the value as a WeakReference, or add the [UnconditionalSuppressMessage("Memory", "MA0002")] attribute with a justification as to why the member will not leak.
src/Core/src/Platform/iOS/MauiRefreshView.cs(19,20): error MA0002: Member '_refreshControl' could cause memory leaks in an NSObject subclass. Remove the member, store the value as a WeakReference, or add the [UnconditionalSuppressMessage("Memory", "MA0002")] attribute with a justification as to why the member will not leak.

I could reproduce a leak in a test such as:

await InvokeOnMainThreadAsync(() =>
{
    var layout = new Grid();
    var refreshView = new RefreshView();
    layout.Add(refreshView);
    var handler = CreateHandler<LayoutHandler>(layout);
    viewReference = new WeakReference(refreshView);
    handlerReference = new WeakReference(refreshView.Handler);
    platformViewReference = new WeakReference(refreshView.Handler.PlatformView);
});

await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
Assert.False(viewReference.IsAlive, "RefreshView should not be alive!");
Assert.False(handlerReference.IsAlive, "Handler should not be alive!");
Assert.False(platformViewReference.IsAlive, "PlatformView should not be alive!");

Solved the issues by making a non-NSObject MauiRefreshViewProxy class.

Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiRefreshView.cs(17,10): error MA0002: Member '_refreshControlParent' could cause memory leaks in an NSObject subclass. Remove the member, store the value as a WeakReference, or add the [UnconditionalSuppressMessage("Memory", "MA0002")] attribute with a justification as to why the member will not leak.
    src/Core/src/Platform/iOS/MauiRefreshView.cs(18,11): error MA0002: Member '_contentView' could cause memory leaks in an NSObject subclass. Remove the member, store the value as a WeakReference, or add the [UnconditionalSuppressMessage("Memory", "MA0002")] attribute with a justification as to why the member will not leak.
    src/Core/src/Platform/iOS/MauiRefreshView.cs(19,20): error MA0002: Member '_refreshControl' could cause memory leaks in an NSObject subclass. Remove the member, store the value as a WeakReference, or add the [UnconditionalSuppressMessage("Memory", "MA0002")] attribute with a justification as to why the member will not leak.

I could reproduce a leak in a test such as:

    await InvokeOnMainThreadAsync(() =>
    {
        var layout = new Grid();
        var refreshView = new RefreshView();
        layout.Add(refreshView);
        var handler = CreateHandler<LayoutHandler>(layout);
        viewReference = new WeakReference(refreshView);
        handlerReference = new WeakReference(refreshView.Handler);
        platformViewReference = new WeakReference(refreshView.Handler.PlatformView);
    });

    await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
    Assert.False(viewReference.IsAlive, "RefreshView should not be alive!");
    Assert.False(handlerReference.IsAlive, "Handler should not be alive!");
    Assert.False(platformViewReference.IsAlive, "PlatformView should not be alive!");

Solved the issues by making a non-NSObject `MauiRefreshViewProxy` class.
@jonathanpeppers jonathanpeppers added perf/memory-leak 💦 Memory usage grows / objects live forever (sub: perf) platform/ios labels Jul 26, 2023
@jonathanpeppers jonathanpeppers marked this pull request as ready for review July 27, 2023 13:18
@rmarinho rmarinho merged commit b2b791c into dotnet:main Jul 27, 2023
@jonathanpeppers jonathanpeppers deleted the OhNoRefreshViewLeak branch July 27, 2023 15:44
@github-actions github-actions bot locked and limited conversation to collaborators Dec 7, 2023
@samhouts samhouts added the fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 perf/memory-leak 💦 Memory usage grows / objects live forever (sub: perf) platform/ios

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants