Search code examples
c#parallel-extensions

Weird behavior when using Parallel.Invoke and static variable


I'm trying to test the C# parallel methods and this is my test program:

class Program
{
    static int counter;
    static void Main(string[] args)
    {
        counter = 0;
        Parallel.Invoke(
            () => func(1),
            () => func(2),
            () => func(3)
            );
        Console.Read();
    }


    static void func(int num)
    {
        for (int i = 0; i < 5;i++ )
        {
            Console.WriteLine(string.Format("This is function #{0} loop. counter - {1}", num, counter));
            counter++;
        }
    }
}

What I tried to do is to have 1 static shared variable and each function instance will increase it by 1.

I expected that counter will be printed in order (1,2,3,...) But the output is surprising:

This is function #1 loop. counter - 0
This is function #1 loop. counter - 1
This is function #1 loop. counter - 2
This is function #1 loop. counter - 3
This is function #1 loop. counter - 4
This is function #3 loop. counter - 5
This is function #2 loop. counter - 1
This is function #3 loop. counter - 6
This is function #3 loop. counter - 8
This is function #3 loop. counter - 9
This is function #3 loop. counter - 10
This is function #2 loop. counter - 7
This is function #2 loop. counter - 12
This is function #2 loop. counter - 13
This is function #2 loop. counter - 14

Can anyone explain to me why this is happening?


Solution

  • The problem is that your code is not thread-safe. For example, what can happen is this:

    • function #2 gets the value of counter to use it in Console.WriteLine()
    • function #1 gets the value of counter, calls Console.WriteLine(), increments counter
    • function #1 gets the value of counter, calls Console.WriteLine(), increments counter
    • function #2 finally calls Console.WriteLine() with the old value

    Also, ++ by itself is not thread-safe, so the final value may not be 15.

    To fix both of these issues, you can use Interlocked.Increment():

    for (int i = 0; i < 5; i++)
    {
        int incrementedCounter = Interlocked.Increment(ref counter);
        Console.WriteLine("This is function #{0} loop. counter - {1}", num, incrementedCounter);
    }
    

    This way, you will get the number after increment, not before, as in your original code. Also, this code still won't print the numbers in the correct order, but you can be sure that each number will be printed exactly once.

    If you do want to have the numbers in the correct order, you will need to use lock:

    private static readonly object lockObject = new object();
    
    …
    
    for (int i = 0; i < 5; i++)
    {
        lock (lockObject)
        {
            Console.WriteLine("This is function #{0} loop. counter - {1}", num, counter);
            counter++;
        }
    }
    

    Of course, if you do this, you won't actually get any parallelism, but I assume this is not your real code.