WPF MultiThreading: How can this still cause an exception? (Careful with that Dispatcher!)

Question

I have the following code in a message handler (that can be invoked on any thread):

private readonly Dictionary<string,IView> _openedViews = new Dictionary<string,IView>();
private readonly object _lockObject = new object();

public MainView() 
{
    Messenger.Default.Register<ViewChangeMessage>(this, "adminView", m =>
    {
         var key = m.ViewName;
         lock (_lockObject)
         {
            if (_openedViews.ContainsKey(key) == false)
                _openedViews.Add(key, GetView(key));
           content.Content = _openedViews[key];
         }
         //...
    });
    //...

How can I still get this exception: An element with the same key already exists in the System.Collections.Generic.Dictionary<TKey,TValue>.

The exception is produced if I rapidly cause the message to be sent multiple times.

EDIT: added more context to the code, Messenger is from Galasoft.MVVMLight

Depends how you set your handler. This code is not enough. Maybe `_openedViews` is static and `_lockObject` is not? Need more detail. — Grzegorz W, Jun 13 '12 at 06:27

score 1 · Answer 1 · answered Jun 13 '12 at 07:43

1

Well in that code you posted I don't see any data race.

If GetView cannot cause a data race you could try to replace that entire block of locked code with a ConcurrentDictionary.GetOrAdd:

private readonly ConcurrentDictionary<string,IView> _openedViews = 
          new ConcurrentDictionary<string,IView>();

public MainView() 
{
    Messenger.Default.Register<ViewChangeMessage>(this, "adminView", m =>
    {
         var key = m.ViewName;
         content.Content = _openedViews.GetOrAdd(key, GetView(key));
         //...
    });
    //...

answered Jun 13 '12 at 07:43

Tudor

61,523
12
102
142

Better yet, use the `GetOrAdd` overload that accepts a `Func` to avoid unnecessary instantiation of the view – Steve Greatrex Jun 13 '12 at 07:53
@Tudor: Thanks for reassuring my thinking, that there's no real race condition in my code. It was really a thread affinity + dispatcher mixup, see my posted answer – TDaver Jun 13 '12 at 13:21

score 0 · Answer 2 · answered Jun 13 '12 at 07:11

0

Have you made sure that all threads are using the same instance of lockObject? If they're not then it won't stop multiple threads getting to your add code.

answered Jun 13 '12 at 07:11

Andy

6,366
1
32
37

given that both _openedViews and _lockObject are private readonly instance fields of the same class, no, I don't think so. – TDaver Jun 13 '12 at 07:13

score 0 · Answer 3 · answered Jun 13 '12 at 08:59

0

Move var key = m.ViewName; inside lock statement.

answered Jun 13 '12 at 08:59

Grzegorz W

3,487
1
21
21

score 0 · Accepted Answer · answered Jun 13 '12 at 13:19

Here is what happened: the GetView instantiated a view, which had a long running operation somewhere (waiting on a background thread), and so that waiting wouldn't lock the UI up someone introduced this code:

public static void DoEvents()
{
    DispatcherFrame frame = new DispatcherFrame();
    Dispatcher.CurrentDispatcher.BeginInvoke(DispatcherPriority.Background,
        new DispatcherOperationCallback(ExitFrame), frame);
    Dispatcher.PushFrame(frame);
}

and thanks to the PushFrame, a second message was handled ON THE SAME THREAD as the first, hence the lock did nothing to stop it.
Once I've rearranged the code to this, the problem went away:

if (_openedViews.ContainsKey(key) == false)
{
   _openedViews.Add(key, null);
   _openedViews[key] = ServiceRegistry.GetService<IShellService>().GetView(key);
}

Good thing you found the problem. Please mark this as accepted for future readers to find quickly. — Tudor, Jun 13 '12 at 14:21

WPF MultiThreading: How can this still cause an exception? (Careful with that Dispatcher!)

4 Answers4