Thread synchronization in the Task Parallel Library (TPL)

Question

I am learning TPL and stuck with a doubt. It is only for learning purpose and I hope people will guide me in the correct direction.

I want only one thread to access the variable sum at one time so that it does not get overwritten.

The code I have is below.

using System;
using System.Threading.Tasks;


class ThreadTest
{
    private Object thisLock = new Object();
    static int sum = 0;
    public void RunMe()
    {
        lock (thisLock)
        {
            sum = sum + 1;
        }
    }

    static void Main()
    {
        ThreadTest b = new ThreadTest();
        Task t1 = new Task(()=>b.RunMe());
        Task t2= new Task(() => b.RunMe());
        t1.Start();
        t2.Start();
        Task.WaitAll(t1, t2);
        Console.WriteLine(sum.ToString());
        Console.ReadLine();
    }
}

Question -Am i right in this code ?

Question-Can I do it without lock because I read it somewhere that it should be avoided as it does not allow task to communicate with each other.I have seen some examples with async and await but I am using .Net 4.0 .

Thanks

Answer 1

Am i right in this code

Implementation wise Yes, but understanding wise No, as you have mixed up the new and old world while trying to implement the logic, let me try to explain in detail.

• Task t1 = new Task(()=>b.RunMe()); doesn't mean as expected as in case of Thread API a new thread every time
• Task API will invoke a thread pool thread, so chances are two Task objects t1,t2, gets executed on same thread most of the times for a short running logic and there's never a race condition, which needs an explicit lock, while trying to update the shared object
• Better way to prevent race condition for Sum object would be Interlocked.Increment(ref sum), which is a thread safe mechanism to do basic operations on primitive types

• For the kind of operation you are doing a better API would be Parallel.For, instead of creating a separate Task, the benefit would be you can run any number of such increment operations with minimal effort, instead of creating a Separate Task and it automatically blocks the Main thread, so your code shall look like:

  using System;
  using System.Threading.Tasks;
  
  class ThreadTest
  {    
     public static int sum;
  }
  
  static void Main()
  {
  
   Parallel.For(0, 1, i =>
  {
      // Some thread instrumentation
      Console.WriteLine("i = {0}, thread = {1}", i,
      Thread.CurrentThread.ManagedThreadId);
  
      Interlocked.Increment(ref ThreadTest.sum);
  });
  
     Console.WriteLine(ThreadTest.sum.ToString());
     Console.ReadLine();
  }
  }
  

• While using the Thread instrumentation you will find that chances are that for two loops, 0,1, managed thread id is same, thus obviating the need for thread safety as suggested earlier

Answer 2

Answer 1:

This is threadsafe for the code that you posted.

However, as Sam has pointed out, this is not currently threadsafe in the general case because the field being incremented is static, but the locking object is not static.

This means that two separate instances of ThreadTest could be created on two separate threads, and then RunMe() could be called from those threads and because each instance has a separate locking object, the locking wouldn't work.

The solution here is to make the locking object static too.

Answer 2:

You can do this without explicit locking using Interlocked.Increment():

public void RunMe()
{
    Interlocked.Increment(ref sum);
}

Answer 3

Now to the point, as I was adding some unhappy comments about downvotes, that have no reasons:).

Your scenario is working and is classic multithreading usage. Classic because of using system lock, that is lock are actually WinAPI locks of the OS, so in order to synchronize, the code has to manage to ring down to the OS and back and of course lose some time with switching threads as some contention may happen especially if you would access RunMe multiple times in each thread running given task or created even more concurrent tasks thank 2.

Please try look on atomic operations. For your scenario it would work very well, Interlocked.Increment(ref sum). From there you have to restrain yourself from directly accessing the sum, but that is not a problem, because the Increment method is returning latest result.

Another option is to use SpinLock, that is IF YOU OPERATION IS REALLY FAST. NEVER ON something ,like Console.WriteLine or any other system operations or long running calculations etc.

Here Inrelocked examples:

using System;
using System.Threading;
using System.Threading.Tasks;

class ThreadTest
{
    /// <summary>  DO NOT TOUCH ME DIRECTLY  </summary>
    private static int sum/* = 0 zero is default*/;
    private static int Add(int add) => Interlocked.Add(ref sum, add);
    private static int Increment() => Interlocked.Increment(ref sum);
    private static int Latest() => Interlocked.Add(ref sum, 0);
    private static void RunMe() => Increment();

    static void Main()
    {
        Task t1 = new Task(RunMe);
        Task t2 = new Task(RunMe);
        t1.Start();
        t2.Start();
        Task.WaitAll(t1, t2);
        Console.WriteLine(Latest().ToString());
        Console.ReadLine();
    }
}