Themes leaking WeakReferences (I think)

JamesLaveryJamesLavery GBBeta, University ✭✭✭✭✭

I've been spending a lot of time trying to track down memory leaks in our Xamarin.Forms app on Android. After a lot of blind alleys and false dawns, I think I may have come across something which is causing the problem.

Using Xamarin Profiler, I can see that as soon as I create a Style and apply it to a control (or in fact just an implicit style), we get Multiple WeakReferences remaining 'Live' - i.e. not garbage collected.

Note that I assume that the objects to which they refer have been GC'd (because the reference to the object is weak), but the WeakReferences themselves are remaining.

Now of course WeakReferences are small I know - but when you have hundreds created on every iteration of a page push/pop, then the memory adds up and we have a significant leak.

Here are the details.
Using Xamarin.Forms 2.3.4.270 (we haven't upgraded because we want to keep with known issues!)
Running on Android - physical device.

App.xaml:

<?xml version="1.0" encoding="utf-8"?>
<Application xmlns="http://xamarin.com/schemas/2014/forms" xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" xmlns:local="clr-namespace:ProfiledFormsApp2;assembly=ProfiledFormsApp2" 
     x:Class="ProfiledFormsApp2.App">
    <Application.Resources>
        <ResourceDictionary>
            <Style TargetType="Label">
                <Setter Property="FontSize" Value="Large" />
                <Setter Property="TextColor" Value="Blue" />
            </Style>
        </ResourceDictionary>
    </Application.Resources>
</Application>

Page XAML:

<?xml version="1.0" encoding="UTF-8"?>
<ContentPage Title="Plain Page" xmlns="http://xamarin.com/schemas/2014/forms" xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" x:Class="ProfiledFormsApp2.PlainPage">
    <ContentPage.Content>
        <StackLayout>
            <StackLayout Orientation="Horizontal">
                <Label Text="Core Navigation"/>
                <Label Text="Number of items:" />
                <Label Text="{Binding ItemsCount}" />
            </StackLayout>
        </StackLayout>
    </ContentPage.Content>
</ContentPage>

When I navigate to the above page and back 3 times, we have the following WeakReference (and related) classes created - the screenshot below is from a Profiler snapshot.

Note we have 55 WeakReferences. Drilling into these shows:

What is interesting is that these WeakReferences seem to be created as part of Behavior and Trigger attaching. Looking at the call tree for the top one gives:

So it appears that the WeakReference was created as part of setting a BindableObject's value and the subsequent setting of the Style.

And it also appears that the WeakReference is still in memory and being referenced by something - the Behavior collection?

Using Profiler, I'm can see that we don't have Labels remaining un GC'd. It seems to be something in the Theme/Behavior/Trigger processing.

I haven't looked at the Xamarin.Forms code on GitHub yet - that might have to be my next action.

Has anyone observed this or got a solution?

@NMackay, @JohnHardman You may be interested in this as this could be the underlying cause of the memory leak I thought I observed some time ago and mentioned in a post here.

Posts

  • JohnHardmanJohnHardman GBUniversity mod
    edited January 2018

    @JamesLavery - After reading something similar about Behaviors some time back (might have been @AdamP who mentioned it, but not certain of that), I have made a point of removing Behaviors from Views when Pages are being popped. I'm hoping it's not necessary to clear Styles when popping pages, as that would mean significantly more cleanup than the limited number of Behaviors that I use. Will get around to profiling again one of these days - in midst of renewing certificates etc on iOS right now, one of my most hated jobs.

  • JamesLaveryJamesLavery GBBeta, University ✭✭✭✭✭

    John - thanks, I remember that post too about Behaviors. I was next considering whether clearing Styles would solve this, but haven't tried it yet. I'll keep you posted.

  • NMackayNMackay GBInsider, University mod
    edited January 2018

    @JamesLavery said:
    John - thanks, I remember that post too about Behaviors. I was next considering whether clearing Styles would solve this, but haven't tried it yet. I'll keep you posted.

    It was probably me, been banging on about this topic for ages.

    https://developer.xamarin.com/guides/xamarin-forms/application-fundamentals/behaviors/creating/

    It's pretty explicit in the documentation

    In addition, note that behaviors are not implicitly removed from controls when pages are popped from the navigation stack. Instead, they must be explicitly removed prior to pages going out of scope.

    This is the easiest way in Prism.

    using Prism.Navigation;
    using Xamarin.Forms;
    using Xamarin.Forms.Xaml;
    
    namespace Foobar.Views.Shared
    {
        [XamlCompilation(XamlCompilationOptions.Compile)]
        public partial class CustomerSearchPage : IDestructible
        {
            public CustomerSearchPage()
            {
                InitializeComponent();
            }
    
            public void Destroy()
            {
                SearchBarCustomerName.Behaviors.Clear();
                ListviewCustomers.Behaviors.Clear();
            }
        }
    }
    
  • JamesLaveryJamesLavery GBBeta, University ✭✭✭✭✭

    @JohnHardman Yes, setting a UI element's Style to null on disappearing (I'm only popping the page in question in my test project) fixes the problem.

    However this only works if you've got an explicit style set e.g. for label1 below.

    <?xml version="1.0" encoding="UTF-8"?>
    <ContentPage Title="Plain Page" xmlns="http://xamarin.com/schemas/2014/forms" xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" x:Class="ProfiledFormsApp2.PlainPage">
        <ContentPage.Content>
            <StackLayout>
                <StackLayout Orientation="Horizontal">
                    <Label x:Name="label1" Style="{StaticResource MyLabelStyle}" Text="Core Navigation"/>
                    <Label x:Name="label2" Text="Number of items:" />
                    <Label x:Name="label3" Text="{Binding ItemsCount}" />
                </StackLayout>
            </StackLayout>
        </ContentPage.Content>
    </ContentPage>
    

    If there is an implicit style, then setting Style to null doesn't work. I've just tried it.

  • JamesLaveryJamesLavery GBBeta, University ✭✭✭✭✭

    @NMackay Posts crossed. Yes, we're clearing Behaviors in a similar manner to you (although we're not using them explicitly).

  • NMackayNMackay GBInsider, University mod

    @JohnHardman said:
    @JamesLavery - After reading something similar about Behaviors some time back (might have been @AdamP who mentioned it, but not certain of that), I have made a point of removing Behaviors from Views when Pages are being popped. I'm hoping it's not necessary to clear Styles when popping pages, as that would mean significantly more cleanup than the limited number of Behaviors that I use. Will get around to profiling again one of these days - in midst of renewing certificates etc on iOS right now, one of my most hated jobs.

    I loath doing iOS certificates, especially when you have to renew production certifications, redo provisioning profiles and of course the lovely ah-hoc inhouse certificates for side loading which expire on a yearly basis.

  • JohnHardmanJohnHardman GBUniversity mod

    @JamesLavery - Oh, that's not good. I use explicit styles a lot :-( But thanks for the headsup - looks like some re-factoring coming up...

  • NMackayNMackay GBInsider, University mod
    edited January 2018

    @JohnHardman said:
    @JamesLavery - Oh, that's not good. I use explicit styles a lot :-( But thanks for the headsup - looks like some re-factoring coming up...

    Is that in 2.5.0? PCL or .NETStandard? it's certainly not a feature. Is it every control? it's certainly an approach we've used a lot too so it would be good to know if it's the control or something that effects all controls or the vanilla Forms controls (eg not 3rd party controls).

    Thanks for posting your results here.

  • JamesLaveryJamesLavery GBBeta, University ✭✭✭✭✭

    Just tried

        protected override void OnDisappearing()
        {
            label1.Style = null;
            label1.Behaviors.Clear();
            label1.Triggers.Clear();
    
            label2.Style = null;
            label2.Behaviors.Clear();
            label2.Triggers.Clear();
    
            label3.Style = null;
            label3.Behaviors.Clear();
            label3.Triggers.Clear();
    
            base.OnDisappearing();
        }
    

    (and another try, setting the Style to null last)

    in case the Style/Theme code was creating Behaviors and Triggers on the controls themselves.

    This doesn't solve it for implicit styles.

  • JamesLaveryJamesLavery GBBeta, University ✭✭✭✭✭

    @NMackay said:

    @JohnHardman said:
    @JamesLavery - Oh, that's not good. I use explicit styles a lot :-( But thanks for the headsup - looks like some re-factoring coming up...

    Is that in 2.5.0? PCL or .NETStandard? it's certainly not a feature. Is it every control? it's certainly an approach we've used a lot too so it would be good to know if it's the control or something that effects all controls or the vanilla Forms controls (eg not 3rd party controls).

    Thanks for posting your results here.

    This is in 2.3.4.270 and PCL.

    This is for vanilla Forms controls - my test is just with a Label.

    Going offline now - (evening bike ride on Wednesdays). Will respond later. ;)

  • JamesLaveryJamesLavery GBBeta, University ✭✭✭✭✭

    Quick update - I think we do need to clear styles to free memory for them :/

    When I profile, if I don't clear styles then there is definitely leaking taking place. This is on latest stable (2.5.0.122203)

  • JohnHardmanJohnHardman GBUniversity mod

    @JamesLavery - Thanks for the update. I'm not aware of that being documented anywhere, so it's probably worth raising a bug if you haven't already (hopefully to get the leak fixed, rather than the documentation changed, but you never know...)

Sign In or Register to comment.