Many controls (mainly iOS) are not collected by GC #18365

heyThorsten · 2023-10-26T01:31:42Z

XamlTest · 2023-10-27T07:28:29Z

Verified this on Visual Studio Enterprise 17.8.0 Preview 5.0(8.0.0-rc.2.9373). Repro on Windows 11, Android 14.0-API34 and iOS 16.4 with below Project:
GCTest.zip

jonmdev · 2023-10-27T08:27:42Z

Wow good catch. 👍

borrmann · 2023-10-27T14:22:42Z

@jonathanpeppers #13520 #14108

jonathanpeppers · 2023-10-27T17:49:27Z

There is a PR open for CarouselView: #18267

Some of these could have also been discovered here: #18318

There is a test we can add additional controls to start validating these:

maui/src/Controls/tests/DeviceTests/Memory/MemoryTests.cs

Lines 39 to 54 in 3c0ff67

    
           [Theory("Handler Does Not Leak")] 
        
           [InlineData(typeof(Border))] 
        
           [InlineData(typeof(ContentView))] 
        
           [InlineData(typeof(CheckBox))] 
        
           [InlineData(typeof(DatePicker))] 
        
           [InlineData(typeof(Entry))] 
        
           [InlineData(typeof(Editor))] 
        
           [InlineData(typeof(GraphicsView))] 
        
           [InlineData(typeof(Image))] 
        
           [InlineData(typeof(Label))] 
        
           [InlineData(typeof(Picker))] 
        
           [InlineData(typeof(RefreshView))] 
        
           [InlineData(typeof(ScrollView))] 
        
           [InlineData(typeof(SwipeView))] 
        
           [InlineData(typeof(TimePicker))] 
        
           public async Task HandlerDoesNotLeak(Type type)

Context: dotnet#18365 I could reproduce a leak in `Frame` by adding a parameterized test: [InlineData(typeof(Frame))] public async Task HandlerDoesNotLeak(Type type) `FrameRenderer` has a circular reference via its base type, `VisualElementRenderer`: * `Frame` -> * `FrameRenderer` / `VisualElementRenderer` -> * `Frame` (via `VisualElementRenderer._virtualView`) To solve this issue, I made `_virtualView` a `WeakReference`, but only on iOS or MacCatalyst platforms. We don't necessarily need this treatment for Windows or Android. My initial attempt threw a `NullReferenceException`, because the `Element` property is accessed before `SetVirtualView()` returns. I had to create a `_tempElement` field to hold the value temporarily, clearing it to avoid a circular reference. The Roslyn analyzer didn't catch this cycle because it doesn't warn about `IPlatformViewHandler`, only `NSObject`'s. Will investigate further on this example here: jonathanpeppers/memory-analyzers#12

Context: dotnet#18365 Adding a parameter to the test: [Theory("Handler Does Not Leak")] [InlineData(typeof(ImageButton))] public async Task HandlerDoesNotLeak(Type type) Shows a memory leak in `ImageButton`, caused by the cycle * `ImageButtonHandler` -> * `UIButton` events like `TouchUpInside` -> * `ImageButtonHandler` I could solve this problem by creating a `ImageButtonProxy` class -- the same pattern I've used in other PRs to avoid cycles. This makes an intermediate type to handle the events and breaks the cycle. Still thinking if the analyzer could have caught this, issue filed at: jonathanpeppers/memory-analyzers#12

Context: dotnet#18365 Adding a parameter to the test: [Theory("Handler Does Not Leak")] [InlineData(typeof(Stepper))] public async Task HandlerDoesNotLeak(Type type) Shows a memory leak in `Stepper`, caused by the cycle * `StepperHandler` -> * `UIStepper.ValueChanged` event -> * `StepperHandler` I could solve this problem by creating a `StepperProxy` class -- the same pattern I've used in other PRs to avoid cycles. This makes an intermediate type to handle the events and breaks the cycle. Still thinking if the analyzer could have caught this, issue filed at: jonathanpeppers/memory-analyzers#12

Context: #18365 Adding a parameter to the test: [Theory("Handler Does Not Leak")] [InlineData(typeof(Stepper))] public async Task HandlerDoesNotLeak(Type type) Shows a memory leak in `Stepper`, caused by the cycle * `StepperHandler` -> * `UIStepper.ValueChanged` event -> * `StepperHandler` I could solve this problem by creating a `StepperProxy` class -- the same pattern I've used in other PRs to avoid cycles. This makes an intermediate type to handle the events and breaks the cycle. Still thinking if the analyzer could have caught this, issue filed at: jonathanpeppers/memory-analyzers#12

