Is this an appropriate way to ensure Xamarin Prism Subscriptions get unsubscribed?

Question

I have a Xamarin/Prism app (not my code) that has various event subscriptions, and a number of memory leaks. I think one of the memory leaks has to do with these event subscriptions. Here is what they used to look like:

HomePage.xaml.cs

public HomePage(IEventAggregator eventAggregator)
{
    InitializeComponent();
    _eventAggregator = eventAggregator;
    _eventAggregator.GetEvent<MyEvent>().Subscribe(async(x) =>
    {
        await MyAction(x);
    });
}

protected override void OnDisappearing()
{
    _eventAggregator.GetEvent<MyEvent>().Unsubscribe(UnrelatedHandleMyEvent);
}

private void UnrelatedHandleMyEvent(MyParameters obj)
{
    Debug.WriteLine(obj);
}

HomePageViewModel.cs

public HomePageViewModel(IEventAggregator eventAggregator)
{
    _eventAggregator = eventAggregator;
    _eventAggregator.GetEvent<MyOtherEvent>().Subscribe(async (x) =>
    {
        await MyOtherAction(x);
    });
}

public override void OnNavigatedFrom(INavigationParameters parameters)
{
    _eventAggregator.GetEvent<MyOtherEvent>().Unsubscribe(UnrelatedHandleMyOtherEvent);
}

private void UnrelatedHandleMyOtherEvent(MyOtherParameters obj)
{
    Debug.WriteLine("Unsubscribing from other Event");
}

I want to replace these with the following:

HomePage.xaml.cs

SubscriptionToken myEventToken;

public HomePage(IEventAggregator eventAggregator)
{
    InitializeComponent();
    _eventAggregator = eventAggregator;
}

protected override void OnAppearing()
{
    myEventToken = _eventAggregator.GetEvent<MyEvent>().Subscribe(async(x) => { await MyAction(x); });
}

protected override void OnDisappearing()
{
    myEventToken.Dispose()
}

HomePageViewModel.cs

SubscriptionToken myOtherEventToken;

public HomePageViewModel(IEventAggregator eventAggregator)
{
    _eventAggregator = eventAggregator;
}

public override void OnNavigatedTo(INavigationParameters parameters)
{
    myOtherEventToken = _eventAggregator.GetEvent<MyOtherEvent>().Subscribe(async (x) => { await MyOtherAction(x); });
}

public override void OnNavigatedFrom(INavigationParameters parameters)
{
    myOtherEventToken.Dispose();
}

My two main changes are replacing the Unsubscribe calls with Dispose calls on the now-saved SubscriptionTokens, and moving the Subscribe calls to the corresponding page lifecycle hooks, rather than constructors. I am quite confident in the Unsubscribe fix, I'm sure that's the main cause of the memory leaks, however I am not sure about moving from the constructors to the lifecycle hooks - everywhere I look online seems to have them in constructors, however this doesn't sit right with me. Lifecycle hooks should be symmetrical and one-to-one - as opposed to constructors.

I am on Xamarin.Forms 4.8, and Prism.DryIoc.Forms 7.2

Am I doing it right?

Answer 1

There's no problem with subscribing outside the constructor, might well not make sense to receive an event before being navigated to :-)

You'll want to have a look at IApplicationLifecycleAware and IPageLifeCycleAware because OnNavigatedTo and OnNavigatedFrom are only called when actually navigating, and not when, e.g. closing the app or pressing the hardware back button.

When the app goes into background and is tapped on later, it may start from scratch or just be resumed, so be aware of possible double-subscriptions.