c# Lock Doesn`t work / Critical Section

I`m trying to advance a static (int) counter using two different threads in a FOR loop, so if the loop is runs 10 times I (should) get counter=20. for some reason i keep getting different values each time i run the loop (19,20,21), even though i used a LOCK to access that counter, (the code runs in console):

public static int Counter = 0;
static object syncObject = new object();

static void Main(string[] args)
        { 

int ForLength = 10;

            Thread FirstThread, SecondThread;

            for (int i = 0; i <= ForLength; i++)
            {
                FirstThread = new Thread(RaiseCounter);
                FirstThread.IsBackground = false;

                SecondThread = new Thread(RaiseCounter);
                SecondThread.IsBackground = false;               


                FirstThread.Start();                
                SecondThread.Start();

                //Console.WriteLine(Counter);
            }


            Console.WriteLine(Counter);

            Console.ReadLine();
        }

        public static void RaiseCounter ()
        {
            lock (syncObject)
            {
                Counter++;
            }
        }
Jon Skeet
people
quotationmark

You've got three problems:

  • You're actually running the loop 11 times:

    for (int i = 0; i <= ForLength; i++) // i=0 to i=10 *inclusive*

  • You're not joining on your threads (or sleeping), so some of them may not have finished by the time you write the output

  • You're not synchronizing on syncObject when you read Counter in the main thread, so you may not observe the most recently written value

If you don't want to use Thread.Join, just add a call to Thread.Sleep, e.g. Thread.Sleep(5000) - it's extremely likely that all the threads will have completed after that. Then you can use:

lock (syncObject)
{
    Console.WriteLine(Counter);
}

In short, there's nothing wrong with lock, although you'd be better off using Interlocked.Increment.

people

See more on this question at Stackoverflow