* [ios] fix memory leak in `ImageButton` Context: #18365 Adding a parameter to the test: [Theory("Handler Does Not Leak")] [InlineData(typeof(ImageButton))] public async Task HandlerDoesNotLeak(Type type) Shows a memory leak in `ImageButton`, caused by the cycle * `ImageButtonHandler` -> * `UIButton` events like `TouchUpInside` -> * `ImageButtonHandler` I could solve this problem by creating a `ImageButtonProxy` class -- the same pattern I've used in other PRs to avoid cycles. This makes an intermediate type to handle the events and breaks the cycle. Still thinking if the analyzer could have caught this, issue filed at: jonathanpeppers/memory-analyzers#12 * [android] fix memory leak in `ImageButton` Context: a270ebd Context: 1bbe79d Context: material-components/material-components-android#2063 Reviewing a GC dump of the device tests, I noticed a `System.Action` keeping the `ImageButton` alive: Microsoft.Maui.Controls.ImageButton System.Action Java.Lang.Thread.RunnableImplementor So next, I looked for `System.Action` and found the path on the `ReferencedTypes` tab: System.Action Microsoft.Maui.Platform.ImageButtonExtensions.[]c__DisplayClass4_0 Google.Android.Material.ImageView.ShapeableImageView Microsoft.Maui.Controls.ImageButton Which led me to the code: public static async void UpdatePadding(this ShapeableImageView platformButton, IImageButton imageButton) { platformButton.SetContentPadding(imageButton); platformButton.Post(() => { platformButton.SetContentPadding(imageButton); }); platformButton.SetContentPadding(imageButton); } ?!? Why is this code calling `SetContentPadding` three times? Reviewing the commit history: * a270ebd * 1bbe79d * material-components/material-components-android#2063 I could comment out the code and the leak is solved, but I found I could also change the code to use `await Task.Yield()` for the same result.

jonathanpeppers · 2023-11-10T15:58:50Z

Thank you, @heyThorsten, for creating this sample!

I turned the above list of controls into a checklist. Updated which have fixes in dotnet/maui/main or open PRs so far.

Context: dotnet#18365 Adding a parameter to the test: [Theory("Handler Does Not Leak")] [InlineData(typeof(Slider))] public async Task HandlerDoesNotLeak(Type type) Shows a memory leak in `Slider`, caused by the cycle * `SliderHandler` -> * `UISlider` events -> * `SliderHandler` I could solve this problem by creating a `SliderProxy` class -- the same pattern I've used in other PRs to avoid cycles. This makes an intermediate type to handle the events and breaks the cycle. Still thinking if the analyzer could have caught this, issue filed at: jonathanpeppers/memory-analyzers#12

Context: #18365 I could reproduce a leak in `Frame` by adding a parameterized test: [InlineData(typeof(Frame))] public async Task HandlerDoesNotLeak(Type type) `FrameRenderer` has a circular reference via its base type, `VisualElementRenderer`: * `Frame` -> * `FrameRenderer` / `VisualElementRenderer` -> * `Frame` (via `VisualElementRenderer._virtualView`) To solve this issue, I made `_virtualView` a `WeakReference`, but only on iOS or MacCatalyst platforms. We don't necessarily need this treatment for Windows or Android. My initial attempt threw a `NullReferenceException`, because the `Element` property is accessed before `SetVirtualView()` returns. I had to create a `_tempElement` field to hold the value temporarily, clearing it to avoid a circular reference. The Roslyn analyzer didn't catch this cycle because it doesn't warn about `IPlatformViewHandler`, only `NSObject`'s. Will investigate further on this example here: jonathanpeppers/memory-analyzers#12

Context: dotnet#18365 I could reproduce a leak in `TableView` by adding a parameterized test: [InlineData(typeof(TableView))] public async Task HandlerDoesNotLeak(Type type) `TableViewModelRenderer` has two cycles: * `TableView` -> * `TableViewRenderer` -> * `TableViewModelRenderer` -> * `TableView` via `View` field * `UITableView.Source` -> * `TableViewModelRenderer` -> * `UITableView` via `Table` field I left the `Table` and `View` fields in place to not break any public APIs. They are now unused and marked `[Obsolete]`. I used `WeakReference<T>` to solve these two cycles and the test now passes. The analyzer warned about these locations, but we don't have the analyzer enabled on `Microsoft.Maui.Controls` yet: dotnet#18318 (comment)

