MVVMLight CanExecute not working until window click

Question

Quick note so I do not waste anyone's time. When installing MVVMLight from nuget I eventually get an error null : The term 'null' is not recognized as the name of a cmdlet, function, script file, or operable program. MVVMLight seems to work fine despite this except the issue that will be described below, but I wanted to mention this just in case.

---

The Problem

I am experiencing an issue with buttons not re-enabling after command execution completes. It seems they sometimes work if the execution is very fast, but any operation that takes a while seems to never work. This screams of race condition.

I am using Task to execute the lengthy operations so the UI can update. The first and last step of the Task is to flip IsBusy appropriately (where each CanExecute method returns !IsBusy;)

I have built a simple example that will use Thread.Sleep to simulate slow operations and it showcases the problem quite well. The WaitOneSecondCommand seems to work intermittently. The WaitTenSecondsCommand and WaitThirtySecondsCommand never work.

By does not work I mean the buttons remain disabled until I click somewhere on the form.

---

Things I've tried

I have done a fair amount of research and the solutions I have tried so far have not changed this behaviour. I eventually resulted to brute forcing all the different "fixes" in case I am misunderstanding.

One thing I tried was expanding the RelayCommands to raise property changed. As an example:

From:

public RelayCommand WaitOneSecondCommand {
    get; set;
}

To:

public RelayCommand WaitTenSecondsCommand {
    get {
        return _waitTenSecondsCommand;
    }

    set {
        _waitTenSecondsCommand = value;
        RaisePropertyChanged();
    }
}

I did not expect it to work, but I wanted to try it. I also tried adding WaitTenSecondsCommand.RaiseCanExecuteChanged();

I also tried adding WaitTenSecondsCommand.RaiseCanExecuteChanged() to the CommandExecute methods, but that also did not change anything.

private void WaitTenSecondsCommandExecute() {
    Task.Run(() => {
        IsBusy = true;
        Thread.Sleep(10000);
        IsBusy = false;
        WaitTenSecondsCommand.RaiseCanExecuteChanged();
    });
}

I also read about the CommandManager, so I added CommandManager.InvalidateRequerySuggested() as well. I added it to the IsBusy thinking this would be very spammy, but I figured it would remove any doubt

Again this smells of race condition, and I am using Task here, but these tasks cannot run at the same time, and cannot conflict with each other due to the IsBusy flag being used.

---

Full code

This is a basic WPF application using .Net 4.6.1 with MVVMLight 5.2.0 installed from Nuget.

MainWindow.xaml

<Window x:Class="StackOverflowExample.View.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        mc:Ignorable="d"
        Title="MainWindow" MinHeight="100" Width="150" ResizeMode="NoResize" SizeToContent="Height"
        DataContext="{Binding Source={StaticResource Locator}, Path=Main}">
    <StackPanel>
    <Button Height="70" Margin="5" Command="{Binding WaitOneSecondCommand}">Wait 1 Second</Button>
    <Button Height="70" Margin="5" Command="{Binding WaitTenSecondsCommand}">Wait 10 Seconds</Button>
    <Button Height="70" Margin="5" Command="{Binding WaitThirtySecondsCommand}">Wait 30 Seconds</Button>
    </StackPanel>
    <Grid>
        <Label HorizontalAlignment="Left" Width="84">IsBusy:</Label>
        <TextBox IsReadOnly="True" HorizontalAlignment="Right" Width="45" Text="{Binding IsBusy}" Margin="4,5,5,5" />
    </Grid>
</Window>

I've added the binding for IsBusy to clearly show that it is updating properly. It's also the only reliable way to know when the 10 and 30 second Commands complete. I do not want any MessageBoxes that force you to click OK causing the CanExecute to update. That is a bandaid fix.

MainViewModel.cs

public class MainViewModel : ViewModelBase {
    private bool _isBusy;
    private RelayCommand _waitTenSecondsCommand;

    public MainViewModel() {
        WaitOneSecondCommand = new RelayCommand(WaitOneSecondCommandExecute, WaitOneSecondCommandCanExecute);
        WaitTenSecondsCommand = new RelayCommand(WaitTenSecondsCommandExecute, WaitTenSecondsCommandCanExecute);
        WaitThirtySecondsCommand = new RelayCommand(WaitThirtySecondsCommandExecute, WaitThirtySecondsCommandCanExecute);
    }

