Cross thread InvalidOperationException, InvokeRequired == false when accessed by 2 threads simultanously

Question

A control is accessed by two worker threads, the 2nd before the first has completed it's work on the control. The second thread (9) gets InvokeRequired == false, and the first thread (17) then gets the exception when calling .Refresh on a child-control.

Is this expected behavior? What exactly causes a thread to see a control's InvokeRequired as true/false?
And finally, what would be a good solution.. Put a lock on all invoke statements, and make them call a separate method instead (to avoid deadlock obviously) ?

private void OnHistoryUpdate(object sender)
{
    Console.WriteLine("<< Invoke? " + this.InvokeRequired   + " " + Thread.CurrentThread.ManagedThreadId );

    if (this.InvokeRequired)
        this.Invoke(new Action<object>(OnHistoryUpdate), sender);  

    LoadTimeSeries(this.Interval);
    Console.WriteLine(">> Invoke? " + this.InvokeRequired   + " " + Thread.CurrentThread.ManagedThreadId);

}

output:

<< Invoke? True Thread: 17
<< Invoke? False Thread:  9
>> Invoke? False Thread:  9

"A control is accessed by two worker threads" well that's 2 problems just there. Worker threads should *never* access controls, even to read from them. — Marc Gravell, Feb 21 '13 at 05:08
@MarcGravell, really? I receive fast streaming socket data which requires updates of a number of controls. What alternative way to update are you suggesting? Or maybe it's wrong to call them worker threads, idk. They are created from socket messages. — bretddog, Feb 21 '13 at 07:01
_Worker threads_ is right, even if your code doesn't create them explicitly; sounds like a socket message is received on a non-UI thread. Then code handling a socket message executes on a non-UI thread and must marshal to the UI thread to access the UI controls. The `InvokeRequired` technique is commonly used but worker thread and UI thread must share code; it's hard to maintain [Separation of concerns](http://en.wikipedia.org/wiki/Separation_of_concerns). Better to supply the UI with socket data, say via an event, letting it update itself. Then worker is not responsible for UI and vice versa. — groverboy, Feb 23 '13 at 01:03
See [Understanding SynchronizationContext (Part I)](http://www.codeproject.com/Articles/31971/Understanding-SynchronizationContext-Part-I) for a nice alternative to the `InvokeRequired` technique. .NET has new stuff like `async`/`await` and TPL that may be better still. — groverboy, Feb 23 '13 at 01:03
@groverboy, thanks! but I don't really see what are the potential problems. All handlers in my controls that subscribe to events initiated by worker threads use InvokeRequired/BeginInvoke. If you could elaborate a bit on the pitfalls would help me understand. My program has a lot of bussiness logic to execute before any UI update occurs, it's not a chat program. I sense Marc's statement "..should never access controls" is wrong. Unless it can be better explained? — bretddog, Feb 23 '13 at 22:09
Honestly I don't really see what you are suggesting. My worker threads DO update the UI with data via events, just not direct socket data. The UI can not interpret that data, that's one of the responsibilities of the business layer and domain model.. Are there some very broad assumptions on the (lack of) complexity of my program going on here? It would be good if you could explain when your ideas are applicable and when not.. — bretddog, Feb 23 '13 at 22:28
While I can't speak for @MarcGravell, "A control is accessed by two worker threads" can't work because this will cause a cross-thread exception. I think it's more accurate to describe your situation as: two worker threads sync to the UI thread in order to update a control. — groverboy, Feb 24 '13 at 23:39
If your UI and socket handler are relatively simple (and unlikely to become complex) then there's probably no issue with _separation of concerns_. But if either one involves complex code then that complexity can be reduced by hiding knowledge. Or if you want to encapsulate the socket handler in its own assembly, not tied to a control. Socket handler's worker threads could be hidden from its consumer (the UI) by raising custom events _on the consumer thread_. The UI's need for `InvokeRequired`, etc. (even the control) could be hidden from socket handler code. See `SynchronizationContext`. — groverboy, Feb 25 '13 at 00:07
So I read more on the `SynchronizationContext`. Still I fail to see how this would make more wonders than avoiding that both worker threads and the UI thread read the same top 3 lines of code in an event handler like in my post. Maybe it's my lack of imagination, but I can't really see how not doing this would prevent any code from being encapsulated in other assemblies..? So I assume instead of `if(InvokeRequired) ... LoadTimeSeries(..)` I would write something like `uiContext.Post(LoadTimeSeries, ..)` ? What else than this slight verbosity change would I benefit? — bretddog, Feb 25 '13 at 03:58
You don't need `SynchronizationContext` to encapsulate worker threads in another assembly, but it helps to create a more flexible design. Say your assembly accepts a control reference so it can check `InvokeRequired` etc. And say your assembly also needs to support a WPF UI. Trouble is, WPF controls don't have this property (you need to call `Dispatcher.CheckAccess`) so your assembly needs to keep track of the control type. Much nicer to keep it abstract: `SynchronizationContext` is an abstract class implemented by both WinForms and WPF (and others). — groverboy, Feb 25 '13 at 04:22
From [Understanding SynchronizationContext (Part I)](http://www.codeproject.com/Articles/31971/Understanding-SynchronizationContext-Part-I): "I will say that you don't have to use this class to sync into the UI thread. You can use the `InvokeRequired` property ... From a design prospective, you never want to have a UI reference within your BI layer ... It would be nice for the BI to have the ability to marshal code to the UI thread without having a reference to a control or a form." — groverboy, Feb 25 '13 at 04:27

groverboy · Accepted Answer · 2013-02-21T04:32:03.393

Your code updates the control twice:

Using the call to Invoke, which delegates another call to OnHistoryUpdate. This is safe because Invoke marshals execution to the UI thread.
After the call to Invoke. Invoke executes synchronously and when it returns your code will call LoadTimeSeries a second time, this time unsafely (it doesn't check InvokeRequired again).

I would change the method as below, and also consider using BeginInvoke instead so the worker thread doesn't block.

private void OnHistoryUpdate(object sender)
{
    Console.WriteLine("<< Invoke? " + this.InvokeRequired   + " " + Thread.CurrentThread.ManagedThreadId );

    if (this.InvokeRequired)
        this.Invoke(new Action<object>(OnHistoryUpdate), sender);
    else  
        LoadTimeSeries(this.Interval);

    Console.WriteLine(">> Invoke? " + this.InvokeRequired   + " " + Thread.CurrentThread.ManagedThreadId);    
}

score 0 · Answer 2 · answered Feb 21 '13 at 04:28

0

If you call Refresh on a control that requires invoke from you thread without using invoke it is expected to get an exception. Make sure that you do not do this and you'll be fine. This is all that is needed.

answered Feb 21 '13 at 04:28

Andrew Savinykh

25,351
17
103
158

Cross thread InvalidOperationException, InvokeRequired == false when accessed by 2 threads simultanously

2 Answers2