Context: dotnet#18365 Context: https://github.com/heyThorsten/GCTest Testing the sample app, I could see a leak in `WebView` on Windows. A memory snapshot showed: Microsoft.Maui.Handlers.WebViewHandler Windows.Foundation.TypedEventHandler<Microsoft.UI.Xaml.Controls.WebView2, Microsoft.UI.Xaml.Controls.CoreWebView2InitializedEventArgs> WinRT._EventSource_global__Windows_Foundation_TypedEventHandler_global__Microsoft_UI_Xaml_Controls_WebView2__global__Microsoft_UI_Xaml_Controls_CoreWebView2InitializedEventArgs_+EventState Windows.Foundation.TypedEventHandler<Microsoft.UI.Xaml.Controls.WebView2, Microsoft.UI.Xaml.Controls.CoreWebView2InitializedEventArgs> [Dependent Handle, Microsoft.Maui.Platform.LayoutPanel <0x2079E7A8D00>] Microsoft.UI.Xaml.RoutedEventHandler WinRT._EventSource_global__Microsoft_UI_Xaml_RoutedEventHandler+EventState Microsoft.UI.Xaml.RoutedEventHandler [Dependent Handle, Microsoft.Maui.Platform.LayoutPanel <0x2079E7A8D00>] I could this same leak in `WebView` by updating `MemoryTests` to do: if (view is WebView webView) { webView.Source = new HtmlWebViewSource { Html = "<p>hi</p>" }; await Task.Delay(1000); } After a bit of debugging, I found that events on `WebView` seem to suffer from the same "circular reference" problem we see on iOS. This makes sense, as it is an unmanaged/native control. (Might be COM?) So we have the cycle: * `WebViewHandler` -> * `WebView2` -> * Various `WebView2` events: * `CoreWebView2Initialized` * `HistoryChanged` * `NavigationStarting` * `NavigationCompleted` * `WebViewHandler` Using the same pattern we've been using for iOS: creating a `WebView2Proxy` type solves the issue. Note that I didn't have to do this for all events, just the ones subscribing to `WebView2`-related events. I also don't see the `WebView2Proxy` object living forever, so the issue does appear to be a cycle.

Yokaichan · 2023-11-17T10:49:13Z

Can you add the "Border" item to the checklist ? Because it seems that I met some trouble with this item.

For example, when I use a list of items which are defined into a border (nearly 100 items).. the application crash on some rendering / navigating... but no crash when I remove the border.

I don't have any log with this (sorry).

jonathanpeppers · 2023-11-17T15:28:49Z

the application crash on some rendering / navigating... but no crash when I remove the border.

@Yokaichan the symptoms you are describing here does not necessarily sound like a memory leak. We already have memory tests in-place to be sure Border doesn't leak: #15946

Can you get the full stack trace of the crash and file a new issue?

Context: #18365 I could reproduce a leak in `TableView` by adding a parameterized test: [InlineData(typeof(TableView))] public async Task HandlerDoesNotLeak(Type type) `TableViewModelRenderer` has two cycles: * `TableView` -> * `TableViewRenderer` -> * `TableViewModelRenderer` -> * `TableView` via `View` field * `UITableView.Source` -> * `TableViewModelRenderer` -> * `UITableView` via `Table` field I left the `Table` and `View` fields in place to not break any public APIs. They are now unused and marked `[Obsolete]`. I used `WeakReference<T>` to solve these two cycles and the test now passes. The analyzer warned about these locations, but we don't have the analyzer enabled on `Microsoft.Maui.Controls` yet: #18318 (comment)