    public RelayCommand WaitOneSecondCommand {
        get; set;
    }

    public RelayCommand WaitTenSecondsCommand {
        get {
            return _waitTenSecondsCommand;
        }

        set {
            _waitTenSecondsCommand = value;
            RaisePropertyChanged();
            WaitTenSecondsCommand.RaiseCanExecuteChanged();
        }
    }

    public RelayCommand WaitThirtySecondsCommand {
        get; set;
    }

    public bool IsBusy {
        get {
            return _isBusy;
        }

        set {
            _isBusy = value;
            RaisePropertyChanged();
            CommandManager.InvalidateRequerySuggested();
        }
    }

    private void WaitOneSecondCommandExecute() {
        Task.Run(() => {
            IsBusy = true;
            Thread.Sleep(1000);
            IsBusy = false;
        });
    }

    private void WaitTenSecondsCommandExecute() {
        Task.Run(() => {
            IsBusy = true;
            Thread.Sleep(10000);
            IsBusy = false;
            WaitTenSecondsCommand.RaiseCanExecuteChanged();
        });
    }

    private void WaitThirtySecondsCommandExecute() {
        Task.Run(() => {
            IsBusy = true;
            Thread.Sleep(30000);
            IsBusy = false;
        });
    }

    private bool WaitOneSecondCommandCanExecute() {
        return !IsBusy;
    }

    private bool WaitTenSecondsCommandCanExecute() {
        return !IsBusy;
    }

    private bool WaitThirtySecondsCommandCanExecute() {
        return !IsBusy;
    }
}

Note that in the viewmodel I only put a backing field on WaitTenSeconds to showcase that it does not change the behaviour.

Answer 1

Full credit goes to Viv over in this question: https://stackoverflow.com/a/18385949/865868

The problem I am facing is that I am updating IsBusy in a separate thread and the main UI thread obviously is depending on it to update the buttons.

Update from 2020!

I wanted to revisit this answer because my understanding was flawed, though I feel there is still a better way. The goal should have been to not modify the setting of IsBusy, or any supporting property at all. Also, async void should be avoided.

The corresponding CommandExecute method now simply makes use of async await

private async Task WaitOneSecondCommandExecute() {
    CanWaitOneSecond = true;
    await Task.Delay(1000);
    CanWaitOneSecond = false;
}

And I made this change to "CanWaitOneSecond"

public bool CanWaitOneSecond {
    get => _canWaitOneSecond ;
    set {
        _canWaitOneSecond  = value;
        Set(() => CanWaitOneSecond , ref _canWaitOneSecond , value);
        RaiseAllCanExecuteChanged();
     }
}

public void RaiseAllCanExecuteChanged() {
    foreach (var command in Commands) {
        command.RaiseCanExecuteChanged();
    }
}

public List<RelayCommand> Commands {
    get;
}

I also made a slight change to the constructor where I set up the RelayCommand to support async Task instead of async void

Commands = new List<RelayCommand>();
Commands.Add(WaitOneSecondCommand = new RelayCommand(async () => await WaitOneSecondCommandExecute(), WaitOneSecondCommandCanExecute));

I hope this saves someone a lot of time researching, and please if there are any issues with this implementation please let me know. I still hope to clean this up

Answer 2

You need to trigger the 'RaiseCanExecuteChanged' from the UI thread. With a Task that might not be the case. The easiest way for you to see if this would fix the problem would be to add this to the IsBusy setter. Note that this is not how you should structure your app (having this in the IsBusy setter) but rather you should detect the completion of the task and do it there.

public bool IsBusy {
    get {
        return _isBusy;
    }

    set {
        _isBusy = value;
        RaisePropertyChanged();
        Application.Current.Dispatcher.Invoke(
                    DispatcherPriority.ApplicationIdle,
                    new Action(() => {
                        WaitOneSecondsCommand.RaiseCanExecuteChanged();
                        WaitTenSecondsCommand.RaiseCanExecuteChanged();
                        WaitThirtySecondsCommand.RaiseCanExecuteChanged();
                    }));        }
}