Task.WaitAll seems bugged

Question

I'm developing a simulation tool that performs various mathematical operations. So far I have not had the need to do parallel operations, but now I really need them. Among the various methods of parallelism I have read that best performance should be achieved using the Tasks.

I wrote this simple program, but I realized that something is wrong.

private static int taskCounter { get; set; }
private static void SimpleTest() { taskCounter--; }

static void Main(string[] args)
{
    for (int N = 0; N < 100; N++)
    {
        taskCounter = 0;
        List<Task> taskList = new List<Task>();
        for (int i = 0; i < 100; i++)
        {
            taskCounter++;
            Task task = Task.Factory.StartNew(() => SimpleTest());
            taskList.Add(task);
        }
        Task.WaitAll(taskList.ToArray());
        Console.WriteLine("Unsolved: {0}", taskCounter);
    }
}

Expected: Unsolved: 0 for all iterations. Result:

[...]
Unsolved: 0
Unsolved: 0
Unsolved: 2
Unsolved: -4
Unsolved: -1
Unsolved: -1
Unsolved: 0
Unsolved: 0
Unsolved: 2
Unsolved: -1
[...]

`taskCounter++` and `taskCounter--` are not atomic operations. Running those operations in a concurrent manner is not safe and will yield unexpected, arbitrary results due to race conditions... Use `Interlocked.Increment` / `Interlocked.Decrement` methods for atomic increment and decrement operations (the Interlocked method would require `taskCounter` to be a field; or keep `taskCounter` a property but explicitly implement a backing field which then would be modified by the Interlocked methods...) — , May 31 '19 at 16:15
To give a more polite answer, you probably need to do a little more research on currency to really understand what's going on. Using multiple threads that access similar data can get very complicated. However, as elgonzo said, if you let each thread in isolation, store the results to something like a concurrent bag (which is designed to be thread safe) and then combine the results after all tasks are complete, you can avoid a lot of the complexity. — Slothario, May 31 '19 at 16:16
Another possible duplicate: [Are incrementers / decrementers (var++, var--) etc thread safe?](https://stackoverflow.com/questions/443500/are-incrementers-decrementers-var-var-etc-thread-safe) — , May 31 '19 at 16:26
This behavior is neither a bug nor unique to `Task`s or `WaitAll()`. The same thing would happen with `Thread`s. — Lance U. Matthews, May 31 '19 at 16:28
If the thing about race conditions is still a mystery to you, Tom Scott made a video a while ago where he (among other things) gave an example of possible race conditions when incrementing numbers concurrently that's simple to understand: https://www.youtube.com/watch?v=RY_2gElt3SA — , May 31 '19 at 16:30

Samuel Vidal · Accepted Answer · 2019-05-31T17:30:58.490

As noted by elgonzo in his comment. The two operations taskCounter++ and taskCounter-- are not atomic, meaning they actually consist of several steps (read the variable from memory, add or substract one to it, then write the result back to memory.

With several threads performing those sequences simultaneously the final result can become incorrect. Say thread 1 and thread 2 read the variable from memory at the same time increment their respective copies independently then try to write the result back to memory. In that case, the final value that was written to memory is not the same as if those two sequences were performed one after the other.

The cache memory of modern CPU makes it very likely that those operations performed in parallel produce an incorrect result. If as suggested you use Interlocked.Increment / Interlocked.Decrement you will get a consistent result. Those two operations use hardware support to guarantee atomicity.

Task.WaitAll seems bugged

1 Answers1