Memory Leak issue in ListView #16450

Tamilarasan-Paranthaman · 2023-07-31T13:08:20Z

Description

I have a CheckBox in the ListView DataTemplate and am switching the ListView's ItemsSource at runtime. Due to this, the app size gradually increases and eventually leads to a crash of the application.

checkbox.mp4

Steps to Reproduce

Run the given sample.
Continuously switch the ItemsSource.

Link to public reproduction project repository

https://github.com/Tamilarasan-Paranthaman/Maui-Itemssource-Switching-Issue

Version with bug

7.0.86

Last version that worked well

Unknown/Other

Affected platforms

Windows, I was not able test on other platforms

Affected platform versions

Windows

Did you find any workaround?

No response

Relevant log output

No response

The text was updated successfully, but these errors were encountered:

MartyIX · 2023-08-03T21:25:57Z

(@jonathanpeppers works on #16346 (iOS memory leaks). CheckBox is mentioned there as well. Not sure if by any chance a fix there can fix it for Windows as well.)

jonathanpeppers · 2023-08-03T21:50:38Z

@Tamilarasan-Paranthaman does this problem happen on .NET 8? The test I added for CheckBox was already passing on Windows.

There are various other fixes that are not in .NET 7.

Tamilarasan-Paranthaman · 2023-08-08T09:28:41Z

@jonathanpeppers I have tested the provided sample with ".NET 8" on the "Windows" platform and encountered the same issue.

jonathanpeppers · 2023-08-08T19:22:26Z

So, it looks like even if you have an empty <ViewCell/> your sample app leaks:

Notice the overall heap size at the end got up to 3,964 kb.

I looked here:

The problem is every Cell subscribes:

maui/src/Controls/src/Core/Cells/Cell.cs

Lines 197 to 201 in 9fcccd5

    
           if (RealParent != null) 
        
           { 
        
           	RealParent.PropertyChanged += OnParentPropertyChanged; 
        
           	RealParent.PropertyChanging += OnParentPropertyChanging; 
        
           }

And so the one ListView keeps all these cells alive.

Now I do think everything would go away if you navigated away from this page. Or a workaround would be to create two ListView and hide/show them instead.

jonathanpeppers · 2023-08-08T20:18:09Z

I can reproduce it in a test here:

main...jonathanpeppers:ListViewOhNo

This line appears to also be problematic, commented out along with RealParent.PropertyChanged & the test passes:

maui/src/Controls/src/Core/Element/Element.cs

Line 322 in 9fcccd5

_changeHandlers.Add(onchanged);

ghost · 2023-08-09T18:29:33Z

We've added this issue to our backlog, and we will work to address it as time and resources allow. If you have any additional information or questions about this issue, please leave a comment. For additional info about issue management, please read our Triage Process.

XamlTest · 2023-08-15T06:20:13Z

Verified this on Visual Studio Enterprise 17.8.0 Preview 1.0. Repro on Windows 11 .NET 8 with below Project:
16450.zip

jonathanpeppers · 2023-08-15T14:06:03Z

I added some logging to track each data-bound item:

List<WeakReference> references = new();

public MainPage()
{
    InitializeComponent();

    listView.ItemTemplate = new DataTemplate(() =>
    {
        var cell = new ViewCell();
        references.Add(new(cell));
        return cell;
    });
}

void Log()
{
    Debug.WriteLine($"Cells alive: {references.Count(l => l.IsAlive)}");
}

It looks like this issue only happens with ListView:

Cells alive: 207
Cells alive: 236
Cells alive: 266
Cells alive: 295
Cells alive: 325
Cells alive: 354

Where if I change to a CollectionView and Label in each row:

Labels alive: 30
Labels alive: 30
Labels alive: 20
Labels alive: 21
Labels alive: 23
Labels alive: 20
Labels alive: 21
Labels alive: 23
Labels alive: 21
Labels alive: 30
Labels alive: 23
Labels alive: 20
Labels alive: 21
Labels alive: 20
Labels alive: 20

So, a second workaround is to use CollectionView instead, while we investigate the issue with ListView.

Context: https://github.com/Tamilarasan-Paranthaman/Maui-Itemssource-Switching-Issue Fixes: dotnet#16450 In a customer sample, swapping a `ListView.ItemsSource` back and forth, such as: listView.ItemsSource = employeeInfoRepository.EmployeesInfo1; listView.ItemsSource = employeeInfoRepository.EmployeesInfo2; Appears to create new `Cell`'s for each call -- and each new cell lives forever. I could reproduce this issue in a device test. Viewing a memory snapshot, each `Cell` via `Cell.Parent` subscribes to events on their parent, the `ListView`: * https://github.com/dotnet/maui/blob/9fcccd57b5a3d664e4788b3c7fc3edc10010beaf/src/Controls/src/Core/Cells/Cell.cs#L197-L201 * https://github.com/dotnet/maui/blob/9fcccd57b5a3d664e4788b3c7fc3edc10010beaf/src/Controls/src/Core/Element/Element.cs#L322 This makes the `ListView` hold onto each `Cell` causing them to live forever. However, the leak would go away if you navigated away from the current `Page`, removed the `ListView` from the screen, etc. Possible workarounds were: 1. Use two `ListView` instead, and show/hide them. 2. Use a `CollectionView` instead. The fix I came up with appears to be reasonable. In `CellControl.OnUnloaded`: void OnUnloaded(object sender, RoutedEventArgs e) { // ... // 🚀 unsubscribe from propertychanged Cell.PropertyChanged -= _propertyChangedHandler; // Allows the Cell to unsubscribe from Parent.PropertyChanged Cell.Parent = null; } This event is fired when WinUI has removed the `CellControl` from the visual tree. We can safely *unset* the `Cell`'s `Parent`, which unsubscribes from the problematic events. If the `Cell` is later re-added to the visual tree, it's `DataContext` will be updated. In `CellControl`'s `SetCell(object newContext)` the `Cell.Parent` is reset to the `ListView`. I added two test cases for this scenario: one for the leak, and one to prove that re-setting `ItemsSource` still works.

Tamilarasan-Paranthaman · 2023-08-16T07:27:26Z

@jonathanpeppers,

I have checked the same test case using the CollectionView. The issue has been reproduced in that particular sample as well. Upon switching between item sources, there's a noticeable increase in the application size, which ultimately leads to a crash. In the attached sample, I simply replaced ListView with CollectionView while keeping the CheckBox within the CollectionView DataTemplate.

<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
             xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
             xmlns:local="clr-namespace:MauiApp1"
             x:Class="MauiApp1.MainPage">

    <ContentPage.BindingContext>
        <local:EmployeeInfoRepository x:Name="employeeInfoRepository"/>
    </ContentPage.BindingContext>

    <Grid RowDefinitions="Auto,*">
        <HorizontalStackLayout Grid.Row="0">
            <Button Text="ItemSource 1" Clicked="ItemSource_One_Button_Clicked"/>
            <Button Text="ItemSource 2" Clicked="ItemSource_Two_Button_Clicked"/>
        </HorizontalStackLayout>
        <CollectionView  Grid.Row="1" x:Name="listView" ItemsSource="{Binding EmployeesInfo1}">
            <CollectionView.ItemTemplate>
                <DataTemplate>
                    <CheckBox x:Name="checkBoxCell" Margin="10,0,0,0" IsChecked="{Binding IsChecked}"
                          VerticalOptions="Center">
                    </CheckBox>
                </DataTemplate>
            </CollectionView.ItemTemplate>
        </CollectionView>
    </Grid>

</ContentPage>

When I replaced the CheckBox with a label in the DataTemplate, the crash did not occur, and the application size did not increase significantly.

It seems that the presence of a CheckBox in the DataTemplate is a recurring issue for both the CollectionView and ListView.

MAUI_checkbox.mp4

jonathanpeppers · 2023-08-16T16:37:34Z

Let me fix the underlying issue with all Cell's first, and investigate CheckBox second.

The control itself doesn't leak in isolation, as we have a test for it:

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

Lines 36 to 51 in 0ebb60b

    
           [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)

jonathanpeppers · 2023-08-17T17:03:09Z

So, I'm testing a local build from PR #16762 (basically .NET 8).

CheckBox looks OK, if I click quickly the number goes up but then back down just fine:

CheckBox's alive: 30
CheckBox's alive: 59
CheckBox's alive: 59
CheckBox's alive: 29
CheckBox's alive: 35
CheckBox's alive: 59
CheckBox's alive: 59
CheckBox's alive: 30
CheckBox's alive: 30
CheckBox's alive: 30
CheckBox's alive: 30

My changes to the app are here: jonathanpeppers/Maui-Itemssource-Switching-Issue@43af9ec

* [windows] fix memory leak in ListView Context: https://github.com/Tamilarasan-Paranthaman/Maui-Itemssource-Switching-Issue Fixes: #16450 In a customer sample, swapping a `ListView.ItemsSource` back and forth, such as: listView.ItemsSource = employeeInfoRepository.EmployeesInfo1; listView.ItemsSource = employeeInfoRepository.EmployeesInfo2; Appears to create new `Cell`'s for each call -- and each new cell lives forever. I could reproduce this issue in a device test. Viewing a memory snapshot, each `Cell` via `Cell.Parent` subscribes to events on their parent, the `ListView`: * https://github.com/dotnet/maui/blob/9fcccd57b5a3d664e4788b3c7fc3edc10010beaf/src/Controls/src/Core/Cells/Cell.cs#L197-L201 * https://github.com/dotnet/maui/blob/9fcccd57b5a3d664e4788b3c7fc3edc10010beaf/src/Controls/src/Core/Element/Element.cs#L322 This makes the `ListView` hold onto each `Cell` causing them to live forever. However, the leak would go away if you navigated away from the current `Page`, removed the `ListView` from the screen, etc. Possible workarounds were: 1. Use two `ListView` instead, and show/hide them. 2. Use a `CollectionView` instead. The fix I came up with appears to be reasonable. In `CellControl.OnUnloaded`: void OnUnloaded(object sender, RoutedEventArgs e) { // ... // 🚀 unsubscribe from propertychanged Cell.PropertyChanged -= _propertyChangedHandler; // Allows the Cell to unsubscribe from Parent.PropertyChanged Cell.Parent = null; } This event is fired when WinUI has removed the `CellControl` from the visual tree. We can safely *unset* the `Cell`'s `Parent`, which unsubscribes from the problematic events. If the `Cell` is later re-added to the visual tree, it's `DataContext` will be updated. In `CellControl`'s `SetCell(object newContext)` the `Cell.Parent` is reset to the `ListView`. I added two test cases for this scenario: one for the leak, and one to prove that re-setting `ItemsSource` still works. * Update ListViewTests.cs

deviprasad987 · 2023-09-14T09:47:05Z

@MartyIX @jonathanpeppers @PureWeen @jsuarezruiz @samhouts
In which VS version this memory leak issue will be fixed?

MartyIX · 2023-09-14T09:54:42Z

It should be in https://github.com/dotnet/maui/releases/tag/8.0.0-rc.1.9171. It mentions:

[windows] fix memory leak in ListView by @jonathanpeppers in https://github.com/dotnet/maui/pull/16762

(But literaly above your post one can see fixed-in-8.0.0-rc.1.9171.)

deviprasad987 · 2023-09-14T09:59:29Z

Is it available in VS preview#17.8.0-pre.1.0

Tamilarasan-Paranthaman · 2023-09-21T09:58:26Z

@jonathanpeppers, @samhouts,

I am still encountering the same issue in version 8.0.0-rc.1.9171, although it was mentioned as fixed. The app size gradually increases and leads to a crash of the application. I have attached my updated sample (using version 8.0.0-rc.1.9171) along with a reference video demonstrating the issue.

Recording.mp4

Sample Link: Application.zip

Additionally, while reviewing your application, I noticed that you added a Label without assigning any text within the ViewCell, instead of using a CheckBox. Is there a specific reason for this choice?

jonathanpeppers · 2023-09-21T14:08:41Z

#16762 fixed a general problem with all ViewCell's no matter what the contents were.

I will retest your app above, thanks.

jonathanpeppers · 2023-09-22T16:18:07Z

@Tamilarasan-Paranthaman in your video is XAML hot reload disabled? (I see the toolbar in your video, so I guess it might not be?)

XAML hot reload is known to cause diagnostic/profiling tools to show incorrect results, they display a warning on this screen:

This is mentioned here:

Tamilarasan-Paranthaman · 2023-09-25T14:58:34Z

@jonathanpeppers, the XAML hot reload is not disabled in my Visual Studio.

However, I'm still facing the same issue even after disabling it.

jonathanpeppers · 2023-09-25T15:20:30Z

@Tamilarasan-Paranthaman for testing this, do you also have some GC.Collect() calls in the app like I did here.

This is mentioned as a troubleshooting step that makes the GC at least slightly more deterministic:

https://github.com/dotnet/maui/wiki/Memory-Leaks#determining-if-there-is-a-leak-or-not

(remove these when your testing is done)

I tested your app again on Friday with GC.Collect() calls, and I did not observe a leak. Memory went up and down for me -- it did not consistently grow.

ghost · 2023-09-25T20:27:57Z

Hi @Tamilarasan-Paranthaman. We have added the "s/needs-info" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

Tamilarasan-Paranthaman · 2023-09-26T06:34:57Z

@jonathanpeppers, I have tested the application with GC.Collect() calls, and the issue did not reproduce when XAML hot reload was disabled.

However, why does the issue reproduce when binding the Checkbox in the XAML page? This issue occurred in both XAML hot reload enabled and disabled cases. There is an issue when binding the Checkbox on the XMAL page?

jonathanpeppers · 2023-09-26T14:20:37Z

issue did not reproduce when XAML hot reload was disabled.

However, why does the issue reproduce when binding the Checkbox in the XAML page? This issue occurred in both XAML hot reload enabled and disabled cases. There is an issue when binding the Checkbox on the XMAL page?

So, you still see an issue? Or not?

If you remove x:Name in the <DataTemplate/> does that change anything for you? Maybe you can share an updated project/video, and I will try it again in the meantime.

Tamilarasan-Paranthaman · 2023-09-27T11:03:03Z

@jonathanpeppers, I'm still encountering the issue even after disabling XAML hot reload in my application, which leads to a crash. I have already attached my updated sample to this query #16450 (comment).

As per your suggestion, I also test it by adding a checkbox along with GC.Collect() calls in the code-behind. In this scenario, the issue did not reproduce when XAML hot reload was disabled.

Samples	XAML hot reload disabled	XAML hot reload enabled
With this sample #16450 (comment)	`Issue reproduced`	Issue reproduced
Sample with GC.Collect() calls like you did here.	`Issue not reproduced`	Issue reproduced

The difference between the two samples lies in the addition of a Checkbox, with one being added in XAML and the other in the code behind.

Recording.mp4

jonathanpeppers · 2023-09-27T13:44:15Z

Did you try this suggestion?

If you remove x:Name in the <DataTemplate/> does that change anything for you? Maybe you can share an updated project/video, and I will try it again in the meantime.

In the video why isn't there a GC.Collect() call on line 19 for testing?

To verify, you are using .NET 8 RC 1 and the project has net8.0 TargetFrameworks? If any say net7.0, then you are still using .NET 7 stable versions of MAUI.

jonathanpeppers · 2023-10-10T18:47:54Z

I tested with the .NET 8 RC 2 release today, only change to your sample was added GC.Collect() calls: Application.zip

I don't feel like I'm seeing a leak at all, when taking a snapshot between each button press:

@Tamilarasan-Paranthaman I would like to solve a memory issue here if there is one, but I'm not currently able to see it. If you can give more details or info, we can investigate further. Thanks!

Tamilarasan-Paranthaman added the t/bug Something isn't working label Jul 31, 2023

jsuarezruiz added platform/windows 🪟 legacy-area-perf Startup / Runtime performance area-controls-checkbox CheckBox labels Jul 31, 2023

ghost added the legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor label Jul 31, 2023

samhouts added the memory-leak 💦 Memory usage grows / objects live forever (sub: perf) label Aug 1, 2023

jonathanpeppers changed the title ~~Memory Leak issue in CheckBox~~ Memory Leak issue in ListView Aug 8, 2023

jonathanpeppers added area-controls-listview ListView and TableView and removed area-controls-checkbox CheckBox labels Aug 8, 2023

jonathanpeppers mentioned this issue Aug 8, 2023

Memory leaks on latest .NET 8 preview with Windows #16436

Open

PureWeen added this to the Backlog milestone Aug 9, 2023

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

jonathanpeppers mentioned this issue Aug 15, 2023

[windows] fix memory leak in ListView #16762

Merged

samhouts modified the milestones: Backlog, .NET 8 GA Aug 15, 2023

samhouts added this to MAUI SDK Ongoing Aug 16, 2023

github-project-automation bot moved this to Todo in MAUI SDK Ongoing Aug 16, 2023

github-project-automation bot moved this from Todo to Done in MAUI SDK Ongoing Aug 17, 2023

samhouts added the fixed-in-8.0.0-rc.1.9171 Look for this fix in 8.0.0-rc.1.9171 label Sep 12, 2023

jonathanpeppers reopened this Sep 21, 2023

samhouts added the s/needs-info Issue needs more info from the author label Sep 25, 2023

samhouts modified the milestones: .NET 8 GA, .NET 8 + Servicing Sep 25, 2023

ghost added s/needs-attention Issue has more information and needs another look and removed s/needs-info Issue needs more info from the author labels Sep 26, 2023

jonathanpeppers closed this as completed Oct 10, 2023

ghost locked as resolved and limited conversation to collaborators Nov 9, 2023

samhouts removed this from MAUI SDK Ongoing Jan 10, 2024

Eilon added t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf) and removed legacy-area-controls Label, Button, CheckBox, Slider, Stepper, Switch, Picker, Entry, Editor legacy-area-perf Startup / Runtime performance labels May 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Memory Leak issue in ListView #16450

