maui icon indicating copy to clipboard operation
maui copied to clipboard

[net7.0 / net8.0] [iOS] Memory leak when CollectionView.ItemsSource changes

Open Sergtek opened this issue 1 year ago • 10 comments

Description

I am aware of the existence of the following bug #13530 but only mention is made of Windows, however in iOS the same memory leak problem also occurs when updating the ItemsSource of a CollectionView several times and the RAM memory is not released even by browsing backwards:

issue1

I leave a video playing an example repository where I can reproduce this problem in iOS until the application crashes due to lack of memory: https://www.youtube.com/watch?v=_UTEGbE4-ug

I leave the repository that I show in the video so you can do the test: https://github.com/nacompllo/MemoryLeakEverywhere/tree/bugfix/memoryLeakItemsSource

Steps to Reproduce

  • Clone the following repository: https://github.com/nacompllo/MemoryLeakEverywhere/tree/bugfix/memoryLeakItemsSource
  • Run the app on an iOS device.
  • Have Xcode Instruments open to see how the RAM increases.
  • Follow the same steps as in the attached video.

Link to public reproduction project repository

https://github.com/nacompllo/MemoryLeakEverywhere/tree/bugfix/memoryLeakItemsSource

Version with bug

7.0 (current)

Last version that worked well

Unknown/Other

Affected platforms

iOS

Affected platform versions

iOS 16

Did you find any workaround?

No response

Relevant log output

No response

Depends on

  • [x] https://github.com/dotnet/maui/pull/15062
  • [x] https://github.com/dotnet/maui/pull/15303
  • [x] https://github.com/dotnet/maui/pull/15831
  • [x] https://github.com/dotnet/maui/pull/15946

Sergtek avatar Apr 19 '23 15:04 Sergtek

How is this different from https://github.com/dotnet/maui/issues/14654, which is not closed?

drasticactions avatar Apr 19 '23 15:04 drasticactions

How is this different from #14654, which is not closed?

The #14654 is about a memory leak when repeatedly navigating between pages and #14664 is a memory leak when the ItemsSource from CollectionView changes.

Sergtek avatar Apr 19 '23 15:04 Sergtek

@nacompllo and you were testing this on .NET 7?

It might be fixed by this one, which is only in main/.NET 8 currently:

  • https://github.com/dotnet/maui/pull/14329

I will test your above sample, though, thanks.

jonathanpeppers avatar May 09 '23 14:05 jonathanpeppers

Note that my comment here:

image

Is specifically talking about a list of "logical children" that grew indefinitely on Windows.

jonathanpeppers avatar May 09 '23 14:05 jonathanpeppers

There appears to be an issue with images on iOS, a failing test:

[Fact("Image Does Not Leak")]
public async Task DoesNotLeak()
{
	SetupBuilder();
	WeakReference reference = null;

	await InvokeOnMainThreadAsync(async () =>
	{
		var layout = new VerticalStackLayout();
		var image = new Image
		{
			Background = Colors.Black,
			Source = "red.png",
		};
		layout.Add(image);

		var handler = CreateHandler<LayoutHandler>(layout);
		await image.Wait();
		reference = new WeakReference(image.Handler.PlatformView);
	});

	await Task.Yield();
	GC.Collect();
	GC.WaitForPendingFinalizers();
	
	Assert.NotNull(reference);
	Assert.False(reference.IsAlive, "PlatformView should not be alive!");
}

Still investigating what the fix would be.

jonathanpeppers avatar May 09 '23 16:05 jonathanpeppers

Hi everyone,

I can see that the issue is still in an Open status, but I am writing to provide some feedback just in case it helps. I have forked the repository indicated by @nacompllo and I have tested the same application compiling with the version released a few days ago of .Net 8 preview 4. The changes to make the compilation are isolated in two independent branches (one for https://github.com/dotnet/maui/issues/14654 and another for https://github.com/dotnet/maui/issues/14664). Here is a link to the repository:

https://github.com/QuiqueLargachaGil/MAUI-POC.MemoryLeakEverywhere/tree/bugfix/updated-to-Net-8-preview-4-in-memoryLeakItemsSource-branch

And the result for the case of this incident is the same. There are still memory leaks when reproducing the issue. I share with you a youtube link to watch the video that shows it.

https://www.youtube.com/watch?v=EAxcUQE6wLE

Many thanks!

QuiqueLargachaGil avatar May 29 '23 14:05 QuiqueLargachaGil

Yes, this is not fixed until we get these merged:

  • https://github.com/dotnet/maui/pull/15062
  • https://github.com/dotnet/maui/pull/15193

But then we'll need to retest the above sample app, to see if there are further issues.

jonathanpeppers avatar May 30 '23 14:05 jonathanpeppers

The list of linked PRs above address various memory issues found the in above sample. However, the memory still appears to grow in the sample with dotnet/maui/main at this time.

Latest Issue

I found the "logical children" grow indefinitely on iOS/Catalyst, after adding some logging the sample above quickly got up to 71 items:

2023-07-06 09:04:27.745 MemoryLeakEverywhere[93127:7440192] Microsoft.Maui.Controls.CollectionView added child -> Microsoft.Maui.Controls.Image, count: 71

I can illustrate the issue in this test:

https://github.com/jonathanpeppers/maui/commit/bbea0597cd0d90e4f0a84445f4ae35445f553bb4

It passes on Windows & Android, but on iOS & Catalyst the list of logical children grows.

It appears to be due to this pattern:

var newCollection = new ObservableCollection<string>();
collectionView.ItemsSource = newCollection;
foreach (var item in data)
{
    newCollection.Add(item);
}

If I change it to set ItemsSource last, it also works around this issue:

var newCollection = new ObservableCollection<string>();
foreach (var item in data)
{
    newCollection.Add(item);
}
collectionView.ItemsSource = newCollection;

This version is faster anyway, but the issue seems to be related to the CollectionChanged event firing.

Workarounds for Above Sample

  • Set ItemsSource or data-bound properties last and do not rely as much on CollectionChanged
    • Example: https://github.com/jonathanpeppers/nacompllo-MemoryLeakEverywhere/commit/117e3af065ee4b0c8c6245bb649a53680018180f
  • We noticed the images in each row were quite large: larger than the area they occupy on screen. We recommend setting BaseSize, so that less memory is used for these images.
    • Example (I just chose a number): https://github.com/jonathanpeppers/nacompllo-MemoryLeakEverywhere/commit/00a57aab9335b7bfdfaaa2839a19df3cfec1a747

jonathanpeppers avatar Jul 07 '23 14:07 jonathanpeppers

Hi @nacompllo. We have added the "s/try-latest-version" label to this issue, which indicates that we'd like you to try and reproduce this issue on the latest available public version. This can happen because we think that this issue was fixed in a version that has just been released, or the information provided by you indicates that you might be working with an older version.

You can install the latest version by installing the latest Visual Studio (Preview) with the .NET MAUI workload installed. If the issue still persists, please let us know with any additional details and ideally a reproduction project provided through a GitHub repository.

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.

ghost avatar Jan 04 '24 08:01 ghost

This is a Memory leak issue, we tried to reproduce it with a user-provided project, and it took over an hour to run. But the memory consumption is still less than 1GB, so it is marked as not reproduce.

homeyf avatar Jan 04 '24 08:01 homeyf