Cannot corrupt data with concurrent thread access

Question

I wrote this experiment to demonstrate to someone that accessing shared data conccurently with multiple threads was a big no-no. To my surprise, regardless of how many threads I created, I was not able to create a concurrency issue and the value always resulted in a balanced value of 0. I know that the increment operator is not thread-safe which is why there are methods like Interlocked.Increment() and Interlocked.Decrement() (also noted here Is the ++ operator thread safe?).

If the increment/decrement operator is not thread safe, then why does the below code execute without any issues and results to the expected value?

The below snippet creates 2,000 threads. 1,000 constantly incrementing and 1,000 constantly decrementing to insure that the data is being accessed by multiple threads at the same time. What makes it worse is that in a normal program you would not have nearly as many threads. Yet despite the exaggerated numbers in an effort to create a concurrency issue the value always results in being a balanced value of 0.

static void Main(string[] args)
    {
        Random random = new Random();
        int value = 0;

        for (int x=0; x<1000; x++)
        {
            Thread incThread = new Thread(() =>
            {
                for (int y=0; y<100; y++)
                {
                    Console.WriteLine("Incrementing");
                    value++;
                }
            });

            Thread decThread = new Thread(() =>
            {
                for (int z=0; z<100; z++)
                {
                    Console.WriteLine("Decrementing");
                    value--;
                }
            });

            incThread.Start();
            decThread.Start();
        }

        Thread.Sleep(TimeSpan.FromSeconds(15));
        Console.WriteLine(value);
        Console.ReadLine();
    }

I'm hoping someone can provide me with an explanation so that I know that all my effort into writing thread-safe software is not in vain, or perhaps this experiment is flawed in some way. I have also tried with all threads incrementing and using the ++i instead of i++. The value always results in the expected value.

Answer 1

You'll usually only see issues if you have two threads which are incrementing and decrementing at very close times. (There are also memory model issues, but they're separate.) That means you want them spending most of the time incrementing and decrementing, in order to give you the best chance of the operations colliding.

Currently, your threads will be spending the vast majority of the time sleeping or writing to the console. That's massively reducing the chances of collision.

Additionally, I'd note that absence of evidence is not evidence of absence - concurrency issues can indeed be hard to provoke, particularly if you happen to be running on a CPU with a strong memory model and internally-atomic increment/decrement instructions that the JIT can use. It could be that you'll never provoke the problem on your particular machine - but that the same program could fail on another machine.

Answer 2

In addition to Jon Skeets answer:

A simple test that at least on my litte Dual Core shows the problem easily:

Sub Main()

    Dim i As Long = 1
    Dim j As Long = 1

    Dim f = Sub()
                While Interlocked.Read(j) < 10 * 1000 * 1000
                    i += 1
                    Interlocked.Increment(j)
                End While
            End Sub

    Dim l As New List(Of Task)
    For n = 1 To 4
        l.Add(Task.Run(f))
    Next

    Task.WaitAll(l.ToArray)
    Console.WriteLine("i={0}   j={1}", i, j)
    Console.ReadLine()


End Sub

i and j should both have the same final value. But they dont have!

EDIT

And in case you think, that C# is more clever than VB:

    static void Main(string[] args)
    {
        long i = 1;
        long j = 1;

        Task[] t = new Task[4];

        for (int k = 0; k < 4; k++)
        {
            t[k] = Task.Run(() => {
                            while (Interlocked.Read(ref j) < (long)(10*1000*1000))
                            {
                                i++;
                                Interlocked.Increment(ref j);
                            }});
        }

        Task.WaitAll(t);
        Console.WriteLine("i = {0}   j = {1}", i, j);
        Console.ReadLine();

    }

it isnt ;)

The result: i is around 15% (percent!) lower than j. ON my machine. Having an eight thread machine, probabyl might even make the result more imminent, because the error is more likely to happen if several tasks run truly parallel and are not just pre-empted.

The above code is flawed of course :(
IF a task is preempted, just AFTER i++, all other tasks continue to increment i and j, so i is expected to differ from j, even if "++" would be atomic. There a simple solution though:

    static void Main(string[] args)
    {
        long i = 0;
        int runs = 10*1000*1000;

        Task[] t = new Task[Environment.ProcessorCount];

        Stopwatch stp = Stopwatch.StartNew();

        for (int k = 0; k < t.Length; k++)
        {
            t[k] = Task.Run(() =>
            {
                for (int j = 0; j < runs; j++ )
                {
                    i++;
                }
            });
        }

        Task.WaitAll(t);

        stp.Stop();

        Console.WriteLine("i = {0}   should be = {1}  ms={2}", i, runs * t.Length, stp.ElapsedMilliseconds);
        Console.ReadLine();

    }

Now a task could be pre-empted somewhere in the loop statements. But that wouldn't effect i. So the only way to see an effect on i would be, if a task is preempted when it just at the i++ statement. And thats what was to be shown: It CAN happen and it's more likely to happen when you have fewer but longer running tasks.
If you write Interlocked.Increment(ref i); instead of i++ the code runs much longer (because of the locking), but i is exactly what it should be!

Answer 3

IMO these loops are too short. I bet that by the time the second thread starts the first thread has already finished executing its loop and exited. Try to drastically increase the number of iterations that each thread executes. At this point you could even spawn just two threads (remove the outer loop) and it should be enough to see wrong values.

For example, with the following code I'm getting totally wrong results on my system:

static void Main(string[] args)
{
    Random random = new Random();
    int value = 0;

    Thread incThread = new Thread(() =>
            {
                for (int y = 0; y < 2000000; y++)
                {
                    value++;
                }
            });

    Thread decThread = new Thread(() =>
            {
                for (int z = 0; z < 2000000; z++)
                {
                    value--;
                }
            });

    incThread.Start();
    decThread.Start();

    incThread.Join();
    decThread.Join();
    Console.WriteLine(value);
}