Memory Leak issue in ListView #16450

Tamilarasan-Paranthaman commented Jul 31, 2023

MartyIX commented Aug 3, 2023

jonathanpeppers commented Aug 3, 2023 •

edited

Loading

Tamilarasan-Paranthaman commented Aug 8, 2023

jonathanpeppers commented Aug 8, 2023 •

edited

Loading

jonathanpeppers commented Aug 8, 2023

ghost commented Aug 9, 2023

XamlTest commented Aug 15, 2023

jonathanpeppers commented Aug 15, 2023

Tamilarasan-Paranthaman commented Aug 16, 2023

jonathanpeppers commented Aug 16, 2023

jonathanpeppers commented Aug 17, 2023

deviprasad987 commented Sep 14, 2023

MartyIX commented Sep 14, 2023

deviprasad987 commented Sep 14, 2023

Tamilarasan-Paranthaman commented Sep 21, 2023

jonathanpeppers commented Sep 21, 2023

jonathanpeppers commented Sep 22, 2023

Tamilarasan-Paranthaman commented Sep 25, 2023

jonathanpeppers commented Sep 25, 2023

ghost commented Sep 25, 2023

Tamilarasan-Paranthaman commented Sep 26, 2023

jonathanpeppers commented Sep 26, 2023

Tamilarasan-Paranthaman commented Sep 27, 2023

