Background Worker: Make sure that ProgressChanged method has finished before executing RunWorkerCompleted

Question

Let's assume I'm using a Background Worker and I've the following methods:

private void bw_DoWork(object sender, DoWorkEventArgs e)
{
    finalData = MyWork(sender as BackgroundWorker, e);
}

private void bw_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    int i = e.ProgressPercentage; // Missused for i
    Debug.Print("BW Progress Changed Begin, i: " + i + ", ThreadId: " + Thread.CurrentThread.ManagedThreadId);
    // I use this to update a table and an XY-Plot, so that the user can see the progess.
    UpdateGUI(e.UserState as MyData);
    Debug.Print("BW Progress Changed End,   i: " + i + ", ThreadId: " + Thread.CurrentThread.ManagedThreadId);
}

private void bw_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    if ((e.Cancelled == true))
    {
        // Cancelled
    }
    else if (!(e.Error == null))
    {
        MessageBox.Show(e.Error.Message);
    }
    else
    {        
        Debug.Print("BW Run Worker Completed Begin, ThreadId: " + Thread.CurrentThread.ManagedThreadId);
        // I use this to update a table and an XY-Plot, 
        // so that the user can see the final data.
        UpdateGUI(finalData);
        Debug.Print("BW Run Worker Completed End,   ThreadId: " + Thread.CurrentThread.ManagedThreadId);
    }
}

Now I would assume that the bw_ProgressChanged method has finished before the bw_RunWorkerCompleted method is called. But that's not the case and I don't understand why?

I get the following output:

Worker, i: 0, ThreadId: 27
BW Progress Changed Begin, i: 0, ThreadId: 8
BW Progress Changed End,   i: 0, ThreadId: 8
Worker, i: 1, ThreadId: 27
BW Progress Changed Begin, i: 1, ThreadId: 8
BW Progress Changed End,   i: 1, ThreadId: 8
Worker, i: 2, ThreadId: 27
BW Progress Changed Begin, i: 2, ThreadId: 8
BW Run Worker Completed Begin, ThreadId: 8
BW Run Worker Completed End,   ThreadId: 8
A first chance exception of type 'System.InvalidOperationException' occurred in mscorlib.dll
ERROR <-- Collection was modified; enumeration operation may not execute.
ERROR <-- NationalInstruments.UI.WindowsForms.Graph.ClearData()

The MagagedID 8 is the Main Thread and 27 is a Worker Thread. I can see this in the Debug / Windows / Threads.

If I don't call UpdateGUI int the bw_ProgressChanged method then no error occurs. But then the user doesn't see any progress in the table and the XY-Plot.

EDIT

The MyWork method looks like that:

public MyData[] MyWork(BackgroundWorker worker, DoWorkEventArgs e)
{
     MyData[] d = new MyData[n];
     for (int i = 0; i < n; i++) 
         d[i] = null;
     for (int i = 0; i < n; i++)
     {
         if (worker.CancellationPending == true)
         {
             e.Cancel = true;
             break;
         }
         else
         {
             d[i] = MyCollectDataPoint(); // takes about 1 to 10 seconds
             Debug.Print("Worker, i: " + i + ", ThreadId: " + Thread.CurrentThread.ManagedThreadId)
             worker.ReportProgress(i, d);
         }
     }
     return d;
}

and the UpdateGUI method looks like that:

private void UpdateGUI(MyData d)
{
   UpdateTable(d); // updates a DataGridView
   UpdateGraph(d); // updates a ScatterGraph (NI Measurement Studio 2015)
}

If I don't call UpdateGraph method it works as aspected. So the ProgressChanged method has finished before executing RunWorkerCompleted.

So I guess the problem is the combination of the ScatterGraph from NI Measurement Studio 2015 and the BackgroundWorker. But I don't understand why?

The UpdateGraph method looks like that:

private void UpdateGraph(MyData d)
{
    plot.ClearData();
    plot.Plots.Clear(); // The error happens here (Collection was modified; enumeration operation may not execute).
    int n = MyGetNFromData(d);        
    for (int i = 0; i < n; i++)
    {
        ScatterPlot s = new ScatterPlot();
        double[] xi = MyGetXiFromData(d, i);
        double[] yi = MyGetYiFromData(d, i);
        s.XAxis = plot.XAxes[0];
        s.YAxis = plot.YAxes[0];
        s.LineWidth = 2;
        s.LineColor = Colors[i % Colors.Length];
        s.ProcessSpecialValues = true;
        s.PlotXY(xi, yi);
        plot.Plots.Add(s);
    }
}

Edit 2

If I set a breakpoint in the bw_RunWorkerCompleted method then the call stack looks like that:

bw_RunWorkerCompleted
[External Code]
UpdateGraph // Line: plot.ClearData()
UpdateGUI
bw_ProgressChanged
[External Code]
Program.Main

