Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ios] test MemoryAnalyzers package #16346

Merged
merged 1 commit into from
Aug 21, 2023

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Jul 25, 2023

Context: https://github.com/jonathanpeppers/memory-analyzers
Context: https://www.nuget.org/packages/MemoryAnalyzers/0.1.0-beta.3

This adds a new Roslyn analyzer that warns about the following cases.

MA0001

Don't define public events in NSObject subclasses:

public class MyView : UIView
{
    // NOPE!
    public event EventHandler MyEvent;
}

MA0002

Don't declare members in NSObject subclasses unless they are:

  • WeakReference or WeakReference<T>
  • Value types
class MyView : UIView
{
    // NOPE!
    public UIView? Parent { get; set; }

    public void Add(MyView subview)
    {
        subview.Parent = this;
        AddSubview(subview);
    }
}

MA0003

Don't subscribe to events inside NSObject subclasses unless:

  • It's your event via this.MyEvent or from a base type.
  • The method is static.
class MyView : UIView
{
    public MyView()
    {
        var picker = new UIDatePicker();
        AddSubview(picker);
        picker.ValueChanged += OnValueChanged;
    }

    void OnValueChanged(object sender, EventArgs e) { }

    // Use this instead and it doesn't leak!
    //static void OnValueChanged(object sender, EventArgs e) { }
}

Places with PRs in flight are marked with:

[UnconditionalSuppressMessage("Memory", "MA0002", Justification = "FIXME: https://github.com/dotnet/maui/pull/16530")]

A few are marked as "not an issue" with an explanation. Others mention a
test with a proof they are OK.

A few places I could actually remove UnconditionalSuppressMessage
where I could improve the analyzer to ignore that case.

@jonathanpeppers jonathanpeppers added the memory-leak 💦 Memory usage grows / objects live forever label Jul 25, 2023
@jonathanpeppers
Copy link
Member Author

The list is:

src/Core/src/Platform/iOS/ContainerViewController.cs(11,11): error MA0002: Member 'currentPlatformView' 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/ContainerViewController.cs(16,11): error MA0002: Member '_pendingLoadedView' 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/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.iOS.cs(10,30): error MA0001: Event 'FrameChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiPicker.cs(21,24): error MA0002: Member 'UIPickerView' 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/MauiDoneAccessoryView.cs(11,19): error MA0002: Member '_doneClicked' 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/MauiImageView.cs(34,30): error MA0001: Event 'WindowChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiTimePicker.cs(13,19): error MA0002: Member '_dateSelected' 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(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.
src/Core/src/Platform/iOS/MauiTimePicker.cs(10,25): error MA0002: Member '_picker' 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/ResignFirstResponderTouchGestureRecognizer.cs(9,11): error MA0002: Member '_targetView' 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/ResignFirstResponderTouchGestureRecognizer.cs(10,10): error MA0002: Member '_token' 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/MauiTextView.cs(37,30): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiTextView.cs(12,20): error MA0002: Member '_placeholderLabel' 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/MauiTimePicker.cs(57,30): error MA0001: Event 'DateSelected' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/WindowOverlay/WindowOverlay.iOS.cs(92,40): error MA0001: Event 'OnTouch' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/PlatformTouchGraphicsView.cs(12,29): error MA0002: Member '_hoverGesture' 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/MauiWKWebView.cs(15,25): error MA0002: Member 'SharedPool' 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/MauiSearchBar.cs(36,63): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiSearchBar.cs(54,32): error MA0001: Event 'OnMovedToWindow' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiSearchBar.cs(55,32): error MA0001: Event 'EditingChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiCheckBox.cs(27,24): error MA0002: Member 'CheckedChanged' 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/MauiSwipeView.cs(21,15): error MA0002: Member '_actionView' 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/MauiTextField.cs(69,32): error MA0001: Event 'SelectionChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiTextField.cs(68,30): error MA0001: Event 'TextPropertySet' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiSwipeView.cs(18,26): error MA0002: Member '_tapGestureRecognizer' 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/MauiSwipeView.cs(19,26): error MA0002: Member '_panGestureRecognizer' 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/MauiSwipeView.cs(20,10): 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/MauiDatePicker.cs(37,27): error MA0003: Subscribing to events with instance method 'OnValueChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.
src/Core/src/Platform/iOS/MauiSearchBar.cs(70,31): error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Jul 25, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiTextView.cs(37,30): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiTextView.cs(12,20): error MA0002: Member '_placeholderLabel' 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 editor = new Editor();
        layout.Add(editor);
        var handler = CreateHandler<LayoutHandler>(layout);
        viewReference = new WeakReference(editor);
        handlerReference = new WeakReference(editor.Handler);
        platformViewReference = new WeakReference(editor.Handler.PlatformView);
    });

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

I will create a similar PR for `Entry` as well.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Jul 25, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiTextField.cs(69,32): error MA0001: Event 'SelectionChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiTextField.cs(68,30): error MA0001: Event 'TextPropertySet' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.