jonathanpeppers commented Sep 27, 2023

jonathanpeppers commented Oct 10, 2023

Memory Leak issue in ListView #16450

Memory Leak issue in ListView #16450

Comments

Tamilarasan-Paranthaman commented Jul 31, 2023

Description

Steps to Reproduce

Link to public reproduction project repository

Version with bug

Last version that worked well

Affected platforms

Affected platform versions

Did you find any workaround?

Relevant log output

MartyIX commented Aug 3, 2023

jonathanpeppers commented Aug 3, 2023 • edited Loading

Tamilarasan-Paranthaman commented Aug 8, 2023

jonathanpeppers commented Aug 8, 2023 • edited Loading

jonathanpeppers commented Aug 8, 2023

ghost commented Aug 9, 2023

XamlTest commented Aug 15, 2023

jonathanpeppers commented Aug 15, 2023

Tamilarasan-Paranthaman commented Aug 16, 2023

jonathanpeppers commented Aug 16, 2023

jonathanpeppers commented Aug 17, 2023

deviprasad987 commented Sep 14, 2023

MartyIX commented Sep 14, 2023

deviprasad987 commented Sep 14, 2023

Tamilarasan-Paranthaman commented Sep 21, 2023

jonathanpeppers commented Sep 21, 2023

jonathanpeppers commented Sep 22, 2023

Tamilarasan-Paranthaman commented Sep 25, 2023

jonathanpeppers commented Sep 25, 2023

ghost commented Sep 25, 2023

Tamilarasan-Paranthaman commented Sep 26, 2023

jonathanpeppers commented Sep 26, 2023

Tamilarasan-Paranthaman commented Sep 27, 2023

jonathanpeppers commented Sep 27, 2023

jonathanpeppers commented Oct 10, 2023

jonathanpeppers commented Aug 3, 2023 •

edited

Loading

jonathanpeppers commented Aug 8, 2023 •

edited

Loading