c# lock not working as expected

Question

This class uses lock and Interlocked.

Both increaseCount.with_lock.Run(); and increaseCount.with_interlock.Run(); prints between 96-100.

I am expecting both of them to print always 100. What did I make mistake?

public static class increaseCount {
    public static int counter = 0;
    public static readonly object myLock = new object();
    public static class with_lock {
        public static void Run() {
            List<Thread> pool = new List<Thread>();
            for(int i = 0; i < 100; i++) {
                pool.Add(new Thread(f));
            }
            Parallel.ForEach(pool, x => x.Start());
            Console.WriteLine(counter); //should print 100
        }

        static void f() {
            lock(myLock) {
                counter++;
            }
        }
    }

    public static class with_interlock {
        public static void Run() {
            List<Thread> pool = new List<Thread>();
            for(int i = 0; i < 100; i++) {
                pool.Add(new Thread(f));
            }
            Parallel.ForEach(pool, x => x.Start());
            Console.WriteLine(counter);//should print 100
        }

        static void f() {
            Interlocked.Add(ref counter, 1);
        }
    }
}

Your code is fine. The only problem is your expectation. Basically, not all 100 threads get to run untill the counter is displayed. Try putting a Thread.Sleep(1000) before the Console.WriteLine(counter) and you shall see what I mean. — Mihai Caracostea, Feb 07 '15 at 23:39

score 4 · Accepted Answer · answered Feb 07 '15 at 23:39

In both cases, you start up your threads but you don't wait for them to complete so you don't reach the 100 before you print the result and the app closes.

If after you start all thread you would wait for all these threads to complete with Thread.Join you would always get the correct result:

List<Thread> pool = new List<Thread>();
for (int i = 0; i < 100; i++)
{
    pool.Add(new Thread(f));
}

Parallel.ForEach(pool, x => x.Start());
foreach (var thread in pool)
{
    thread.Join();
}

Console.WriteLine(counter);

Note: This seems like a test of some kind, but you should know that blocking multiple threads on a single lock is a huge waste of resources.

score 1 · Answer 2 · answered Feb 07 '15 at 23:40

I believe it's because your Parallel.Foreach call simply calls start on all the threads in pool but they haven't necessarily completed by the time the loops finished and the Console.WriteLine is called. If you were to insert a Thread.Sleep(5000); // 5s sleep or similar before the Console.WriteLine it would likely always print out what you expect.

score 1 · Answer 3 · answered Feb 07 '15 at 23:40

Your code is fine. The only problem is your expectation. Basically, not all 100 threads get to run untill the counter is displayed. Try putting a Thread.Sleep(1000) before the Console.WriteLine(counter) and you shall see what I mean.

Edit: wrongly posted as a comment the first time.

c# lock not working as expected

3 Answers3