Unordered threads problem

Question

I had asked question about lock in here and people responded there is no problem in my lock implementation. But i catched problem. Here is same lock implementation and i am getting weird result. I expect to see numbers starts from 1 but it starts from 5.Example is at below.

class Program
{
    static object locker = new object();
    static void Main(string[] args)
    {
        for (int j = 0; j < 100; j++)
        {
            (new Thread(new ParameterizedThreadStart(dostuff))).Start(j);
        }
        Console.ReadKey();
    }
    static void dostuff(dynamic input)
    {
        lock (locker)
        {
            Console.WriteLine(input);
        }
    }
}

Answer 1

The code is fine. But you cannot guarantee the order the threads are executed in. When I run the code I get:

0 1 3 5 2 4 6 10 9 11 7 12 8 etc

If you need to run the threads in a specified order, you could look into using ThreadPool.QueueUserWorkItem instead.

class Program
{
    static object locker = new object();
    static EventWaitHandle clearCount =new EventWaitHandle(false, EventResetMode.ManualReset);
    static void Main(string[] args)
    {
        for (int j = 0; j < 100; j++)
        {
            ThreadPool.QueueUserWorkItem(dostuff, j);
        }
        clearCount.WaitOne();
    }
    static void dostuff(dynamic input)
    {
        lock (locker)
        {
            Console.WriteLine(input);
            if (input == 99) clearCount.Set();
        }
    }
}

Answer 2

It doesn't make sense to put a lock where you're putting it, as you're not locking code which changes a value shared by multiple threads. The section of code you're locking doesn't change any variables at all.

The reason the numbers are out of order is because the threads aren't guaranteed to start in any particular order, unless you do something like @Mikael Svenson suggests.

For an example of a shared variable, if you use this code:

class Program
{
    static object locker = new object();
    static int count=0;

    static void Main(string[] args)
    {
        for (int j = 0; j < 100; j++)
        {
            (new Thread(new ParameterizedThreadStart(dostuff))).Start(j);
        }
        Console.ReadKey();
    }

    static void dostuff(object Id)
    {
        lock (locker)
        {
            count++;
            Console.WriteLine("Thread {0}: Count is {1}", Id, count);
        }
    }
}

You'll probably see that the Thread numbers aren't in order, but the count is. If you remove the lock statement, the count won't be in order either.

Answer 3

You have a couple of problems and wrong assumptions here.

• Creating 100 threads in this fashion is not recommended.
• The threads are not going to execute in the order they are started.
• Placing the lock where you have it will effectively serialize the execution of the threads immediately removing any advantage you were hoping to gain by using threading.

The best approach to use is to partition your problem into separate independent chunks which can be computed simultaneously using only the least amount of thread synchronization as possible. These partitions should be executed on small and fairly static number of threads. You can use the ThreadPool, Parallel, or Task classes for doing this.

I have included a sample pattern using the Parallel.For method. To make the sample easy to understand lets say you have a list of objects that you want to clone and land into a separate list. Lets assume the clone operation is expensive and that you want to parallelize the cloning of many objects. Here is how you would do it. Notice the placement and limited use of the lock keyword.

public static void Main()
{
  List<ICloneable> original = GetCloneableObjects();
  List<ICloneable> copies = new List<ICloneable>();
  Parallel.For(0, 100,
    i =>
    {
      ICloneable cloneable = original[i];
      ICloneable copy = cloneable.Clone();
      lock (copies)
      {
        copies.Add(copy);
      }
    });
}