and the first [External Code] block:

System.dll!System.ComponentModel.BackgroundWorker.OnRunWorkerCompleted(System.ComponentModel.RunWorkerCompletedEventArgs e) Unknown
[Native to Managed Transition]  
mscorlib.dll!System.Delegate.DynamicInvokeImpl(object[] args)   Unknown
System.Windows.Forms.dll!System.Windows.Forms.Control.InvokeMarshaledCallbackDo(System.Windows.Forms.Control.ThreadMethodEntry tme) Unknown
System.Windows.Forms.dll!System.Windows.Forms.Control.InvokeMarshaledCallbackHelper(object obj) Unknown
mscorlib.dll!System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)   Unknown
mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state, bool preserveSyncCtx)   Unknown
mscorlib.dll!System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext executionContext, System.Threading.ContextCallback callback, object state) Unknown
System.Windows.Forms.dll!System.Windows.Forms.Control.InvokeMarshaledCallback(System.Windows.Forms.Control.ThreadMethodEntry tme)   Unknown
System.Windows.Forms.dll!System.Windows.Forms.Control.InvokeMarshaledCallbacks()    Unknown
System.Windows.Forms.dll!System.Windows.Forms.Control.MarshaledInvoke(System.Windows.Forms.Control caller, System.Delegate method, object[] args, bool synchronous) Unknown
System.Windows.Forms.dll!System.Windows.Forms.Control.Invoke(System.Delegate method, object[] args) Unknown
System.Windows.Forms.dll!System.Windows.Forms.WindowsFormsSynchronizationContext.Send(System.Threading.SendOrPostCallback d, object state)  Unknown
NationalInstruments.Common.dll!NationalInstruments.Restricted.CallbackManager.CallbackDispatcher.SynchronousCallbackDispatcher.InvokeWithContext(System.Delegate handler, object sender, System.EventArgs e, System.Threading.SynchronizationContext context, object state) Unknown
NationalInstruments.Common.dll!NationalInstruments.Restricted.CallbackManager.a(NationalInstruments.Restricted.CallbackManager.CallbackDispatcher A_0, object A_1, object A_2, System.EventArgs A_3)    Unknown
NationalInstruments.Common.dll!NationalInstruments.Restricted.CallbackManager.RaiseEvent(object eventKey, object sender, System.EventArgs e)    Unknown
NationalInstruments.Common.dll!NationalInstruments.ComponentBase.RaiseEvent(object eventKey, System.EventArgs e)    Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.XYCursor.OnAfterMove(NationalInstruments.UI.AfterMoveXYCursorEventArgs e) Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.XYCursor.a(object A_0, NationalInstruments.Restricted.ControlElementCursorMoveEventArgs A_1)  Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.XYCursorElement.OnAfterMove(NationalInstruments.Restricted.ControlElementCursorMoveEventArgs e)  Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.XYCursorElement.a(NationalInstruments.UI.Internal.CartesianPlotElement A_0, double A_1, double A_2, int A_3, bool A_4)   Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.XYCursorElement.MoveCursorFreely(double xValue, double yValue, bool isInteractive, NationalInstruments.UI.Internal.XYCursorElement.Movement movement)    Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.XYCursorElement.MoveCursorXY(double xValue, double yValue, bool isInteractive)   Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.XYCursorElement.ResetCursor()    Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.XYCursorElement.a(object A_0, NationalInstruments.Restricted.ControlElementEventArgs A_1)    Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.PlotElement.OnDataChanged(NationalInstruments.Restricted.ControlElementEventArgs e)  Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.PlotElement.OnDataChanged()  Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.CartesianPlotElement.a(object A_0, NationalInstruments.UI.Internal.PlotDataChangedEventArgs A_1) Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.XYDataManager.a(NationalInstruments.UI.Internal.PlotDataChangedEventArgs A_0)    Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.XYDataManager.a(NationalInstruments.UI.Internal.PlotDataChangeCause A_0, int A_1)    Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.XYDataManager.ClearData(bool raiseDataChanged)   Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.CartesianPlotElement.ClearData(bool raiseDataChanged)    Unknown
NationalInstruments.UI.dll!NationalInstruments.UI.Internal.PlotElement.ClearData()  Unknown
NationalInstruments.UI.dll!NationalInstruments.Restricted.XYGraphManager.ClearData()    Unknown
NationalInstruments.UI.WindowsForms.dll!NationalInstruments.UI.WindowsForms.Graph.ClearData()   Unknown

I guess we need at least the `MyWork` code to see what you do there. And probably more details about `UpdateGUI()`, too. Normally the events of backgroundworkers are invoked on the ui thread. So if you call `ReportProgress()` in the intended way, this should not happen. — René Vogt, Sep 21 '17 at 09:22
Does `UpdateGUI` call any asynchronous methods where you don't wait for the tasks to complete? — René Vogt, Sep 21 '17 at 09:23
This is really strange... In the last line of your output you added "not executed"..just to clarify: does this end message from progresschanged occur in the output or is that line completely missing? The later would mean progress changed has somehow returned before the output, though I don't know how that would happen...an exception should have crashed the app as long as you don't catch and hide it somewhere else. Did you try to debug (first determine at which value of `i` is the last progress changed event)? — René Vogt, Sep 21 '17 at 12:38
@RenéVogt: The last line is missing. For the value of `i` I will edit my question. — Wollmich, Sep 21 '17 at 12:41
The `i` was not for me :) but for you to know when/what to debug. — René Vogt, Sep 21 '17 at 12:51
But [Rick](https://stackoverflow.com/a/46344333/5528593) has a point: you didn't specify if this is windows forms or wpf. I'm not quite sure how different the behaviour of the message pump in a wpf ui thread is from a winforms message pump. My assumptions are totally based on winforms. — René Vogt, Sep 21 '17 at 12:54
@RenéVogt with `if (i < n - 1) worker.ReportProgress(i, d);` it would work without the error. But that's a very stupid workaround. I don't like that. I would like to understand it. — Wollmich, Sep 21 '17 at 13:02
I'm running out of ideas. Can you add a try/catch block to your progresschanged handler and check if there is any exception raised? That's really my last idea, although this would have crashed your app (as long as you don't catch exceptions globally e.g by `Application.ThreadException` handler) and it would not really explain the later enumeration exception. — René Vogt, Sep 21 '17 at 13:08
So you're really doing something magic: It seems that your completed event is indeed executed _parallel_ to the progress changed event. That's something I can by no means reproduce here. Do you somehow create two different ui threads or create the form on a different thread than the ui thread? — René Vogt, Sep 21 '17 at 13:26
@RenéVogt No there is just one UI thread. I tried as well `Control.Invoke` didn't help. I think this parallel stuff is someware in the NI `ScatterGraph`. — Wollmich, Sep 21 '17 at 13:28

Hans Passant · Accepted Answer · 2017-09-21T14:58:18.550

6

Well, you have hard evidence that the RunWorkerCompleted event runs while the ProgressChanged event runs. That is not normally possible of course, they are supposed to run on the same thread.

There are two possible ways that this can happen anyway. The more obvious one is that the event handlers don't actually run on the UI thread. Which is fairly common mishap, although you tend to notice from the InvalidOperationException that causes. That exception is however not always reliably raised, it uses a heuristic. Beware that your UpdateGraph() method is not so likely to trip it since it doesn't appear to use a standard .NET control.

Diagnosing this mishap is otherwise easy, just set a breakpoint on the event handler and use the Debug > Windows > Threads debugging window to verify it runs on the main thread. Using Debug.Print to display the value of Thread.CurrentThread.ManagedId can help ensure that all invocations run on the UI thread. You fix it by ensuring that the RunWorkerAsync() call is executed on the main thread.

And then there is the rat trap of a re-entrancy bug, it occurs when ProgressChanged does something that gets the UI dispatcher running again. Tends to be about as hard to debug as a threading race. Three basic ways that can happen:

using the infamous Application.DoEvents()
its evil step-sister, ShowDialog(). ShowDialog is DoEvents in disguise, it pretends to be less lethal by disabling the windows of the UI. Which tends to work okay, except when you run code that isn't activated by the UI. Like this code. Beware that you do appear to use MesssageBox.Show() for debugging, never a good idea. Always favor breakpoints and Debug.Print() to avoid this trap.
doing something that blocks the UI thread, like lock, Thread.Join(), WaitOne(). Blocking an STA thread is formally illegal, high odds for deadlock, so the CLR does something about it. It pumps its own message loop to ensure deadlock is avoided. Yes, like DoEvents does, it does some filtering to avoid the nasty cases. But not otherwise enough for this code. Beware that this might be done by code you did not write, like that Graph control.

Diagnose a re-entrancy bug by setting a breakpoint on the RunWorkerCompleted event. You should see the ProgressChanged event handler back, buried deep in the call stack. And the statement that causes the re-entrancy. If the trace doesn't help you figure it out then post it in your question.

edited Sep 21 '17 at 14:58

answered Sep 21 '17 at 13:31

Hans Passant

922,412
146
1,693
2,536

Thx, my brain started to hurt :) With an `Application.DoEvents()` in a progress changed handler I can at least reproduce a "parallel" execution of that handler...The messagebox in the completed event should have no effect, but there surely must be something in the methods called by OP's progress changed handler that triggers that re-entrancy bug. – René Vogt Sep 21 '17 at 13:41
Another way that the BGW could not properly dispatch to the UI thread is if it's called in the main thread, but it's started at a time where the current synchronization context isn't the winforms sync context (which would be even less likely, but possible). This could happen either because someone (or some library code you were calling out to) was messing with the sync context, or (very slightly more likely) the worker was started before the application loop was started and had an opportunity to set the sync context, for example in the main form's constructor (rather than the load event). – Servy Sep 21 '17 at 13:42
@RenéVogt If it's option 1, yes, the problems is in that handler somewhere, but it could be further down the call stack or somewhere in the code of any of the controls it's manipulating. If it's option 2, then the problem isn't with the progress changed handler. – Servy Sep 21 '17 at 13:42
Thanks for your answer. If I set breakpoint at `plot.Plots.Clear()` I can see that this is in all cases in the `Main Thread`. Removing the `MessageBox.Show("Done!")` doesn't help either. So I guess it's the `re-entrancy bug`. But I still don't understand it and I'm looking for a workaround. – Wollmich Sep 21 '17 at 14:28
1

Diagnose a re-entrancy bug by setting a breakpoint on your RWC event handler. You should see your ProgressChanged event handler deep down in the call stack. If that's the case and you still can't make sense of it then post the stack trace in your question. Also Debug.Print the Thread.CurrentThread.ManagedId so you can be sure that *every* invocation occurs on the correct thread. – Hans Passant Sep 21 '17 at 14:36
@HansPassant Thanks a lot for the very helpful hints. I've edited my question and posted the call stack. – Wollmich Sep 22 '17 at 06:44
3

It is indeed a re-entrancy bug, caused by the National Instruments control. It is doing the equivalent of Control.Invoke() to raise an event. It tries too hard to ensure the event is raised on the UI thread. Unnecessarily so, and makes little sense for a UI control, the code already runs on the UI thread. You need to file a bug with the company, but it is fairly unlikely that they'll fix it. This is a workaround for customers that did this wrong in the past, updating the control from a worker thread. You have no workaround other than avoid updating the graph in ProgressChanged. – Hans Passant Sep 22 '17 at 08:21
2

Perhaps you need to do this just as wrong as prior customers, updating the graph inside DoWork instead of ProgressChanged. I have no insight in how many race bugs that could trigger. They are a lot less frequent and a lot harder to diagnose. Talking to a NI programmer is best. – Hans Passant Sep 22 '17 at 08:25
If I'm not using a `NationalInstruments.UI.XYCursor` in the `NationalInstruments.UI.WindowsFormsScatterGraph` then the problem is solved. With this is a workaround I could live. Thanks again very much for your helpful hints. – Wollmich Sep 22 '17 at 09:23

score -1 · Answer 2 · answered Sep 21 '17 at 12:46

-1

The biggest flaw is your assumption below is wrong.

Now I would assume that the bw_ProgressChanged method has finished before the bw_RunWorkerCompleted method is called. But that's not the case and I don't understand why?

Do not get caught up into mentally serializing the flow of logic. With WinForms/WPF, you have two completely independent and asynchronous events occurring. You have the BGW sending a request (via worker.ReportProgress) to the UI to perform a progress update. The UI thread must receive that request and schedule when the bw_ProgressChanged event runs.

Independent of that the BGW (via myWork) decides to terminate, perhaps by fully completing the job, or because an untrapped exception was thrown, or perhaps the end-user desired to cancel the work at a given instance. This then sends a request to the UI thread to run the bw_RunWorkerCompleted method. Once again the UI must schedule it on its many list of things to do.

answered Sep 21 '17 at 12:46

Rick Davin

1,010
8
10

1

But these two events occure one after another in the worker thread. I would really be very very suprised if OP gets his UI thread to reproducably schedule the later event before the earlier one. Of course, `myWork` does not wait until progresschanged is finished, but progresschanged is definitly scheduled _before_ workercompleted. You can even see this in the output (progresschanged **began** before completed). And the two events are _executed_ on the same ui thread, not parallel. – René Vogt Sep 21 '17 at 12:50
@RenéVogt Yes, progresschanged is scheduled before workercompleted, but this does not mean progresschanged finishes before workercompleted gets called, which is what OP asked. – Rick Davin Sep 21 '17 at 13:49
1

Yes it does (in _normal_ circumstances, not in those described by [Hans](https://stackoverflow.com/a/46345352/5528593)). The handlers are (normally) all executed on the same ui thread, and therefor "completed" can only run when "progresschanged" has finished. `BackgroundWorker` takes care of _invoking_ the handlers on the ui thread. – René Vogt Sep 21 '17 at 13:51

Background Worker: Make sure that ProgressChanged method has finished before executing RunWorkerCompleted

2 Answers2

Linked