RelayCommand on view still being evaluated after ShowDialog is closed

Question

I execute LogInRequest() which calls LogInView.ShowDialog(). That view has a Command called VerifyLogInCommand. When the Command is executed, it completes with this.CloseAction() which appears to close the Dialog. However, my breakpoint in that view's command's CanExecute method VerifyLogInCanExecute is still being hit after the dialog is closed (nonstop). I've tried setting the view to null after calling ShowDialog but no change.

Why is the Command/CanExecute still being evaluated when the window is closed/null?

LogInView.xaml.cs

public LogInOutView()
{
    InitializeComponent();

    // Data context
    IModule existingVM = SessionViewModel.Instance.ModulesOpen.Single(mod => mod.ModuleName == "LogIn");
    LogInViewModel livm = (LogInViewModel)existingVM;
    this.DataContext = livm;

    // Cancel Handler
    livm.CloseAction = new Action(() => this.Close());
}

LogInViewModel.cs

public Action CloseAction { get; set; }

private RelayCommand verifyLogInCommand;
public RelayCommand VerifyLogInCommand
{
  get
  {
    if (verifyLogInCommand == null)
    {
      verifyLogInCommand = new RelayCommand(
        param => VerifyLogInExecute(),
        param => VerifyLogInCanExecute);
    }
    return verifyLogInCommand;
  }
}

public void VerifyLogInExecute()
{
  // Validate Login
  Employee employee = ValidateLogin(Password);

  // Clear password field
  ResetExecute();

  // Return false if invalid login
  if (employee == null)
  {
    Result = LogInOutDialogResults.Cancel;
    ConfirmationView c = new ConfirmationView("Invalid Login!");
    c.ShowDialog();
    return;
  }

  // Set Result to LogIn status
  Result = LogInOutDialogResults.LogIn;

  // Set LastAuthorizedEmployee
  SessionViewModel.Instance.LastAuthorizedEmployee = employee;

  // Close View to go back where it was called
  this.CloseAction();
}

public bool VerifyLogInCanExecute
{
  get
  {
    // Password length restrictions
    if (!CheckRequiredPasswordLength(Password)) { return false; }
    return true;
  }
}

public static LogInOutDialogResults LogInRequest()
{
  // Show Login View
  LogInOutDialogResults LogInOutResult = LogInOutDialogResults.Cancel;
  LogInOutView LogInOutView = new LogInOutView()
  {
    Title = "Log In",
    ShowInTaskbar = false,
    Topmost = true,
    ResizeMode = ResizeMode.NoResize,
    Owner = SessionViewModel.Instance.ProfitPOSView
  };
  LogInOutView.ShowDialog();
  LogInOutResult = ((LogInViewModel)LogInOutView.DataContext).Result;

  // LogIn
  if (LogInOutResult == LogInOutDialogResults.LogIn)
  {
    LogInOutView = null;
    return LogInOutDialogResults.LogIn;
  }
}

Mike Strobel · Accepted Answer · 2013-10-30T16:04:13.773

If you are using the RelayCommand from MvvmLight, it implements its CanExecuteChanged event by forwarding subscriptions to CommandManager.RequerySuggested. This effectively allows a RelayCommand to update its own status the way a RoutedCommand does in WPF; the RequerySuggested event fires on certain conditions, including every time the focus changes or a window is (de)activated. The RequerySuggested event uses weak event handlers to mitigate leaked subscriptions, but the weak event implementation used by WPF isn't terribly diligent about cleaning up after itself, so the subscription may still remain active for a while (perhaps even indefinitely).

Your CanExecute callback appears to be reevaluating non-stop because every time you hit a breakpoint, Visual Studio steals focus from your application, and when you hit 'Resume', your application is reactivated, thus triggering the RequerySuggested event and causing CanExecute to be reevaluated. That, in turn, triggers the breakpoint again, and you get caught in a cycle until you disable the breakpoint.

If your View Model is aware of its closed state, I would change your VerifyLogInCanExecute property to something like:

public bool VerifyLogInCanExecute
{
    get { return !IsClosed && CheckRequiredPasswordLength(Password); }
}

At least then you will not be doing more work than is necessary. Another option would be to set your login command to null (or an empty/no-op command) when the View Model is closed (and raise the appropriate PropertyChanged event). That will cause any buttons bound to the command to unsubscribe from its CanExecuteChanged event.

Thanks for the great explanation. I just found similar explanation of setting the Command to null when done [here as well](http://stackoverflow.com/a/6695530/1992193) — Brock Hensley, Oct 30 '13 at 16:15
I've the same issue, however it happens in my project, even after I'm setting the command to null, any ideas? — Chen, May 23 '16 at 12:04
@Chen are you raising `PropertyChanged` when you null-out your command? If not, the bound control doesn't know the command has been replaced, and will continue to query the old one. — Mike Strobel, May 23 '16 at 14:27
I've a view model that I'm trying to cleanup, one of the things that I do is to null all command. After the cleanup I get called in the can execute. What is the best way to avoid it? Thank you. — Chen, May 24 '16 at 17:39

score 1 · Answer 2 · answered Oct 30 '13 at 16:15

Upon what Mike Storbl pointed out about Mvvm light's RelayCommand which uses

 CommandManager.RequerySuggested

which is very expansive performance wise , and is prone to allot of bugs since it raises

  CanExecuteChangedEvent

every time the VisualTree is focused .

You should implement your own as such :

public class RelayCommand : ICommand
{
    private Func<bool> _canExecute;
    private Action _execute;

    public RelayCommand(Action execute , Func<bool> canExecute = null)
    {
        _execute = execute;
        _canExecute = canExecute;
    }

    public bool CanExecute(object parameter)
    {
        return _canExecute == null ? true : _canExecute();
    }

    public event EventHandler CanExecuteChanged;

    public void RaiseCanExecuteChanged()
    {
        var temp = Volatile.Read(ref CanExecuteChanged);

        if (temp != null)
            temp(this, new EventArgs());
    }

    public void Execute(object parameter)
    {
        _execute();
    }
}

and raise it from your code when needed , for instance when Password was changed.

  verifyLogInCommand.RaiseCanExecuteChanged();

RelayCommand on view still being evaluated after ShowDialog is closed

2 Answers2