Prism 5 DelegateCommandBase.RaiseCanExecuteChanged throws InvalidOperationException

Question

I've just updated from Prism 4.1 to 5 and code that used to work fine now throws InvalidOperationExceptions. I suspect that the root cause is that the updated async DelegateCommands don't marshall to the UI thread properly.

I need to be able to call command.RaiseCanExecuteChanged() from any thread and for that to raise the CanExecuteChanged event on the UI thread. The Prism documentation says that that's what the RaiseCanExecuteChanged() method is supposed to do. However, with the Prism 5 update, that no longer works. The CanExecuteChanged event gets called on a non-UI thread and I get downstream InvalidOperationExceptions as UI elements are accessed on this non-UI thread.

Here's the Prism documentation that provides a hint of a solution:

DelegateCommand includes support for async handlers and has been moved to the Prism.Mvvm portable class library. DelegateCommand and CompositeCommand both use the WeakEventHandlerManager to raise the CanExecuteChanged event. The WeakEventHandlerManager must be first constructed on the UI thread to properly acquire a reference to the UI thread’s SynchronizationContext.

However, the WeakEventHandlerManager is static, so I can't construct it...

Does anyone know how I might go about constructing the WeakEventHandlerManager on the UI thread, per the Prism docs?

Here's a failing unit test that reproduces the problem:

    [TestMethod]
    public async Task Fails()
    {
        bool canExecute = false;
        var command = new DelegateCommand(() => Console.WriteLine(@"Execute"),
                                          () =>
                                          {
                                              Console.WriteLine(@"CanExecute");
                                              return canExecute;
                                          });
        var button = new Button();
        button.Command = command;

        Assert.IsFalse(button.IsEnabled);

        canExecute = true;

        // Calling RaiseCanExecuteChanged from a threadpool thread kills the test
        // command.RaiseCanExecuteChanged(); works fine...
        await Task.Run(() => command.RaiseCanExecuteChanged());

        Assert.IsTrue(button.IsEnabled);
    }

And here's the exception stack:

Test method Calypso.Pharos.Commands.Test.PatientSessionCommandsTests.Fails threw exception: System.InvalidOperationException: The calling thread cannot access this object because a different thread owns it. at System.Windows.Threading.Dispatcher.VerifyAccess() at System.Windows.DependencyObject.GetValue(DependencyProperty dp) at System.Windows.Controls.Primitives.ButtonBase.get_Command() at System.Windows.Controls.Primitives.ButtonBase.UpdateCanExecute() at System.Windows.Controls.Primitives.ButtonBase.OnCanExecuteChanged(Object sender, EventArgs e) at System.Windows.Input.CanExecuteChangedEventManager.HandlerSink.OnCanExecuteChanged(Object sender, EventArgs e) at Microsoft.Practices.Prism.Commands.WeakEventHandlerManager.CallHandler(Object sender, EventHandler eventHandler) at Microsoft.Practices.Prism.Commands.WeakEventHandlerManager.CallWeakReferenceHandlers(Object sender, List`1 handlers) at Microsoft.Practices.Prism.Commands.DelegateCommandBase.OnCanExecuteChanged() at Microsoft.Practices.Prism.Commands.DelegateCommandBase.RaiseCanExecuteChanged() at Calypso.Pharos.Commands.Test.PatientSessionCommandsTests.<>c__DisplayClass10.b__e() in PatientSessionCommandsTests.cs: line 71 at System.Threading.Tasks.Task.InnerInvoke() at System.Threading.Tasks.Task.Execute() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter.GetResult() at Calypso.Pharos.Commands.Test.PatientSessionCommandsTests.d__12.MoveNext() in PatientSessionCommandsTests.cs: line 71 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter.GetResult()

Answer 1

I don't know if you still need an answer, but perhaps someone will observe the same error.

So the problem is, as you correctly mentioned, that the RaiseCanExecuteChanged() method does not always post the event handler call to the UI thread's synchronization context.

If we take a look on the WeakEventHandlerManager implementation, we see two things. First, this static class has a private static field:

private static readonly SynchronizationContext syncContext = SynchronizationContext.Current;

And second, there is a private method, which should use this synchronization context and actually post the event handler calls to that context:

    private static void CallHandler(object sender, EventHandler eventHandler)
    {
        if (eventHandler != null)
        {
            if (syncContext != null)
            {
                syncContext.Post((o) => eventHandler(sender,  EventArgs.Empty), null);
            }
            else
            {
                eventHandler(sender, EventArgs.Empty);
            }
        }
    }

So, it looks quite good, but...

As I said before, this call posting happens 'not always'. 'Not always' means, for example, this circumstance:

• your assembly has been built in release configuration and with optimization turned on
• you do not attach a debugger to your assembly

In this situation, the .NET framework optimizes the code execution and, now important, may initialize the static syncContext field at any time but before it is first time being used. So, this is happening in our case - this field gets initialized only when you first call the CallHandler() method (of course indirectly, by calling the RaiseCanExecuteChanged()). And because you may call this method from a thread pool, there is no synchronization context in that case, so the field will just be set to null and the CallHandler() method calls the event handler on current thread, but not on the UI thread.

A solution for this is, from my point of view, a hack or some kind of code smell. I don't like it anyway. You should just ensure that the CallHandler() is first time called from the UI thread, for example, by calling a RaiseCanExecuteChanged() method on a DelegateCommand instance which has valid CanExecuteChanged event subscriptions.

Hope this helps.

Answer 2

Unit Tests ensure that your functionality is not breaking after a code change in any condition, I have seen different approach for Unit Test writing

1. some people writes Unit Test for code coverage.
2. Some guys writes Unit Test only to cover their functionalities or business requirement.

Whatever it is, Unit Test means you are expecting some result based on you inputs. I would suggest you to avoid referring UI components in your Unit Test, because your Test case would not work if you change the Button to some other control, also the async and await modifier is not required. You should use async and await inside DelegateCommand still if you want. Prism 5 support this and you can check the source code in codeplex.

Whenever you calls RaiseCanExecuteChanged it fires the CanExecute delegate attached to your DelegateCommand and try to disable/Enable the UI control. UI controls are in UI thread but your RaiseCanExecuteChanged is in a Worker thread. Normally this breaks your code.

My suggestion is to write test cases to expect below output

1. Execute method should fire if CanExecute method returns true

2. Excute method should not fire if CanExecute method returns false

   [TestMethod]
   public void Fails()
   {
       bool isExecuted = false;
       bool canExecute = false;
       var command = new DelegateCommand(() => 
                                         {
                                            Console.WriteLine(@"Execute");
                                            isExecuted = true;
                                         }
                                         () =>
                                         {
                                             Console.WriteLine(@"CanExecute");
                                             return canExecute;
                                         });
   
       // assert before execute
       Assert.IsFalse(IsExecuted);
       command.RaiseCanExecuteChanged();
       Assert.IsFalse(IsExecuted);
   
       canExecute = true;
       Assert.IsFalse(IsExecuted);
       command.RaiseCanExecuteChanged();
       Assert.IsTrue(IsExecuted);
   }
   

Unit Test always do Assertion to validate the output, So you don't need to mark async and await for your test method