I could reproduce a leak in a test such as:

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

    await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
    Assert.False(viewReference.IsAlive, "Entry should not be alive!");
    Assert.False(handlerReference.IsAlive, "Handler should not be alive!");
    Assert.False(platformViewReference.IsAlive, "PlatformView should not be alive!");
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Jul 26, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiTextView.cs(37,30): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiTextView.cs(12,20): error MA0002: Member '_placeholderLabel' 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 editor = new Editor();
        layout.Add(editor);
        var handler = CreateHandler<LayoutHandler>(layout);
        viewReference = new WeakReference(editor);
        handlerReference = new WeakReference(editor.Handler);
        platformViewReference = new WeakReference(editor.Handler.PlatformView);
    });

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

I will create a similar PR for `Entry` as well.
rmarinho pushed a commit that referenced this pull request Jul 26, 2023
Context: #16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiTextView.cs(37,30): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiTextView.cs(12,20): error MA0002: Member '_placeholderLabel' 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 editor = new Editor();
        layout.Add(editor);
        var handler = CreateHandler<LayoutHandler>(layout);
        viewReference = new WeakReference(editor);
        handlerReference = new WeakReference(editor.Handler);
        platformViewReference = new WeakReference(editor.Handler.PlatformView);
    });

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

I will create a similar PR for `Entry` as well.
rmarinho pushed a commit that referenced this pull request Jul 26, 2023
Context: #16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiTextField.cs(69,32): error MA0001: Event 'SelectionChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiTextField.cs(68,30): error MA0001: Event 'TextPropertySet' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.

I could reproduce a leak in a test such as:

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

    await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
    Assert.False(viewReference.IsAlive, "Entry should not be alive!");
    Assert.False(handlerReference.IsAlive, "Handler should not be alive!");
    Assert.False(platformViewReference.IsAlive, "PlatformView should not be alive!");
@jonathanpeppers
Copy link
Member Author

Down to 27 for iOS now:

src/Core/src/Platform/iOS/ContainerViewController.cs(11,11): error MA0002: Member 'currentPlatformView' 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/ContainerViewController.cs(16,11): error MA0002: Member '_pendingLoadedView' 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/MauiPicker.cs(21,24): error MA0002: Member 'UIPickerView' 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.
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/MauiCheckBox.cs(27,24): error MA0002: Member 'CheckedChanged' 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/MauiDoneAccessoryView.cs(11,19): error MA0002: Member '_doneClicked' 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/MauiSearchBar.cs(36,63): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiSearchBar.cs(55,32): error MA0001: Event 'EditingChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiSearchBar.cs(54,32): error MA0001: Event 'OnMovedToWindow' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiImageView.cs(34,30): error MA0001: Event 'WindowChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiTimePicker.cs(57,30): error MA0001: Event 'DateSelected' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiTimePicker.cs(13,19): error MA0002: Member '_dateSelected' 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/MauiTimePicker.cs(10,25): error MA0002: Member '_picker' 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/ResignFirstResponderTouchGestureRecognizer.cs(9,11): error MA0002: Member '_targetView' 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/ResignFirstResponderTouchGestureRecognizer.cs(10,10): error MA0002: Member '_token' 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/MauiSwipeView.cs(18,26): error MA0002: Member '_tapGestureRecognizer' 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/MauiSwipeView.cs(20,10): 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/MauiSwipeView.cs(19,26): error MA0002: Member '_panGestureRecognizer' 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/MauiSwipeView.cs(21,15): error MA0002: Member '_actionView' 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/PlatformTouchGraphicsView.cs(12,29): error MA0002: Member '_hoverGesture' 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/WindowOverlay/WindowOverlay.iOS.cs(92,40): error MA0001: Event 'OnTouch' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiWKWebView.cs(15,25): error MA0002: Member 'SharedPool' 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/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.iOS.cs(10,30): error MA0001: Event 'FrameChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiDatePicker.cs(37,27): error MA0003: Subscribing to events with instance method 'OnValueChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.
src/Core/src/Platform/iOS/MauiSearchBar.cs(70,31): error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Jul 26, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiDoneAccessoryView.cs(11,19): error MA0002: Member '_doneClicked' 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.

After writing a test for it, I found a pattern the analyze doesn't
currently catch:

    public MauiDoneAccessoryView(Action doneClicked)
    {
        //...
        var doneButton = new UIBarButtonItem(UIBarButtonSystemItem.Done, OnClicked);
        SetItems(new[] { spacer, doneButton }, false);
    }

This creates a cycle:

* `MauiDoneAccessoryView` -> `UIBarButtonItem` via list of items

* `UIBarButtonItem` -> `Action` -> `MauiDoneAccessoryView` via `OnClicked`

`MauiDoneAccessoryView` lives forever, as well as the owners of any
`Action` delegate values passed in.

I could resolve these problems by creating a new non-NSObject
`BarButtonItemProxy` type to handle events.

This solves one leak with the following controls that use
`MauiDoneAccessoryView`:

* `Editor`
* `Picker`
* `DatePicker`
* `TimePicker`

However, I'll need to send future PRs to verify all these controls are
100% leak free.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Jul 26, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiSearchBar.cs(36,63): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(54,32): error MA0001: Event 'OnMovedToWindow' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(55,32): error MA0001: Event 'EditingChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(70,31): error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

