C# TaskFactory not completing all iterations

Question

I am experiencing some oddities when using TaskFactory:

Task<int[]> parent = Task.Run(() =>
{
    int length = 100;
    var results = new int[length];
    TaskFactory tf = new TaskFactory(TaskCreationOptions.AttachedToParent,
        TaskContinuationOptions.ExecuteSynchronously);

    // Create a list of tasks that we can wait for
    List<Task> taskList = new List<Task>();
    for (int i = 0; i < length - 1; i++) // have to set -1 on length else out of bounds exception, huh?
    {
        taskList.Add(tf.StartNew(() => results[i] = i));
    }
    // Now wait for all tasks to complete
    Task.WaitAll(taskList.ToArray());

    return results;
});

parent.Wait();

var finalTask = parent.ContinueWith(
     parentTask =>
     {
        foreach (int i in parentTask.Result)
        {
            Console.WriteLine(i);
        }
    });

finalTask.Wait();

Console.ReadKey();

This gives an output similar to:

0 0 0 0 4 5 0 0 0 0 10 0 12 13 14 ... 0 99

I do not understand why not all indices are non zero.

Thanks,

Joe

Answer 1

When you capture a variable with a lambda, this variable is placed into a compiler-generated object which is shared between the inner and outer scope. When you do this:

for (int i = 0; i < length - 1; i++)
{
    taskList.Add(tf.StartNew(() => results[i] = i));
}

The variable i is shared between both your loop and all the child tasks. There is only one i and it is being modified by the loop as the tasks are running. This is a race condition and will result in seemingly random data in the array every time, depending on how the tasks are scheduled.

The simplest way to solve this is to make an immutable variable scoped to the loop body:

for (int i = 0; i < length; i++) // You can now use length instead of length - 1
{
    int innerI = i;
    taskList.Add(tf.StartNew(() => results[innerI] = innerI));
}

There is now a separate innerI variable for every task, and it is assigned the value i exactly once and will not change.

---

Imagine the old code as transformed by the compiler into

class Generated1
{
    public int i;
}
var context = new Generated1(); // Exactly one copy of i
for (context.i = 0; context.i < length - 1; context.i++)
{
    taskList.Add(tf.StartNew(() => results[context.i] = context.i));
}

While the new code as transformed by the compiler becomes

class Generated2
{
    public int innerI;
}
for (int i = 0; i < length - 1; i++)
{
    var context = new Generated2(); // New copy of innerI for every loop iteration
    context.innerI = i;
    taskList.Add(tf.StartNew(() => results[context.innerI] = context.innerI));
}

---

Regarding why you had to use length - 1: Some of your tasks didn't run until the loop was complete. At that point, i == length, so you would get an IndexOutOfRangeException when trying to use i as an index into your array. When you fix the race condition with i, this can no longer happen.