ContentPage not removing XAML bindings when popped from NavigationPage?

DevologyDevology GBMember ✭✭

Hi,

I'm using the MVVM pattern with XamarinForms, so I create a model, assign it to the Xamarin Page (the page is a subclass of ContentPage) and because of the bindings declared in XAML everything gets registered beautifully.

However, if I can push and popping the same view on/off a NavigationPage (say 10 times) and then change a visual element (i.e. by typing in a bound text field), I get a console output for all the old bindings as well as the current binding. I have a base class for my MVVM models that includes the logging...

    public void PropChanged(String property) {
        if (PropertyChanged != null)
        {
            Console.WriteLine(this.GetType().ToString()+" PropChanged:" + property);
            PropertyChanged(this,new PropertyChangedEventArgs(property));
        }
    }

The only way I have so far been able to resolve the issue is to modify the Page to include an override OnDissapearing method, I tried to call UnapplyBindings() but that didn't help, however, setting the BindingContext to null has done the trick...

    protected override void OnDisappearing()
    {
        base.OnDisappearing();

        // Only this seems to remove the bindings created from XAML from the ViewModel
        this.BindingContext = null; // <--- YAY!!! Bindings now removed

        this.ListDetailViewModel.Dispose(); // <-- Making sure the view model also tidies up
        this.ListDetailViewModel = null;

    }

My question is, shouldn't the ContentPage OnDisappearing be removing all the bindings anyway?

Although what I'm doing works, is this the right approach?

Given that ContentPage.UnapplyBindings doesn't remove the bindings, but setting ContentPage.BindingContext=null does, what the heck does UnapplyBindings actually do then?

You might have noticed that in addition, I delegate through to my view model to call Dispose which has some clean up code, is this 'okay', or should I simply have a method I call something like CleanUp - I'm not sure when the Dispose is 'meant' to be called, but it would seem it doesn't unless I manually invoke it.

Thanks.

Answers

  • I have similar memory leak issues when navigating across multiple instances of the same page. It seems that references are not being cleaned/disposed properly when a page gets removed from the navigation stack.

    This is what I do to get some memory back but it doesn't help much:

    nav.PushAsync(view);
    nav.Navigation.RemovePage(oldPage);
    var oldVM = (ViewModel)oldPage.BindingContext;
    oldPage.BindingContext = null;
    oldVM.Dispose();
    oldVM = null;
    GC.Collect();

    Wouldn't be possible to have a Dispose method that cleans view instance's resources and call it automatically when views are discarded?

  • DevologyDevology GBMember ✭✭

    Hi @ManuelEspinosa.0395 Here are some resources I've found useful since I fast started with c# / Xamarin Forms.

    Instead if invoking Dispose yourself, you can use 'using' blocks, or you can use a pattern something like this in your classes.

    (source: https://stackoverflow.com/questions/6967108/is-idisposable-dispose-called-automatically)

    class Foo : IDisposable
    {
        private bool m_disposed = false;
    
        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }
    
        ~Foo()
        {
            Dispose(false);
        }
    
        protected void Dispose(bool disposing)
        {
            if (!m_disposed)
            {
                if (disposing)
                { 
                    //release managed resources
                }
                //release unmanaged resources
    
                m_disposed = true;
            }
        }
    }
    

    I would imagine that you might have some event listeners still registered, so you'll want to ensure they are de-registered, pay particular attention to lambda functons being assigned to events - you will want to hold a reference to these so you can de-register them; convienience comes at a cost otherwise. There's a good post here (https://developer.xamarin.com/guides/android/deployment,_testing,_and_metrics/performance/#removeeventhandlers)

    Also found this tip here (https://forums.xamarin.com/discussion/61872/hunting-memory-leaks-best-practices)...

    Event handlers are definitely something to watch, can be leaky if you don't unwire them.
    Tip: If there is a risk of leaking, you can turn them into an IObservable using Observable.FromEvent and then you can dispose the subscribers using an IDisposable.
    

    Plus, consider using CachedImage or FFImageLoading insted of Image.

    Whilst I haven't read this blog yet (https://igorelikblog.wordpress.com/2016/07/08/xamarin-form-memory-management/) it looks like a goldmine of inofrmation to track down a memory leak.

    I'll have to admit that I've been cheating recently, my views / pages generally only need to have one instance each, so I've started using Ninject (Inversion of Contorl) to register them as singletons, this way, whenever I want to get hold of a page to add it to the navigation stack, I know that I wont be creating a new one each time. Lazy? yes, but also pragmatic ;-)

  • Thanks for the info Rob. I changed all my event handlers so that all lambdas are now instance members and all handlers are properly detached from their events. Unfortunately, memory is still going up every time a go to a new instance of the page.

    The Ninject approach you suggest wouldn't work in my application because it creates a new instance of the view/viewmodel to point to the item that was chosen in a MasterDetail page.

    Using VS Diagnostic Tools, I found that the major increase in heap size and object instances come from
    Xamarin.Forms.BindableObject+BindablePropertyContext with
    Count diff: +7,249 (instances)
    Size diff: +202,388 (bytes)

    But against all odds, those numbers are not coming back as I navigate through the app (pages are getting popped out from Navigation stack and objects disposed and cleared as mentioned before).

    I am about to try the solution described here: https://stackoverflow.com/a/43191356, which points to the Dispose method the PageRenderer.

    Do you know if there's anyway to get the renderer associated to a current View?

  • DevologyDevology GBMember ✭✭

    "Do you know if there's anyway to get the renderer associated to a current View?"...

    I'm not sure that I follow, if you're adding events in the page/view then you can remove them there too, I'm not sure why you need to dig deep into the platform renderers themselves. Based on the CustomPageRenderer then presumably you could do something here...

    private void OnPagePopped(object sender, NavigationEventArgs args)
        {
            if (args.Page != Element)
            {
                return;
            }
    
            Dispose(true); 
    
        args.Page.DoSomething(); // <<<<--- MAYBE HERE??
    
            _navigationPage.Popped -= OnPagePopped;
        }
    

    But to me it feels hacky, perhaps you can just fire an event when popping and listen to it elsewhere?

    How are you registering your MVVM property change listeners, maybe you need to deregister those too?

  • batmacibatmaci DEMember ✭✭✭✭✭

    @Devology did you implement IDisposable on your viewmodel ? is it necessary to implement it?

    this.ListDetailViewModel.Dispose(); // <-- Making sure the view model also tidies up
    this.ListDetailViewModel = null;

  • DevologyDevology GBMember ✭✭

    Hi @batmaci I didn't override the Dispose method and I don't think that you need to - I had read a lot of articles about the use of Dispose and concluded it wasn't needed (sorry I don't remember the exact reasons why anymore). If your model creates stuff that doesn't go out of scope naturally when garbage collected then maybe you will need to.

Sign In or Register to comment.