I could reproduce a leak in a test such as:

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

    await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
    Assert.False(viewReference.IsAlive, "SearchBar 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 `MauiSearchBarProxy` class.

I found & fixed a couple related issues:

* `SearchBarExtensions.GetSearchTextField()` would have always thrown
  `StackOverlowException` if iOS was less than 13? It just called itself?!?

* Removed `MauiSearchBar.EditingChanged`, as we could subscribe to the
  event from the `UITextField` directly instead.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Jul 26, 2023
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.
rmarinho pushed a commit that referenced this pull request Jul 26, 2023
Context: #16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiDoneAccessoryView.cs(11,19): error MA0002: Member '_doneClicked' 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.

After writing a test for it, I found a pattern the analyze doesn't
currently catch:

    public MauiDoneAccessoryView(Action doneClicked)
    {
        //...
        var doneButton = new UIBarButtonItem(UIBarButtonSystemItem.Done, OnClicked);
        SetItems(new[] { spacer, doneButton }, false);
    }

This creates a cycle:

* `MauiDoneAccessoryView` -> `UIBarButtonItem` via list of items

* `UIBarButtonItem` -> `Action` -> `MauiDoneAccessoryView` via `OnClicked`

`MauiDoneAccessoryView` lives forever, as well as the owners of any
`Action` delegate values passed in.

I could resolve these problems by creating a new non-NSObject
`BarButtonItemProxy` type to handle events.

This solves one leak with the following controls that use
`MauiDoneAccessoryView`:

* `Editor`
* `Picker`
* `DatePicker`
* `TimePicker`

However, I'll need to send future PRs to verify all these controls are
100% leak free.
rmarinho pushed a commit that referenced this pull request Jul 27, 2023
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.
@jonathanpeppers
Copy link
Member Author

Down to 20:

src/Core/src/WindowOverlay/WindowOverlay.iOS.cs(92,40): error MA0001: Event 'OnTouch' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.iOS.cs(10,30): error MA0001: Event 'FrameChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/ResignFirstResponderTouchGestureRecognizer.cs(9,11): error MA0002: Member '_targetView' 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/ResignFirstResponderTouchGestureRecognizer.cs(10,10): error MA0002: Member '_token' 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/MauiTimePicker.cs(10,25): error MA0002: Member '_picker' 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/MauiTimePicker.cs(13,19): error MA0002: Member '_dateSelected' 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/MauiTimePicker.cs(57,30): error MA0001: Event 'DateSelected' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiPicker.cs(21,24): error MA0002: Member 'UIPickerView' 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/ContainerViewController.cs(11,11): error MA0002: Member 'currentPlatformView' 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/ContainerViewController.cs(16,11): error MA0002: Member '_pendingLoadedView' 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/MauiSearchBar.cs(36,63): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiSearchBar.cs(54,32): error MA0001: Event 'OnMovedToWindow' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/MauiSearchBar.cs(55,32): error MA0001: Event 'EditingChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
src/Core/src/Platform/iOS/PlatformTouchGraphicsView.cs(12,29): error MA0002: Member '_hoverGesture' 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/MauiSwipeView.cs(20,10): 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/MauiSwipeView.cs(21,15): error MA0002: Member '_actionView' 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/MauiSwipeView.cs(18,26): error MA0002: Member '_tapGestureRecognizer' 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/MauiSwipeView.cs(19,26): error MA0002: Member '_panGestureRecognizer' 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/MauiDatePicker.cs(37,27): error MA0003: Subscribing to events with instance method 'OnValueChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.
src/Core/src/Platform/iOS/MauiSearchBar.cs(70,31): error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

PRs are already out for TimePicker and SearchBar, which should get it down to 14.

@Eilon Eilon added the legacy-area-perf Startup / Runtime performance label Aug 1, 2023
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Aug 3, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiDatePicker.cs(37,27): error MA0003: Subscribing to events with instance method 'OnValueChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

I could reproduce a leak in `MemoryTests.cs`:

    ++[InlineData(typeof(DatePicker))]
    public async Task HandlerDoesNotLeak(Type type)

Solved the problem by fixing two places:

* `MauiDatePicker` now uses a `UIDatePickerProxy` type, for iOS

* `DatePickerHandler.MacCatalyst.cs` now uses a `UIDatePickerProxy` type,
  for MacCatalyst.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Aug 3, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiSearchBar.cs(36,63): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(54,32): error MA0001: Event 'OnMovedToWindow' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(55,32): error MA0001: Event 'EditingChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(70,31): error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

I could reproduce a leak in a test such as:

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

    await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
    Assert.False(viewReference.IsAlive, "SearchBar 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 `MauiSearchBarProxy` class.

I found & fixed a couple related issues:

* `SearchBarExtensions.GetSearchTextField()` would have always thrown
  `StackOverlowException` if iOS was less than 13? It just called itself?!?

* Removed `MauiSearchBar.EditingChanged`, as we could subscribe to the
  event from the `UITextField` directly instead.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Aug 3, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiSwipeView.cs(20,10): 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/MauiSwipeView.cs(21,15): error MA0002: Member '_actionView' 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/MauiSwipeView.cs(18,26): error MA0002: Member '_tapGestureRecognizer' 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/MauiSwipeView.cs(19,26): error MA0002: Member '_panGestureRecognizer' 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 `MemoryTests.cs`:

    ++[InlineData(typeof(SwipeView))]
    public async Task HandlerDoesNotLeak(Type type)

Solved the problem by:

* Remove the backing field for `MauiSwipeView.Element`. We can use
  `CrossPlatformLayout`, which is already safe, and just cast to
  `ISwipeView` instead.

* Create a `SwipeRecognizerProxy` type for handling callbacks from
  gesture recognizers.

Other cleanup:

* Marked fields `readonly` as Roslyn suggested.

* Fixed a place `.Count() > 0` was used where Roslyn suggested to use
  `.Any()` instead.

* `GetIsVisible()` can also be `static` to avoid a closure on `this`:

    Any(s => this.GetIsVisible(s))
jonathanpeppers added a commit that referenced this pull request Aug 4, 2023
Context: #16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiSwipeView.cs(20,10): 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/MauiSwipeView.cs(21,15): error MA0002: Member '_actionView' 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/MauiSwipeView.cs(18,26): error MA0002: Member '_tapGestureRecognizer' 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/MauiSwipeView.cs(19,26): error MA0002: Member '_panGestureRecognizer' 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 `MemoryTests.cs`:

    ++[InlineData(typeof(SwipeView))]
    public async Task HandlerDoesNotLeak(Type type)

Solved the problem by:

* Remove the backing field for `MauiSwipeView.Element`. We can use
  `CrossPlatformLayout`, which is already safe, and just cast to
  `ISwipeView` instead.

* Create a `SwipeRecognizerProxy` type for handling callbacks from
  gesture recognizers.

Other cleanup:

* Marked fields `readonly` as Roslyn suggested.

* Fixed a place `.Count() > 0` was used where Roslyn suggested to use
  `.Any()` instead.

* `GetIsVisible()` can also be `static` to avoid a closure on `this`:

    Any(s => this.GetIsVisible(s))
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Aug 10, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiPicker.cs(21,24): error MA0002: Member 'UIPickerView' 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 `MemoryTests.cs`:

    ++[InlineData(typeof(Picker))]
    public async Task HandlerDoesNotLeak(Type type)

Solved the problem by:

* Introduce `MauiPickerProxy` for all event subscriptions. Same pattern
  as in other PRs.

* The `ShouldBeginEditing` event was originally subscribed in
  `CreatePlatformView()` and never unsubscribed. I moved to be
  subscribed/unsubscribed the same way as the other events.

* Refactored the `CreateAlert()` method to be `DisplayAlert()` instead.
  This allows the handler to do all the work -- and the
  `MauiPickerProxy` type can call a method on `PickerHandler` once.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Aug 11, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/WindowOverlay/WindowOverlay.iOS.cs(92,40): error MA0001: Event 'OnTouch' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.

I could reproduce a leak by writing a custom test.

Solved the problem by:

* Removed the `PassthroughView.OnTouch` event completely. We could just
  call `WindowOverlay.OnTappedInternal()` directly instead of going
  through an intermediate event.

* `WindowOverlay overlay` is now a `WeakReference<WindowOverlay>`.
rmarinho pushed a commit that referenced this pull request Aug 14, 2023
Context: #16346

This addresses the memory leak discovered by:

    src/Core/src/WindowOverlay/WindowOverlay.iOS.cs(92,40): error MA0001: Event 'OnTouch' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.

I could reproduce a leak by writing a custom test.

Solved the problem by:

* Removed the `PassthroughView.OnTouch` event completely. We could just
  call `WindowOverlay.OnTappedInternal()` directly instead of going
  through an intermediate event.

* `WindowOverlay overlay` is now a `WeakReference<WindowOverlay>`.
jonathanpeppers added a commit that referenced this pull request Aug 15, 2023
* Add swipe for uitests

* Bump the androidx group with 1 update (#16474)

Bumps the androidx group with 1 update: [Xamarin.AndroidX.RecyclerView](https://github.com/xamarin/AndroidX).

- [Commits](https://github.com/xamarin/AndroidX/commits)

---
updated-dependencies:
- dependency-name: Xamarin.AndroidX.RecyclerView
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: androidx
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Document Element (#16299)

* Begin doc progress

* Document element

* More explicit interface implementations

* Address PR feedback

* Document interfaces + delete xml

* whitespace

* Prevent compile issues

* Document IElementController (and mark as obsolete)

* Remove Obsolete attribute from IElementController

This would break the build in various places - and would go out of scope of the PR

* Document IVusalElementController

Apparently is only for internal use. We can't make it private so the only choice is to mark everything as for internal use only

* Prevent mdoc from breaking

You can't really cref a namespace

* Fixed other crefs

* Address Dave's feedback

* Move powershell pack script into cake (#16588)

* 'pwsh' is broken on net8

PowerShell/PowerShell#19679

* Remove the script

* oops

* Fix Graphics Font comparison method (#15985)

* Fix the issue

* Added more tests

* Updated Impl

* [WinUI] Implement PointerPressed and PointerReleased (#16213)

* Implement on Windows and add unit tests

* Update src/Controls/src/Core/PointerGestureRecognizer.cs

Co-authored-by: Samantha Houts <samhouts@users.noreply.github.com>

---------

Co-authored-by: Samantha Houts <samhouts@users.noreply.github.com>

* [X] Optimize OnPlatform element syntax (#5611)

* [X] Optimize OnPlatform element syntax

- fixes #5583

* Auto-format source code

* don't generate x:Name for removed OnPlatforms

* Update src/Controls/src/SourceGen/Controls.SourceGen.csproj

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>

* fix string comparison

---------


Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>

* [Housekeeping] Add 8.0 preview 7 (#16613)

also adds a checkbox to make it clear if this is a regression or not

* [X] Support DynResources as AppThemeBinding values (#16597)

Because, you know, why not ? It could be useful

- fixes #13619

* [main] Update dependencies from dotnet/xharness (#16600)

* Update dependencies from https://github.com/dotnet/xharness build 20230807.2

Microsoft.DotNet.XHarness.CLI , Microsoft.DotNet.XHarness.TestRunners.Common , Microsoft.DotNet.XHarness.TestRunners.Xunit
 From Version 8.0.0-prerelease.23403.1 -> To Version 8.0.0-prerelease.23407.2

* Run device tests on 16.4

* Force to 16.4

* Skip test that fails on iOS

---------

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Rui Marinho <me@ruimarinho.net>

* Adds prompts when creating a new page. (#16331)

* Adds prompts when creating a new page.

* Updates description as per suggested PR comments

* [ios] fix memory leak in GraphicsView (#16605)

Context: #16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/PlatformTouchGraphicsView.cs(12,29): error MA0002: Member '_hoverGesture' 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 `MemoryTests.cs`:

    ++[InlineData(typeof(GraphicsView))]
    public async Task HandlerDoesNotLeak(Type type)

Solved the problem by fixing two places:

* `PlatformTouchGraphicsView` now stores the `IGraphicsView` as a
  `WeakReference`.

* A `UIHoverGestureRecognizerProxy` is used for callbacks to the
  `UIHoverGestureRecognizer`.

* [create-pull-request] automated change (#16592)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Setting binding on Label doesn't set text to incoming binding (#16410)

* Test demonstrating binding issue

* [C] SetBinding overrides SetValue, but only when the value is set

* - reenable tests

---------

Co-authored-by: Stephane Delcroix (HE/HIM) (from Dev Box) <stdelc@microsoft.com>

* Added DeviceTest to validate CharacterSpacing on iOS Button (#16603)

* Use runtime check for setting WKWebView inspectable flag (#16631)

* Use runtime check

* Remove private

* Add back line

* Line endings?

* Use System.OperatingSystem

* Update src/BlazorWebView/src/Maui/iOS/BlazorWebViewHandler.iOS.cs

Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>

---------

Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>

* [Housekeeping] Fix broken bug template (#16643)

* [Housekeeping] Fix broken bug template

checkbox <> checkboxes

* Update bug-report.yml

* Update bug-report.yml

* Implement Additional PointerGestureRecognizer Platform Arguments (#16426)

* Pointer Platform Event Args

* remove extension and use property instead

* make ctor internal

* nullable consistency, spelling, and private setter

* Public API changes and remove nullable windows routedargs

* uitests part 1

* only run ui tests on windows and mac

* move device conditional to test

* Change the tests to Pointer Tests for more reusability

* Add documentation

* Add extension method for ignoring platforms

* Update the documentation

* Change platform specific docs

* [C] fix Specificity for VSM (#16404)

* [C] fix Specificity for VSM

- fixes #11204

* remove skipped test

* Prevent Android timer from adding multiple callbacks; (#16078)

Fixes #10257

* [Housekeeping] Remove checkboxes from bug template (#16650)

I did not realize that they would be editable tasks on the issue. Replaced with a dropdown. Also reordered the versions to be the most likely default options to speed up the reporting process.

* [Android] Fix SwipeView not swiping using Label as direct content (#14824)

* Fix Android SwipeView not swiping using Label as Content

* Refactoring code

* Added sample to validate 6154

---------

Co-authored-by: Javier Suárez <6755973+jsuarezruiz@users.noreply.github.com>

* Locate MAUI View for PlatformView (#16463)

* Locate xplat view from platformview

* - dispatcher

* - fix layout comparison on xunit

* - PR review comments

* - tizen fix

* Bump the aspnetcore group with 7 updates (#16634)

Bumps the aspnetcore group with 7 updates:

| Package | Update |
| --- | --- |
| [Microsoft.AspNetCore.Authorization](https://github.com/dotnet/aspnetcore) | 7.0.9 to 7.0.10 |
| [Microsoft.AspNetCore.Components.WebView](https://github.com/dotnet/aspnetcore) | 7.0.9 to 7.0.10 |
| [Microsoft.JSInterop](https://github.com/dotnet/aspnetcore) | 7.0.9 to 7.0.10 |
| [Microsoft.AspNetCore.Components.Web](https://github.com/dotnet/aspnetcore) | 7.0.9 to 7.0.10 |
| [Microsoft.AspNetCore.Authentication.Facebook](https://github.com/dotnet/aspnetcore) | 7.0.9 to 7.0.10 |
| [Microsoft.AspNetCore.Authentication.Google](https://github.com/dotnet/aspnetcore) | 7.0.9 to 7.0.10 |
| [Microsoft.AspNetCore.Authentication.MicrosoftAccount](https://github.com/dotnet/aspnetcore) | 7.0.9 to 7.0.10 |


Updates `Microsoft.AspNetCore.Authorization` from 7.0.9 to 7.0.10
- [Release notes](https://github.com/dotnet/aspnetcore/releases)
- [Changelog](https://github.com/dotnet/aspnetcore/blob/main/docs/ReleasePlanning.md)
- [Commits](dotnet/aspnetcore@v7.0.9...v7.0.10)

Updates `Microsoft.AspNetCore.Components.WebView` from 7.0.9 to 7.0.10
- [Release notes](https://github.com/dotnet/aspnetcore/releases)
- [Changelog](https://github.com/dotnet/aspnetcore/blob/main/docs/ReleasePlanning.md)
- [Commits](dotnet/aspnetcore@v7.0.9...v7.0.10)

Updates `Microsoft.JSInterop` from 7.0.9 to 7.0.10
- [Release notes](https://github.com/dotnet/aspnetcore/releases)
- [Changelog](https://github.com/dotnet/aspnetcore/blob/main/docs/ReleasePlanning.md)
- [Commits](dotnet/aspnetcore@v7.0.9...v7.0.10)

Updates `Microsoft.AspNetCore.Components.Web` from 7.0.9 to 7.0.10
- [Release notes](https://github.com/dotnet/aspnetcore/releases)
- [Changelog](https://github.com/dotnet/aspnetcore/blob/main/docs/ReleasePlanning.md)
- [Commits](dotnet/aspnetcore@v7.0.9...v7.0.10)

Updates `Microsoft.AspNetCore.Authentication.Facebook` from 7.0.9 to 7.0.10
- [Release notes](https://github.com/dotnet/aspnetcore/releases)
- [Changelog](https://github.com/dotnet/aspnetcore/blob/main/docs/ReleasePlanning.md)
- [Commits](dotnet/aspnetcore@v7.0.9...v7.0.10)

Updates `Microsoft.AspNetCore.Authentication.Google` from 7.0.9 to 7.0.10
- [Release notes](https://github.com/dotnet/aspnetcore/releases)
- [Changelog](https://github.com/dotnet/aspnetcore/blob/main/docs/ReleasePlanning.md)
- [Commits](dotnet/aspnetcore@v7.0.9...v7.0.10)

Updates `Microsoft.AspNetCore.Authentication.MicrosoftAccount` from 7.0.9 to 7.0.10
- [Release notes](https://github.com/dotnet/aspnetcore/releases)
- [Changelog](https://github.com/dotnet/aspnetcore/blob/main/docs/ReleasePlanning.md)
- [Commits](dotnet/aspnetcore@v7.0.9...v7.0.10)

---
updated-dependencies:
- dependency-name: Microsoft.AspNetCore.Authorization
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: aspnetcore
- dependency-name: Microsoft.AspNetCore.Components.WebView
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: aspnetcore
- dependency-name: Microsoft.JSInterop
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: aspnetcore
- dependency-name: Microsoft.AspNetCore.Components.Web
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: aspnetcore
- dependency-name: Microsoft.AspNetCore.Authentication.Facebook
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: aspnetcore
- dependency-name: Microsoft.AspNetCore.Authentication.Google
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: aspnetcore
- dependency-name: Microsoft.AspNetCore.Authentication.MicrosoftAccount
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: aspnetcore
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* [create-pull-request] automated change (#16655)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* [ios] fix memory leak in SwipeItem (#16614)

Context: #16346

This addresses the memory leak discovered by:

    src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.iOS.cs(10,30): error MA0001: Event 'FrameChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.

I could reproduce the leak with a custom test in `SwipeViewTests`.

Solved the problem by introducing `SwipeItemButtonProxy` with the same
pattern from other PRs.

* [ios/catalyst] fix memory leak in DatePicker (#16589)

Context: #16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiDatePicker.cs(37,27): error MA0003: Subscribing to events with instance method 'OnValueChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

I could reproduce a leak in `MemoryTests.cs`:

    ++[InlineData(typeof(DatePicker))]
    public async Task HandlerDoesNotLeak(Type type)

Solved the problem by fixing two places:

* `MauiDatePicker` now uses a `UIDatePickerProxy` type, for iOS

* `DatePickerHandler.MacCatalyst.cs` now uses a `UIDatePickerProxy` type,
  for MacCatalyst.

Other changes:

* Skip test on Android API 23

This is working for me locally on an API 23 emulator -- so I don't think
it is really leaking.

We skipped API 23 in another place:

https://github.com/dotnet/maui/blob/9fcccd57b5a3d664e4788b3c7fc3edc10010beaf/src/Controls/tests/DeviceTests/Elements/NavigationPage/NavigationPageTests.cs#L302-L303

We are more interested in iOS on this PR anyway.

* [build] Bump XCode to 14.31 (#16374)

* Add doc comments for common types used in templates, maps, webview (#16552)

* Add doc comments for common types used in templates, maps, webview
* Enable WarnigsAsErrors for inline docs on Maps

I went through every type/method/etc. used in the default MAUI app template and ensured there are doc for all of them.

I also added a few miscellaneous docs for WebView types and Map (many were copied from Xamarin.Forms docs).
---------
Co-authored-by: Gerald Versluis <gerald.versluis@microsoft.com>

* Update CODEOWNERS (#16682)

* Fix the case where silent was requested (#16676)

* Use the correct WASDK property (#16683)

* Re-land "[Windows] fixing window glitches while moving or resizing"  (#16637)

Initially merged in #14861 but there was a few test failures due to 83398c3 so it was reverted in #16541

* Update dependencies from https://github.com/xamarin/xamarin-macios build 20230815.5

Microsoft.tvOS.Sdk
 From Version 16.4.8694-net8-p7 -> To Version 16.4.8751-net8-rc1

* Update dependencies from https://github.com/xamarin/xamarin-macios build 20230815.5

Microsoft.MacCatalyst.Sdk
 From Version 16.4.8694-net8-p7 -> To Version 16.4.8751-net8-rc1

* Update dependencies from https://github.com/xamarin/xamarin-macios build 20230815.5

Microsoft.macOS.Sdk
 From Version 13.3.8694-net8-p7 -> To Version 13.3.8751-net8-rc1

* Update dependencies from https://github.com/xamarin/xamarin-macios build 20230815.5

Microsoft.iOS.Sdk
 From Version 16.4.8694-net8-p7 -> To Version 16.4.8751-net8-rc1

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Scott Banning <scoban@microsoft.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Juan Diego Herrera <juherrera@microsoft.com>
Co-authored-by: Matthew Leibowitz <mattleibow@live.com>
Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
Co-authored-by: Rachel Kang <rachelkang@microsoft.com>
Co-authored-by: Samantha Houts <samhouts@users.noreply.github.com>
Co-authored-by: Stephane Delcroix <stephane@delcroix.org>
Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Rui Marinho <me@ruimarinho.net>
Co-authored-by: Mausam Shrestha <46900712+mausam-shrestha@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
Co-authored-by: Stephane Delcroix (HE/HIM) (from Dev Box) <stdelc@microsoft.com>
Co-authored-by: Tim Miller <drasticactions@users.noreply.github.com>
Co-authored-by: Rolf Bjarne Kvinge <rolf@xamarin.com>
Co-authored-by: TJ Lambert <50846373+tj-devel709@users.noreply.github.com>
Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com>
Co-authored-by: Javier Suárez <6755973+jsuarezruiz@users.noreply.github.com>
Co-authored-by: scoban <sbanni@users.noreply.github.com>
Co-authored-by: Eilon Lipton <Eilon@users.noreply.github.com>
jfversluis pushed a commit that referenced this pull request Aug 15, 2023
Context: #16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiPicker.cs(21,24): error MA0002: Member 'UIPickerView' 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 `MemoryTests.cs`:

    ++[InlineData(typeof(Picker))]
    public async Task HandlerDoesNotLeak(Type type)

Solved the problem by:

* Introduce `MauiPickerProxy` for all event subscriptions. Same pattern
  as in other PRs.

* The `ShouldBeginEditing` event was originally subscribed in
  `CreatePlatformView()` and never unsubscribed. I moved to be
  subscribed/unsubscribed the same way as the other events.

* Refactored the `CreateAlert()` method to be `DisplayAlert()` instead.
  This allows the handler to do all the work -- and the
  `MauiPickerProxy` type can call a method on `PickerHandler` once.
Context: https://github.com/jonathanpeppers/memory-analyzers
Context: https://www.nuget.org/packages/MemoryAnalyzers/0.1.0-beta.3

This adds a new Roslyn analyzer that warns about the following cases.

 ## MA0001

Don't define `public` events in `NSObject` subclasses:

```csharp
public class MyView : UIView
{
    // NOPE!
    public event EventHandler MyEvent;
}
```

 ## MA0002

Don't declare members in `NSObject` subclasses unless they are:

* `WeakReference` or `WeakReference<T>`
* Value types

```csharp
class MyView : UIView
{
    // NOPE!
    public UIView? Parent { get; set; }

    public void Add(MyView subview)
    {
        subview.Parent = this;
        AddSubview(subview);
    }
}
```

 ## MA0003

Don't subscribe to events inside `NSObject` subclasses unless:

* It's your event via `this.MyEvent` or from a base type.
* The method is `static`.

```csharp
class MyView : UIView
{
    public MyView()
    {
        var picker = new UIDatePicker();
        AddSubview(picker);
        picker.ValueChanged += OnValueChanged;
    }

    void OnValueChanged(object sender, EventArgs e) { }

    // Use this instead and it doesn't leak!
    //static void OnValueChanged(object sender, EventArgs e) { }
}
```

This is also on NuGet, but I just commited the package until we can get
it added to the `dotnet-public` feed.

Places with PRs in flight are marked with:

    [UnconditionalSuppressMessage("Memory", "MA0002", Justification = "FIXME: dotnet#16530")]

A few are marked as "not an issue" with an explanation. Others mention a
test with a proof they are OK.

A few places I could actually *remove* `UnconditionalSuppressMessage`
where I could improve the analyzer to ignore that case.
@jonathanpeppers
Copy link
Member Author

Enabling the analyzer should help catch future leaks. Any that have open PRs I ignored with a link to the PR for now.

@jonathanpeppers jonathanpeppers marked this pull request as ready for review August 17, 2023 00:17
@jonathanpeppers jonathanpeppers requested a review from a team as a code owner August 17, 2023 00:17
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Aug 17, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiSearchBar.cs(36,63): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(54,32): error MA0001: Event 'OnMovedToWindow' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(55,32): error MA0001: Event 'EditingChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(70,31): error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

I could reproduce a leak in a test such as:

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

    await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
    Assert.False(viewReference.IsAlive, "SearchBar 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 `MauiSearchBarProxy` class.

I found & fixed a couple related issues:

* `SearchBarExtensions.GetSearchTextField()` would have always thrown
  `StackOverlowException` if iOS was less than 13? It just called itself?!?

* Removed `MauiSearchBar.EditingChanged`, as we could subscribe to the
  event from the `UITextField` directly instead.
rmarinho pushed a commit that referenced this pull request Aug 19, 2023
Context: #16346

This addresses the memory leak discovered by:

    src/Core/src/WindowOverlay/WindowOverlay.iOS.cs(92,40): error MA0001: Event 'OnTouch' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.

I could reproduce a leak by writing a custom test.

Solved the problem by:

* Removed the `PassthroughView.OnTouch` event completely. We could just
  call `WindowOverlay.OnTappedInternal()` directly instead of going
  through an intermediate event.

* `WindowOverlay overlay` is now a `WeakReference<WindowOverlay>`.
rmarinho pushed a commit that referenced this pull request Aug 19, 2023
Context: #16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiPicker.cs(21,24): error MA0002: Member 'UIPickerView' 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 `MemoryTests.cs`:

    ++[InlineData(typeof(Picker))]
    public async Task HandlerDoesNotLeak(Type type)

Solved the problem by:

* Introduce `MauiPickerProxy` for all event subscriptions. Same pattern
  as in other PRs.

* The `ShouldBeginEditing` event was originally subscribed in
  `CreatePlatformView()` and never unsubscribed. I moved to be
  subscribed/unsubscribed the same way as the other events.

* Refactored the `CreateAlert()` method to be `DisplayAlert()` instead.
  This allows the handler to do all the work -- and the
  `MauiPickerProxy` type can call a method on `PickerHandler` once.
@rmarinho rmarinho merged commit 2e65626 into dotnet:main Aug 21, 2023
39 checks passed
@jonathanpeppers jonathanpeppers deleted the test-memory-analyzers branch August 21, 2023 15:59
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Aug 25, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiSearchBar.cs(36,63): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(54,32): error MA0001: Event 'OnMovedToWindow' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(55,32): error MA0001: Event 'EditingChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(70,31): error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

I could reproduce a leak in a test such as:

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

    await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
    Assert.False(viewReference.IsAlive, "SearchBar 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 `MauiSearchBarProxy` class.

I found & fixed a couple related issues:

* `SearchBarExtensions.GetSearchTextField()` would have always thrown
  `StackOverlowException` if iOS was less than 13? It just called itself?!?

* Removed `MauiSearchBar.EditingChanged`, as we could subscribe to the
  event from the `UITextField` directly instead.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Aug 28, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiSearchBar.cs(36,63): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(54,32): error MA0001: Event 'OnMovedToWindow' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(55,32): error MA0001: Event 'EditingChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(70,31): error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

I could reproduce a leak in a test such as:

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

    await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
    Assert.False(viewReference.IsAlive, "SearchBar 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 `MauiSearchBarProxy` class.

I found & fixed a couple related issues:

* `SearchBarExtensions.GetSearchTextField()` would have always thrown
  `StackOverlowException` if iOS was less than 13? It just called itself?!?

* Removed `MauiSearchBar.EditingChanged`, as we could subscribe to the
  event from the `UITextField` directly instead.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Nov 9, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiSearchBar.cs(36,63): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(54,32): error MA0001: Event 'OnMovedToWindow' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(55,32): error MA0001: Event 'EditingChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(70,31): error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

I could reproduce a leak in a test such as:

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

    await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
    Assert.False(viewReference.IsAlive, "SearchBar 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 `MauiSearchBarProxy` class.

I found & fixed a couple related issues:

* `SearchBarExtensions.GetSearchTextField()` would have always thrown
  `StackOverlowException` if iOS was less than 13? It just called itself?!?

* Removed `MauiSearchBar.EditingChanged`, as we could subscribe to the
  event from the `UITextField` directly instead.
jonathanpeppers added a commit to jonathanpeppers/maui that referenced this pull request Nov 10, 2023
Context: dotnet#16346

This addresses the memory leak discovered by:

    src/Core/src/Platform/iOS/MauiSearchBar.cs(36,63): error MA0001: Event 'TextSetOrChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(54,32): error MA0001: Event 'OnMovedToWindow' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(55,32): error MA0001: Event 'EditingChanged' could cause memory leaks in an NSObject subclass. Remove the event or add the [UnconditionalSuppressMessage("Memory", "MA0001")] attribute with a justification as to why the event will not leak.
    src/Core/src/Platform/iOS/MauiSearchBar.cs(70,31): error MA0003: Subscribing to events with instance method 'OnEditingChanged' could cause memory leaks in an NSObject subclass. Remove the subscription or convert the method to a static method.

I could reproduce a leak in a test such as:

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

    await AssertionExtensions.WaitForGC(viewReference, handlerReference, platformViewReference);
    Assert.False(viewReference.IsAlive, "SearchBar 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 `MauiSearchBarProxy` class.

I found & fixed a couple related issues:

* `SearchBarExtensions.GetSearchTextField()` would have always thrown
  `StackOverlowException` if iOS was less than 13? It just called itself?!?

* Removed `MauiSearchBar.EditingChanged`, as we could subscribe to the
  event from the `UITextField` directly instead.
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
@Eilon Eilon added the t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) label May 10, 2024
@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 legacy-area-perf Startup / Runtime performance memory-leak 💦 Memory usage grows / objects live forever platform/iOS 🍎 t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants