For-loop multi-threading passes upper bound

Question

We are learning multi-threadding today in class and we came across a very curious error. When doing a for loop in our new thread the upper bound of the for loop keeps getting passed. The thread is being killed but then another value will appear and end another thread.

For the purpose of debugging the error I changed the upper bound to 90 to avoid the OutOfRange Exception on the progressbar.

While outputting the counter to the progressing bar and updating the progress bar I got this in my output window.

If i commented out the updating on the progress bar (pbLoad.Value = i;) I got this in my output window

I have tried changing the loop to i<101 and also tried moving where the i++ was but it made no difference

EDIT: This is coming from the BeginInvoke. When i switched it to Invoke it worked but then I will get a deadlock when trying to use the cancel button.

Here is the code:

public partial class Form1 : Form
    {
        Thread backgroundThread;
        bool stopExecution = false;

        public Form1()
        {
            InitializeComponent();
        }

        private void btnStart_Click(object sender, EventArgs e)
        {
            stopExecution = false;
            btnStart.Enabled = false;
            backgroundThread = new Thread(DoDomethingThatTakesAWhile);
            backgroundThread.Start();
        }

        private void DoDomethingThatTakesAWhile()
        {
            for (int i = 0; i <= 100; i++)
            {
                if (!stopExecution)
                {
                    Thread.Sleep(100);

                    if (pbLoad.InvokeRequired)
                    {
                        MethodInvoker myMethod
                            = new MethodInvoker(
                                delegate
                                {
                                    if (!stopExecution)
                                    {
                                        pbLoad.Value = i;
                                        Debug.WriteLine(i); //i to output window                                        
                                    }
                                });
                        pbLoad.BeginInvoke(myMethod);
                    }
                    else
                    {
                        pbLoad.Value = i;
                    }
                }
                else
                {
                    break;
                }
            }
        }

        private void btnCancel_Click(object sender, EventArgs e)
        {
            //backgroundThread.Abort();
            stopExecution = true;
            backgroundThread.Join();

            pbLoad.Value = 0;
            btnStart.Enabled = true;
        }
    }

Answer 1

When you call MethodInvoke it will not occurs at that moment, but some time later.

In your scenario you have a chance of following to occurs:

• invoked code is finally executed;
• the loop is already finished (and i become 101)
• you are accessing i directly and you read 101.

And to fix it you can make a copy of i (by passing it as a parameter to invoked method):

pbLoad.BeginInvoke(new Action<int>(a =>
{
    if (!stopExecution)
    {
        pbLoad.Value = a;
        Debug.WriteLine(a); //a to output window                                        
    }
}), new object[] { i });

P.S: you don't need to check for InvokeRequired, unless you plan to call DoDomethingThatTakesAWhile method directly, which I assume is not the case.

Answer 2

You're using BeginInvoke which explicitly opens the possibility for races. I recommend synchronous invoking.

Furthermore, you are capturing i, not its value. This is racy and only works by accident because you're sleeping.

Either of the changes will fix the problem. Do both of them.

If you can, abolish this low-level use of synchronization and use async/await.