Context: #18365 Context: https://github.com/heyThorsten/GCTest Testing the sample app, I could see a leak in `WebView` on Windows. A memory snapshot showed: Microsoft.Maui.Handlers.WebViewHandler Windows.Foundation.TypedEventHandler<Microsoft.UI.Xaml.Controls.WebView2, Microsoft.UI.Xaml.Controls.CoreWebView2InitializedEventArgs> WinRT._EventSource_global__Windows_Foundation_TypedEventHandler_global__Microsoft_UI_Xaml_Controls_WebView2__global__Microsoft_UI_Xaml_Controls_CoreWebView2InitializedEventArgs_+EventState Windows.Foundation.TypedEventHandler<Microsoft.UI.Xaml.Controls.WebView2, Microsoft.UI.Xaml.Controls.CoreWebView2InitializedEventArgs> [Dependent Handle, Microsoft.Maui.Platform.LayoutPanel <0x2079E7A8D00>] Microsoft.UI.Xaml.RoutedEventHandler WinRT._EventSource_global__Microsoft_UI_Xaml_RoutedEventHandler+EventState Microsoft.UI.Xaml.RoutedEventHandler [Dependent Handle, Microsoft.Maui.Platform.LayoutPanel <0x2079E7A8D00>] I could this same leak in `WebView` by updating `MemoryTests` to do: if (view is WebView webView) { webView.Source = new HtmlWebViewSource { Html = "<p>hi</p>" }; await Task.Delay(1000); } After a bit of debugging, I found that events on `WebView` seem to suffer from the same "circular reference" problem we see on iOS. This makes sense, as it is an unmanaged/native control. (Might be COM?) So we have the cycle: * `WebViewHandler` -> * `WebView2` -> * Various `WebView2` events: * `CoreWebView2Initialized` * `HistoryChanged` * `NavigationStarting` * `NavigationCompleted` * `WebViewHandler` Using the same pattern we've been using for iOS: creating a `WebView2Proxy` type solves the issue. Note that I didn't have to do this for all events, just the ones subscribing to `WebView2`-related events. I also don't see the `WebView2Proxy` object living forever, so the issue does appear to be a cycle.

haavamoa · 2024-01-13T23:05:39Z

I just noticed that Border on iOS has memory leak when:

 <Border>
        <Border.StrokeShape>
            <RoundRectangle CornerRadius="8"
                            StrokeThickness="0" />
        </Border.StrokeShape>
    </Border>

Try add that to the unit tests @jonathanpeppers , I've spent all day trying to figure out why this component we've created is leaking. If I remove lines 62-66 it does not leak anymore.

Edit:
I just tried to simplify it even more, it doesn't matter what kind of Shape you give it, it leaks as soon as you set a StrokeShape to whatever shape you'd like.

AdamEssenmacher · 2024-01-15T07:27:31Z

It looks like NavigationPage itself leaks on iOS too.

Easy to reproduce. Just do something like:

Application.Current.MainPage = new NavigationPage(new PageA());
...
Application.Current.MainPage = new PageB();
...
Application.Current.MainPage = new PageC();

You'll find that the NavigationPage isn't collected, but PageB is collected once it is replaced by PageC.

Let me know if you'd rather see another issue opened for this. Thought you might just want to add it to the list.

This one might be particularly nasty if the leak keeps the NavgationPage's entire nav stack in memory.

AdamEssenmacher · 2024-01-15T10:37:35Z

@haavamoa's discovery highlights a weakness in the methods currently at work to detect these leaks.

While a great start, it's not enough to load an empty (or near-empty) control into a test. Each of these controls have broader APIs that, when exercised, might also introduce leaks.

I've cobbled together a detection mechanism that capitalizes on the cascading nature of these leaks (i.e. propagating to the host page) to provide near-immediate feedback when such leaks occur. It centrally tracks page navigation events, forces garbage collection, and then reports on the collection status of the previous page.

The project, inspired by @heyThorsten's in-app testing approach, demonstrates how memory testing techniques from the MAUI wiki can be applied to instrument a MAUI app to actively detect page-level memory leaks during development.

PureWeen · 2024-01-20T00:13:53Z

I've broken the 3 remaining controls for this issue out into separate tracking issues.

@Yokaichan @haavamoa @AdamEssenmacher
Can you log new issues with repros so we can track them alongside this effort?

Thank you for all the work you did on this @heyThorsten, we all really appreciate it!

heyThorsten added the t/bug Something isn't working label Oct 26, 2023

jsuarezruiz added platform/iOS 🍎 legacy-area-perf Startup / Runtime performance labels Oct 26, 2023

XamlTest added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Oct 27, 2023

This comment was marked as off-topic.

Sign in to view

jonathanpeppers added the memory-leak 💦 Memory usage grows / objects live forever (sub: perf) label Oct 27, 2023

PureWeen added this to the .NET 8 SR1 milestone Oct 27, 2023

PureWeen added this to MAUI SDK Ongoing Oct 27, 2023

github-project-automation bot moved this to Todo in MAUI SDK Ongoing Oct 27, 2023

jonathanpeppers self-assigned this Oct 27, 2023

jonathanpeppers mentioned this issue Oct 30, 2023

[ios] fix memory leaks in various controls #18434

Merged

jonathanpeppers moved this from Todo to In Progress in MAUI SDK Ongoing Oct 30, 2023

This was referenced Nov 6, 2023

[ios] fix memory leak in Frame, VisualElementRenderer #18552

Merged

[android] fix memory leak with CarouselView #18584

Merged

jonathanpeppers mentioned this issue Nov 7, 2023

[ios] fix memory leak in ImageButton #18602

Merged

jonathanpeppers mentioned this issue Nov 9, 2023

[ios] fix memory leak in Stepper #18663

Merged

This was referenced Nov 13, 2023

[ios] fix memory leak in TableView #18718

Merged

Add/Remove CarouselView Layout Listener when view is add/removed from window #18771

Merged

jonathanpeppers mentioned this issue Nov 16, 2023

[windows] fix memory leak in WebView #18810

Merged

PureWeen modified the milestones: .NET 8 SR1, .NET 8 SR2 Nov 22, 2023

samhouts added the p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint label Dec 7, 2023

drasticactions mentioned this issue Dec 28, 2023

Many Controls end up preventing Garbage Collection #19606

Closed

jonathanpeppers mentioned this issue Jan 3, 2024

[net7.0 / net8.0] [iOS] Memory leak when the CollectionView uses CollectionChanged and when adding a CollectionView to a Page makes it and the entire page live forever #14654

Closed

2 tasks

Redth modified the milestones: .NET 8 SR2, .NET 8 SR3 Jan 10, 2024

This was referenced Jan 20, 2024

RadioButton is not currently being Garbage Collected #20023

Closed

SearchBar is not currently being Garbage Collected #20024

Closed

ListView is not currently being Garbage Collected #20025

Closed

PureWeen closed this as completed Jan 20, 2024

github-project-automation bot moved this from In Progress to Done in MAUI SDK Ongoing Jan 20, 2024

github-actions bot locked and limited conversation to collaborators Feb 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Many controls (mainly iOS) are not collected by GC #18365

Many controls (mainly iOS) are not collected by GC #18365

heyThorsten commented Oct 26, 2023 •

edited by PureWeen

Loading

XamlTest commented Oct 27, 2023

jonmdev commented Oct 27, 2023

This comment was marked as off-topic.

borrmann commented Oct 27, 2023

jonathanpeppers commented Oct 27, 2023

jonathanpeppers commented Nov 10, 2023

Yokaichan commented Nov 17, 2023

jonathanpeppers commented Nov 17, 2023

haavamoa commented Jan 13, 2024 •

edited

Loading

AdamEssenmacher commented Jan 15, 2024

AdamEssenmacher commented Jan 15, 2024

PureWeen commented Jan 20, 2024

Many controls (mainly iOS) are not collected by GC #18365

Many controls (mainly iOS) are not collected by GC #18365

Comments

heyThorsten commented Oct 26, 2023 • edited by PureWeen Loading

Description

Steps to Reproduce

Link to public reproduction project repository

Version with bug

Is this a regression from previous behavior?

Last version that worked well

Affected platforms

Affected platform versions

Did you find any workaround?

Relevant log output

XamlTest commented Oct 27, 2023

jonmdev commented Oct 27, 2023

This comment was marked as off-topic.

borrmann commented Oct 27, 2023

jonathanpeppers commented Oct 27, 2023

jonathanpeppers commented Nov 10, 2023

Yokaichan commented Nov 17, 2023

jonathanpeppers commented Nov 17, 2023

haavamoa commented Jan 13, 2024 • edited Loading

AdamEssenmacher commented Jan 15, 2024

AdamEssenmacher commented Jan 15, 2024

PureWeen commented Jan 20, 2024

heyThorsten commented Oct 26, 2023 •

edited by PureWeen

Loading

haavamoa commented Jan 13, 2024 •

edited

Loading