EventHandler not being removed despite being unsubscribed?

JamesLaveryJamesLavery GBBeta, University ✭✭✭✭✭

We've got a Xamarin.Forms Android app in which we're displaying progress on a loading page, the progress value being sent by an event from another class.

We're using FreshMvvm which has ViewIsAppearing and ViewIsDisappearing overrides available in the PageModel.

So we're subscribing on ViewIsAppearing, and unsubscribing in ViewIsDisappearing - we're also unsubscribing in a PrepareForDispose method which is intended to ensure the PageModel has cleaned up so that it can be disposed.

Code is below. ProgressManager is supplied by IoC

        protected override void ViewIsAppearing (object sender, EventArgs e)
        {
            base.ViewIsAppearing (sender, e);

            ProgressManager.ProgressEvent += ProgressManager_ProgressEvent;

        }

        protected override void ViewIsDisappearing (object sender, EventArgs e)
        {
            base.ViewIsDisappearing (sender, e);

            RemoveEventHandlers();
        }

        public override void PrepareForDispose()
        {
            RemoveEventHandlers();
            base.PrepareForDispose();
        }

        private void RemoveEventHandlers()
        {
            ProgressManager.ProgressEvent -= ProgressManager_ProgressEvent;
        }

The problem is that, when examining object in Profiler, we can see the LoadingPageModel is still in memory, because of the EventArgs created in the ViewIsAppearing (examining the "Paths To Root" in Profiler tells us this).

When I log/debug the app, I can see that RemoveEventHandlers has been called.

So is ProgressManager.ProgressEvent -= ProgressManager_ProgressEvent; failing to remove the handler, or is there another reason that we've still got a reference to the PageModel from the EventArgs?

Best Answer

  • JamesLaveryJamesLavery GB ✭✭✭✭✭
    Accepted Answer

    It turns out that we were subscribing twice (due to the lifecyle of the page in question at the start of the app lifecycle). Therefore the single unsubscribe was leaving a subscription behind.

    Fixing this fixed the problem.

Answers

  • JamesLaveryJamesLavery GBBeta, University ✭✭✭✭✭
    Accepted Answer

    It turns out that we were subscribing twice (due to the lifecyle of the page in question at the start of the app lifecycle). Therefore the single unsubscribe was leaving a subscription behind.

    Fixing this fixed the problem.

  • DarenPannulloDarenPannullo USMember

    Hi James, I'm working on a similar memory leak issue. I'm using Visual Studio for MAC Community. How is it that you are able to use the Xamarin Profiler with the Community Edition?

    Thanks,
    Daren

  • JohnHardmanJohnHardman GBUniversity mod
    edited March 2018

    @JamesLavery - To defend against that problem, the way I tend to do the addition of an event handler is as follows:

                ProgressManager.ProgressEvent -= ProgressManager_ProgressEvent;
                ProgressManager.ProgressEvent += ProgressManager_ProgressEvent;
    

    That's not ideal, as it doesn't highlight to me when I am accidentally calling that twice. I keep meaning to think about a pattern that will throw an exception if I do call that twice without removing the event handler in between, but without repeating the same code all over the place, and without doing something that would have a noticeable performance impact. One day...

    (the obvious way of doing the check on pre-defined events would involve reflection, which would be ok in a debug build for testing, but would have a bit of a performance hit)

  • JamesLaveryJamesLavery GBBeta, University ✭✭✭✭✭

    @JohnHardman Yes that's what we've done - but as you say it doesn't highlight areas where your subscribing twice unintentionally (or rather the code which does the subscription is running twice unintentionally).

Sign In or Register to comment.