Unsubscribe event using dispose in MVVM

Question

Actually I m trying to close my window by firing the event from my ViewModel. Everything works fine and awesome, but I know that I must unsbscribe my event to avoid memory leaks. thus I implement the IDisposable interface and I unsbscribe the event inside the Dispose method.

Below is my code :

public partial class MainWindow : Window, IDisposable
{
    private MainViewModel viewModel;
    public MainWindow()
    {
        InitializeComponent();
        DataContext = viewModel =  new MainViewModel();
        this.viewModel.RequestClose += CloseWindow;
    }

    void CloseWindow(object sender, EventArgs e)
    {
        this.Close();
    }

    public void Dispose()
    {
        ////here we need to unsubscribe the event
        this.viewModel.RequestClose -= this.CloseWindow;
    }
}

What I need to know :

1. Is that code correct
2. When the GC will be called and excute the dispose method
3. Is there a better way to do such a thing

Answer 1

but I know that I must unsbscribe my event to avoid memory leaks

Memory leak occurs, when short-lived object subscribes an event of long-lived objects (or static event), and does not unsubscribe later (e.g., see this answer). I suppose, that this is not your case.

When the GC will be called and excute the dispose method

GC doesn't call IDisposable.Dispose (e.g., see this answer). At all. If you haven't any code, that calls MainWindow.Dispose explicitly, it will be called never.

Is there a better way to do such a thing

I'd avoid IDisposable and events. Attached behavior here is more convenient, IMO (at least, this is reusable):

public static class WindowClosingBehavior
{
        public static bool GetIsClosingInitiated(DependencyObject obj)
        {
            return (bool)obj.GetValue(IsClosingInitiatedProperty);
        }

        public static void SetIsClosingInitiated(DependencyObject obj, bool value)
        {
            obj.SetValue(IsClosingInitiatedProperty, value);
        }

        public static readonly DependencyProperty IsClosingInitiatedProperty = DependencyProperty.RegisterAttached(
            "IsClosingInitiated", 
            typeof(bool), 
            typeof(WindowClosingBehavior),
            new FrameworkPropertyMetadata(false, FrameworkPropertyMetadataOptions.BindsTwoWayByDefault, IsClosingInitiatedChanged));

        private static void IsClosingInitiatedChanged(DependencyObject target, DependencyPropertyChangedEventArgs e)
        {
            var window = target as Window;
            if (window != null && (bool)e.NewValue)
            {
                window.Close();
            }
        }
}

Somewhere in window's XAML:

behaviors:WindowClosingBehavior.IsClosingInitiated="{Binding IsClosingInitiated}"

where IsClosingInitiated is a property from view model:

public class SomeViewModel
{
     // ...

     private void Foo()
     {
         // ...
         IsClosingInitiated = true;
     }
}

Answer 2

You only need to unsubscribe events when the source and the handler have different lifetimes, otherwise they both go out of scope at the same time and they are garbage collected together.

So in this case IDisposable is not needed. Anyway if you implement IDisposable you need to explicitly call it, otherwise you don't have control about when it is called.

Answer 3

Actually when the Window.CloseWindow subscribe to the event, it makes the view model point to the window.

The reverse is also true because there is a ViewModel field in the Window.

Both window and view model reference to each other.

If there is no other reference to them, garbage collection will make the job.

Dispose will be called if some code calls it.

To my knowledge, it won't happen, unless you surround the creation of the windows with a using or explicitly call Dispose

The best way here is to not implement IDisposable / Dispose : keep it simple.

Regards

Answer 4

I'd say using an event is a more than acceptable method of achieving this. For a more complete dispose pattern, use the following snippet:

#region IDisposable

//Dispose() calls Dispose(true)
public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}

// NOTE: Delete the finalizer if this class doesn't 
// own unmanaged resources itself.
~ClassName() 
{
    //Finalizer calls Dispose(false)
    Dispose(false);
}

//The bulk of the clean-up code is implemented in Dispose(bool)
protected virtual void Dispose(bool disposing)
{
    if (disposing) 
    {
        //free managed resources (Example below)
        if (managedResource != null)
        {
            managedResource.Dispose();
            managedResource = null;
        }
    }

    //Free native resources if there are any. (Example below)
    if (nativeResource != IntPtr.Zero) 
    {
        Marshal.FreeHGlobal(nativeResource);
        nativeResource = IntPtr.Zero;
    }
}

#endregion

In your case, your dispose method will be this:

public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}

~MainWindow()
{
    Dispose();
}

protected virtual void Dispose(bool disposing)
{
    if (disposing) 
    {
        if (viewModel != null)
        {
            viewModel.RequestClose -= CloseWindow;
            viewModel.Dispose();
            viewModel = null;
        }
    }
}

As pointed out by Dennis, you'll need to keep the finalizer to ensure that Dispose gets called when the MainWindow is closed, for example in the event of the application being exited.