4

I want to use ThreadPool to complete long running jobs in less time. My methods does more jobs of course but I prepared a simple example for you to understand my situation. If I run this application it throws ArgumentOutOfRangeException on the commented line. Also it shows that i is equal to 10. How can it enter the for loop if it is 10?

If I don't run the application and debug this code it does not throw exception and works fine.

public void Test()
{
    List<int> list1 = new List<int>();
    List<int> list2 = new List<int>();

    for (int i = 0; i < 10; i++) list1.Add(i);
    for (int i = 0; i < 10; i++) list2.Add(i);

    int toProcess = list1.Count;

    using (ManualResetEvent resetEvent = new ManualResetEvent(false))
    {
        for (int i = 0; i < list1.Count; i++)
        {
            ThreadPool.QueueUserWorkItem(
                new WaitCallback(delegate(object state)
                {
                    // ArgumentOutOfRangeException with i=10
                    Sum(list1[i], list2[i]);

                    if (Interlocked.Decrement(ref toProcess) == 0)
                        resetEvent.Set();

                }), null);
        }

        resetEvent.WaitOne();
    }

    MessageBox.Show("Done");
}

private void Sum(int p, int p2)
{
    int sum = p + p2;
}

What is the problem here?

Orkun Bekar
  • 1,447
  • 1
  • 15
  • 36

2 Answers2

3

The problem is that i==10, but your lists have 10 items (i.e. a maximum index of 9).

This is because you have a race condition over a captured variable that is being changed before your delegate runs. Will the next iteration of the loop increment the value before the delegate runs, or will your delegate run before the loop increments the value? It's all down to the timing of that specific run.

Your instinct is that i will have a value of 0-9. However, when the loop reaches its termination, i will have a value of 10. Because the delegate captures i, the value of i may well be used after the loop has terminated.

Change your loop as follows:

for (int i = 0; i < list1.Count; i++)
{
    var idx=i;
    ThreadPool.QueueUserWorkItem(
        new WaitCallback(delegate(object state)
        {
            // ArgumentOutOfRangeException with i=10
            Sum(list1[idx], list2[idx]);

            if (Interlocked.Decrement(ref toProcess) == 0)
                resetEvent.Set();

        }), null);
}

Now your delegate is getting a "private", independent copy of i instead of referring to a single, changing value that is shared between all invocations of the delegate.

I wouldn't worry too much about the difference in behaviour between debug and non-debug modes. That's the nature of race conditions.

spender
  • 117,338
  • 33
  • 229
  • 351
2

What is the problem here?

Closure. You're capturing the i variable which isn't doing what you expect it to do.

You'll need to create a copy inside your for loop:

var currentIndex = i:
Sum(list1[currentIndex], list2[currentIndex]);
Community
  • 1
  • 1
Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321