Task Parallel Library and Loops

Question

I have come across a situation where Tasks I am creating only seem to work when I am debugging the code.

As you will see below I keep getting a index out of range exception which should not be possible as the loop should never get to 5.

If I add a break point to the loop and then keep pressing F10 through to the end of the program all works as expected.

Any ideas what I am doing wrong?

using System;
using System.Collections.Generic;
using System.Threading.Tasks;

namespace ConsoleApplication1
{
class Program
{        
    static void Main(string[] args)
    {
        int[] numbers = new int[5] { 1, 2, 3, 4, 5 };
        List<int> nums = new List<int>();

        var tasks = new Task[5];

        for (int i = 0; i < numbers.Length; i++)
        {
            tasks[i] = Task.Factory.StartNew(() =>
                {
                    nums.Add(numbers[i]);
                }, 
                TaskCreationOptions.None);
        }
        Task.WaitAll(tasks);

        for (int i = 0; i < nums.Count; i++)
        {
            Console.WriteLine(nums[i]);
        }
        Console.ReadLine();
    }
}

}

I would expect to see 1, 2, 3, 4, 5 but not in any particular order as running async

Strangely this does work but I don't see the difference other than the extra typing.

using System;
using System.Collections.Generic;
using System.Threading.Tasks;

namespace ConsoleApplication1
{
class Program
{        
    static void Main(string[] args)
    {
        int[] numbers = new int[5] { 1, 2, 3, 4, 5 };
        List<int> nums = new List<int>();

        var tasks = new Task[5]
        {
            Task.Factory.StartNew(() => {nums.Add(numbers[0]);}, TaskCreationOptions.None),
            Task.Factory.StartNew(() => {nums.Add(numbers[1]);}, TaskCreationOptions.None),
            Task.Factory.StartNew(() => {nums.Add(numbers[2]);}, TaskCreationOptions.None),
            Task.Factory.StartNew(() => {nums.Add(numbers[3]);}, TaskCreationOptions.None),
            Task.Factory.StartNew(() => {nums.Add(numbers[4]);}, TaskCreationOptions.None)
        };

        Task.WaitAll(tasks);

        for (int i = 0; i < nums.Count; i++)
        {
            Console.WriteLine(nums[i]);
        }
        Console.ReadLine();
    }
}

}

Thanks

Mike

just to be clear, you're saying that the first piece of code does **not** work, but the second **does**, correct? — vlad, Aug 12 '11 at 12:33

Tom · Accepted Answer · 2011-08-12T12:56:14.590

3

Since the task happens outside the loop, which has finished by the time the task executes, i at this time is 5 (loop exit condition).

In your second example, you dont use i, but instead hard code the values so the problem disappears.

In the debugger, the timing is different and the tasks can execute before the loop completes: again, the problem disappears.

I think you could fix it like this:

var ii=i;
tasks[i] = Task.Factory.StartNew(() =>
                {
                    nums.Add(numbers[ii]);
                }, 
                TaskCreationOptions.None);

edited Aug 12 '11 at 12:56

answered Aug 12 '11 at 12:49

Tom

7,994
8
45
62

I think this is what Resharper is talking about when it whines at you for "Access to a modified closure": http://stackoverflow.com/questions/304258/access-to-modified-closure-2 – Rebecca Jul 09 '14 at 12:25
As a further point, I find it easier to work with a tasks = new List() and tasks.Add(mytask); then do Task.WaitAll(tasks.toArray()); – Rebecca Jul 09 '14 at 12:26

Ronald Wildenberg · Answer 2 · 2012-09-20T10:09:13.933

UPDATE: In C#5 this is no longer a problem. A breaking change was made so that the loop variable of a foreach is logically inside the loop.

This is yet another example of one of the most common mistakes you can make in C# code: capturing the loop variable in an anonymous delegate. Eric Lippert has a good explanation of exactly what goes wrong.

Actually, your situation is even worse because in theory it could go right or wrong randomly. Suppose you made the following change:

for (int i = 0; i < numbers.Length; i++)
{
    tasks[i] = ...;
    tasks[i].Wait();
}

then your program suddenly works as expected. The reason is, that your task takes the value of i (the loop variable) when the task executes, not the value of i when that task was created. So the following sequence is possible in your original program:

i = 0
Create task 0
i = 1
Run task 0: This task sees i = 1
Create task 1
i = 2
Create task 2
i = 3
Create task 3
i = 4
Create task 4
i = 5
Run task 1: This task sees i = 5 and throws an exception

Tom's answer fixes this problem by introducing a new variable ii inside the loop. When each task is created, it captures this variable, which has a fixed value for each iteration.

score 2 · Answer 3 · answered Aug 12 '11 at 16:32

The problem here is that the Task access the i variable, and by the time the task is running, this variable will be changed, and the reason you are getting the exception in the last iteration the variable i will be 5, and although the loop will stop after this increment, but the task body still reference it, so this line will throw

nums.Add(numbers[i]);

because obviously 5 is out of range value.

To fix this you need to pass i to the StartNew method as a state parameter

 tasks[i] = Task.Factory.StartNew((obj) =>
            {
                int index = (int) obj;
                nums.Add(numbers[index]);
            }, 
            i);

Task Parallel Library and Loops

3 Answers3