How can I ensure closed dialogs are unregistered from INPC events?

Question

I have an MVVM-Light based WPF application, with a dialog service (called WindowManager) that opens up dialog windows bound to pre-initiated dialog view-models, like this:

private enum ViewModelKind
{
    PlanningGridVM,
    InputDialogVM,
    TreeViewDialogVM,
    SaveFileDialogVM,
    MessageBoxVM
}

/// <summary>
/// Shows the Window linked to this ViewModel as a dialog window.
/// </summary>
/// <typeparam name="TViewModel">The type of the view model.</typeparam>
/// <returns>Tri-state boolean dialog response.</returns>
public bool? ShowDialog<TViewModel>(string key = null)
{
    ViewModelKind name;

    // Attempt to parse the type-parameter to enum
    Enum.TryParse(typeof(TViewModel).Name, out name);

    Window view = null;
    switch (name)
    {
        // removed some irrelevant cases...
        case ViewModelKind.InputDialogVM:
            view = new InputDialogView();
            System.Diagnostics.Debug.WriteLine(
                view.GetHashCode(), "New Window HashCode");
            view.Height = 200;
            result = view.ShowDialog();

        default:
            return true;
    }
}

The dialog's XAML starts with this:

<Window
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:b="clr-namespace:MyCompany.Common.Behaviours"

    x:Class="MyCompany.Common.Views.InputDialogView" mc:Ignorable="d"
    DataContext="{Binding InputDialogVM, Source={StaticResource Locator}}"
    Title="{Binding DisplayName}" MinHeight="200" MinWidth="300" MaxHeight="200"
    b:WindowBehaviours.DialogResult="{Binding DialogResult}"
    WindowStyle="ToolWindow" ShowInTaskbar="False"
    WindowStartupLocation="CenterScreen"
    Height="200" Width="300">

The view-models appropriately register with Messenger in their constructors, and they respond to initialization messages by resetting the view-model properties. This all works as intended.

In order to properly close my "Okay/Cancel" dialogs, I have an attached property called DialogResult, which also works as expected...

/// <summary>
/// DialogResult
/// </summary>
public static readonly DependencyProperty DialogResultProperty = DependencyProperty
    .RegisterAttached(
        "DialogResult",
        typeof(bool?),
        typeof(WindowBehaviours),
        new PropertyMetadata(null, DialogResultChanged));

public static void SetDialogResult(Window target, bool? value)
{
    target.SetValue(DialogResultProperty, value);
}

private static void DialogResultChanged(
    DependencyObject obj, DependencyPropertyChangedEventArgs e)
{
    var window = obj as Window;

    System.Diagnostics.Debug.WriteLine(
        window.GetHashCode(), "Attempting to update DialogResult on Hashcode");

    if (window != null && window.IsActive)
    {
        window.DialogResult = e.NewValue as bool?;
    }
}

...but for one caveat. Did you notice that Debug output I added to track the window instance's HashCode? It confirmed the following for me:

When you have one reusable view-model instance, accessed by the dialog view through a DataContext binding in the XAML, and you sequentially open up a new dialog several times, those dialog instances remain open, even after their OnClosed event has been raised, and even though the dialog is not visible anymore!

The nett effect of this is that I have to check the window's IsActive property in conjunction with checking the window for null. If I don't, the system will try to set window.DialogResult on every dialog phantom that still remains, resulting in a System.InvalidOperationException exception: "DialogResult can be set only after Window is created and shown as dialog".

---

Debug Output

New Window HashCode: 4378943
Attempting to update DialogResult on Hashcode: 4378943

New Window HashCode: 53142588
Attempting to update DialogResult on Hashcode: 53142588

New Window HashCode: 47653507
Attempting to update DialogResult on Hashcode: 53142588
Attempting to update DialogResult on Hashcode: 47653507

New Window HashCode: 57770831
Attempting to update DialogResult on Hashcode: 53142588
Attempting to update DialogResult on Hashcode: 57770831

New Window HashCode: 49455573
Attempting to update DialogResult on Hashcode: 53142588
Attempting to update DialogResult on Hashcode: 57770831
Attempting to update DialogResult on Hashcode: 49455573

New Window HashCode: 20133242
Attempting to update DialogResult on Hashcode: 53142588
Attempting to update DialogResult on Hashcode: 57770831
Attempting to update DialogResult on Hashcode: 49455573
Attempting to update DialogResult on Hashcode: 20133242

---

Question

Many times, I have seen it said that an attached behaviour stores the value of the property specific to an instance. Why is this behaving the opposite way?

It is clear now that those expired dialogs are still registered to the single view-model instance's INPC events. How can I ensure closed dialogs are unregistered from INPC events?

Answer 1

Thanks to Kess for pointing me in the right direction... the problem was never about destroying a dialog instance, but rather to make sure you have a unique view-model instance for each dialog instance! Then you only have to unregister the view-model from Messenger with the built-in Cleanup() method.

The trick is to use method, GetInstance<T>(string key) of ServiceLocator and to pass that key into your WindowManager.

---

Solution

InputDialogView XAML:

Remove the line that assigns DataContext to a ViewModelLocator property

DataContext="{Binding InputDialogVM, Source={StaticResource Locator}}"

WindowManager:

Use the string key passed into ShowDialog and use ServiceLocator (with the key) to fetch the unique view-model, and explicitly set view.DataContext

public bool? ShowDialog<TViewModel>(string key = null)
{
    ViewModelKind name;

    Enum.TryParse(typeof(TViewModel).Name, out name);

    Window view = null;
    switch (name)
    {
        case ViewModelKind.InputDialogVM:
            view = new InputDialogView();

            // Added this line
            view.DataContext = ServiceLocator.Current
                .GetInstance<InputDialogVM>(key);

            view.Height = 200;
            result = view.ShowDialog();

        default:
            return true;
    }
}

MainViewModel:

Create a new view-model with ServiceLocator using a unique string key

internal void ExecuteChangeState()
{
    string key = Guid.NewGuid().ToString();
    InputDialogVM viewModel = ServiceLocator.Current
        .GetInstance<InputDialogVM>(key);

    // Use a nested tuple class here to send initialization parameters
    var inputDialogParams = new InputDialogVM.InitParams(
        "Please provide a reason", "Deactivation Prompt", true);

    // This message sends some critical values to all registered VM instances
    Messenger.Default.Send(new NotificationMessage<InputDialogVM.InitParams>(
        inputDialogParams, CommonMsg.InitInputDialog));

    if (_windowManager.ShowDialog<InputDialogVM>(key) == true)
    {
        var logEntry = new DealsLogEntry()
        {
            LogDateTime = DateTime.Now,
            LogUsername = this.CurrentUser.Username,
            Reason = viewModel.InputString
        };

        _logRepository.Create(logEntry);
    }

    // Unregister this instance from the Messenger class
    viewModel.Cleanup();
}