[Bug] Android Forms9Patch Image retains page and view model indefinitely in memory

[ASankey-Ideagen]

Description

Using the Xamarin Profiler to profile a page with a Forms9Patch image, we noticed that the page and view model were being retained in memory after the page had been closed, incrementing each time the page was opened. Replacing the image element for a standard Xamarin Forms image resulted in the page being collected.

Steps to Reproduce

1. Launch the app using Xamarin Profiler
2. Take a snapshot in Xamarin Profiler
3. Open Forms9PatchImage page
4. Navigate back
5. Repeat steps 3 & 4 several times
6. Take a snapshot in Xamarin Profiler
7. Compare snapshots
8. Search for F9PImageViewModel within snapshot
9. Several instances will be live in memory.

Repeat same steps for Standard XF Image page and only 1 instance will generally be kept alive in memory no matter how many times the page is opened.

Expected Behavior

Page and view model are collected and removed from memory after page is closed.

Actual Behavior

Page and view model are retained in memory, incrementing each time page is opened.

Basic Information

• Version with issue: 2.4.2
• Last known good version: Unknown
• IDE:
• Platform Target Frameworks:
  • iOS: Not testet
  • Android: 9.0
  • UWP: Not tested
• Android Support Library Version:
• Nuget Packages:
• Affected Devices:

Screenshots

Reproduction Link - a link to a small demo project that reproduces this issue

https://github.com/ASankey-Ideagen/Forms9Patch_MemoryIssue

Workaround

If Xamarin.Forms ImageSource.FromResource is used to get the embedded resources rather than the Forms9Patch versions, along with explicitly setting the source on the image element instead of within a style will allow the page to be collected.

[baskren]

a lot of Forms9Patch elements are IDisposable. As such you, as the page or view author, are responsible for disposing them.

[baskren]

I should have added a little bit more. This is a VERY common problem for which Xamarin didn't build-in support. If they had added IDisposable garbage collection to their Page stacks, then most of the work would be done for you. As such, it is still pretty straight forward to add that in. There are a couple of basics things to implement:

1. A event handler for when pages are popped off the page stack(s). For example:

           if (NavigationPage == null)
            {
                LoginPage = new Views.LoginPage();
                NavigationPage = new NavigationPage(LoginPage);
            }

            NavigationPage.Popped += async (object sender, NavigationEventArgs e) =>
            {
                if (NavigationPage.CurrentPage == LoginPage)
                    await LoginPage.LoginView.OnAppearing();
                if (e.Page is IDisposable disposable)
                    disposable.Dispose();
            };

The above code was added to the app's App.xaml.cs file in the cross platform .NetStandard project. If you're using modal pages, then you'll have to do something similar with the modal stack.

2. Implement IDisposable on the pages that contain IDisposable descendant elements. For example:

        private bool disposed;
        protected virtual void Dispose(bool disposing)
        {
            if (!disposed && disposing)
            {
                disposed = true;

                Xamarin.Essentials.Connectivity.ConnectivityChanged -= OnConnectivity_ConnectivityChanged;
                _settingsButton.Clicked -= OnSettingsButtonClicked;

                _saveSettingsButton.Dispose();
                _backButton.Dispose();
                _logoutButton.Dispose();
            }
        }

Note that this isn't some crazy design pattern unique to Forms9Patch. This is how you'll need to do things if you're subscribed to events (in Xamarin.Essentials, for example) on your page / view.

[ASankey-Ideagen]

Thanks for the swift response. I can verify that disposing the image when the page is removed does release everything and allows the page to be collected.
I am curious though as to why this is needed at all? As mentioned in the work around section, if you use Xamarin forms ImageSource to get the embedded file resource and set the source directly on the image element rather than in a style, the page is collected without the need to dispose of anything. Using a Xamarin forms image also handles all scenarios without needing to be disposed either.

[baskren]

Short answer: performance.

Long answer:

Without spending a lot of time looking into this, I can see that you're using Forms9Patch.Image in the page in question:

<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://xamarin.com/schemas/2014/forms"
             xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
             xmlns:f9p="clr-namespace:Forms9Patch;assembly=Forms9Patch"
             xmlns:d="http://xamarin.com/schemas/2014/forms/design"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
             mc:Ignorable="d"
             x:Class="MemoryAllocationTest.Views.F9PImagePage"
             Title="{Binding Title}">
    <ContentPage.Content>
        <StackLayout
            Padding="20"
            Spacing="20">
            <Label 
                Text="Forms9Patch Image causing page to be retained."
                FontSize="20"
                VerticalOptions="Start" 
                HorizontalOptions="CenterAndExpand" />

            <f9p:Image
                Style="{StaticResource F9PImageStyle}"/>

        </StackLayout>
    </ContentPage.Content>
</ContentPage>

Looking at the disposing code for Forms9Patch.Image (starting at line 673):

Forms9Patch/Forms9Patch/Forms9Patch/Elements/Image.cs

Lines 673 to 700 in fa1c848

	private bool _disposed;
	/// <summary>
	/// Disposed the image
	/// </summary>
	/// <param name="disposing"></param>
	protected virtual void Dispose(bool disposing)
	{
	if (!_disposed && disposing)
	{
	_disposed = true;
	Source = null;
	_f9pImageData?.Dispose();
	_f9pImageData = null;
	_sourceRangeLists = null;
	P42.Utils.Debug.RemoveFromCensus(this);
	}
	}
	/// <summary>
	/// Dispose the image
	/// </summary>
	public void Dispose()
	{
	Dispose(true);
	GC.SuppressFinalize(this);
	}

... you can see that the only thing that is disposed is _f9pImageData. So what is _f9pImageData?

Forms9Patch/Forms9Patch/Forms9Patch/Elements/Image.cs

Line 518 in fa1c848

F9PImageData _f9pImageData;

Which takes you to ... https://github.com/baskren/Forms9Patch/blob/master/Forms9Patch/Forms9Patch/Models/F9PImageData.cs

At which point you may be a bit overwhelmed. To simplify, F9PImageData is a SkiaSharp cache of the image data (bitmap or vector path). It's retained by Forms9Patch.Image to allow for fast rendering when the Image element needs to be resized (which, as it turns out, can be a bit too often).

So, the Image data is retained for fast rendering. The down side is that it needs to be released (disposed) or else it will leak.

Again, this is a VERY common design pattern in .NET. My opinion is this is something that isn't talked about in almost all "Getting Started" or "Tutorial" type of documents because, like any other framework, memory management is a bit complicated and most developers are not motivated to learn it until they have to.