Is Josh Smith's implementation of the RelayCommand flawed?

Question

Consider the reference article, specifically the example implementation of a RelayCommand (In Figure 3). (No need to read through the entire article for this question.)

In general, I think the implementation is excellent, but I have a question about the delegation of CanExecuteChanged subscriptions to the CommandManager's RequerySuggested event. The documentation for RequerySuggested states:

Since this event is static, it will only hold onto the handler as a weak reference. Objects that listen for this event should keep a strong reference to their event handler to avoid it being garbage collected. This can be accomplished by having a private field and assigning the handler as the value before or after attaching to this event.

Yet the sample implementation of RelayCommand does not maintain any such to the subscribed handler:

public event EventHandler CanExecuteChanged
{
    add { CommandManager.RequerySuggested += value; }
    remove { CommandManager.RequerySuggested -= value; }
}

1. Does this leak the weak reference up to the RelayCommand's client, requiring that the user of the RelayCommand understand the implementation of CanExecuteChanged and maintain a live reference themselves?

2. If so, does it make sense to, e.g., modify the implementation of RelayCommand to be something like the following to mitigate the potential premature GC of the CanExecuteChanged subscriber:

   // This event never actually fires.  It's purely lifetime mgm't.
   private event EventHandler canExecChangedRef;
   public event EventHandler CanExecuteChanged
   {
       add 
       { 
           CommandManager.RequerySuggested += value;
           this.canExecChangedRef += value;
       }
       remove 
       {
           this.canExecChangedRef -= value;
           CommandManager.RequerySuggested -= value; 
       }
   }
   

Answer 1

I may be missing the point here but doesn't the following constitute the strong reference to the event handler in the contructor?

    _canExecute = canExecute;           

Answer 2

Well, according to Reflector it's implemented the same way in the RoutedCommand class, so I guess it must be OK... unless someone in the WPF team made a mistake ;)

Answer 3

I believe it is flawed.

By rerouting the events to the CommandManager, you do get the following behavior

This ensures that the WPF commanding infrastructure asks all RelayCommand objects if they can execute whenever it asks the built-in commands.

However, what happens when you wish to inform all controls bound to a single command to re-evaluate CanExecute status? In his implementation, you must go to the CommandManager, meaning

Every single command binding in your application is reevaluated

That includes all the ones that don't matter a hill of beans, the ones where evaluating CanExecute has side effects (such as database access or long running tasks), the ones that are waiting to be collected... Its like using a sledgehammer to drive a friggen nail.

You have to seriously consider the ramifications of doing this.