How can I keep track of how many async tasks have completed in a loop?

Question

I have some code that loops through a list of records, starts an export task for each one, and increases a progress counter by 1 each time a task finishes so the user knows how far along the process is.

But depending on the timing of my loops, I often see the output showing a higher number before a lower number.

For example, I would expect to see output like this:

Exporting A
Exporting B
Exporting C
Exporting D
Exporting E
Finished 1 / 5
Finished 2 / 5
Finished 3 / 5
Finished 4 / 5
Finished 5 / 5

But instead I get output like this

Exporting A
Exporting B
Exporting C
Exporting D
Exporting E
Finished 1 / 5
Finished 2 / 5
Finished 5 / 5
Finished 4 / 5
Finished 3 / 5

I don't expect the output to be exact since I'm not locking the value when I update/use it (sometimes it outputs the same number twice, or skips a number), however I wouldn't expect it to go backwards.

My test data set is 72 values, and the relevant code looks like this:

var tasks = new List<Task>();
int counter = 0;

StatusMessage = string.Format("Exporting 0 / {0}", count);

foreach (var value in myValues)
{
    var valueParam = value;

    // Create async task, start it, and store the task in a list
    // so we can wait for all tasks to finish at the end
    tasks.Add(
        Task.Factory.StartNew(() =>
        {
            Debug.WriteLine("Exporting " + valueParam );

            System.Threading.Thread.Sleep(500);
            counter++;
            StatusMessage = string.Format("Exporting {0} / {1}", counter, count);

            Debug.WriteLine("Finished " + counter.ToString());
        })
    );
}

// Begin async task to wait for all tasks to finish and update output
Task.Factory.StartNew(() =>
{
    Task.WaitAll(tasks.ToArray());
    StatusMessage = "Finished";
});

The output can appear backwards in both the debug statements and the StatusMessage output.

What's the correct way to keep count of how many async tasks in a loop are completed so that this problem doesn't occur?

Answer 1

You get mixed output, because counter is not incremented in the same order as Debug.WriteLine(...) method is executed.

To get a consistent progress report, you can introduce a reporting lock into the task

tasks.Add(
    Task.Factory.StartNew(() =>
    {
        Debug.WriteLine("Exporting " + valueParam );

        System.Threading.Thread.Sleep(500);
        lock(progressReportLock)
        {
           counter++;
           StatusMessage = string.Format("Exporting {0} / {1}", counter, count);
           Debug.WriteLine("Finished " + counter.ToString());
        }
    })
);

Answer 2

In this sample the counter variable represents shared state among several threads. Using the ++ operator on shared state is simply unsafe and will give you incorrect results. It essentially boils down to the following instructions

1. push counter to stack
2. push 1 to stack
3. add values on the stack
4. store into counter

Because multiple threads are executing this statement it's possible for one to interrupt the other partway through completing the above sequence. This would cause the incorrect value to end up in counter.

Instead of ++ use the following statement

Interlocked.Increment(ref counter);

This operation is specifically designed to update state which may be shared among several threads. The interlocked will happen atomically and won't suffer from the race conditions I outlined

The actual out of order display of values suffers from a similar problem even after my suggested fix. The increment and display operation aren't atomic and hence one thread can interrupt the other in between the increment and display. If you want the operations to be un-interruptable by other threads then you will need to use a lock.

object lockTarget = new object();
int counter = 0; 

...

lock (lockTarget) {
  counter++;
  StatusMessage = string.Format("Exporting {0} / {1}", counter, count);
  Debug.WriteLine("Finished " + counter.ToString());
}

Note that because the increment of counter now occurs inside the lock there is no longer a need to use Interlocked.Increment