Locking on field or local variable?

Question

After I read this question with an answer from Marc....

I sometimes see people locking on a local variable.

Is this code broken?

public void Do()
{
 object  o  = new Object();
 lock (o)
     {
      ...
     }
}

I believe object o = new Object(); should be outside the method as a Field.

Since each thread is getting a new instance of o , there will be multiple locks.

What am I missing here? Shouldn't it lock on fields in this specific case?

Answer 1

Yes. It is broken.

You want a static readonly object as a private field to lock on. As you suspect, your example code creates a new object every time you call Do, and hence the lock will have nothing to hold onto and won't work at all.

private static object syncRoot = new object();

lock (syncRoot) { }

Answer 2

Locking on local variable, the lock won's work. Locking on global variable can take effect to synchronize multiple thread.

using System;
        using System.Collections.Generic;
        using System.Linq;
        using System.Text;
        using System.Threading.Tasks;
        using System.Threading;

        namespace testLock
        {
            class Program
            {
                public static void Main()
                {
                    // Start a thread that calls a parameterized static method.
                    for(int i = 0; i< 10;i++)
                    {
                        Thread newThread = new Thread(DoWork);
                        newThread.Start(i);
                    }

                    Console.ReadLine();
                }

                static object gObject= new object();
                public static void DoWork(object data)
                {
                    int len = (int)data % 3;
                    object tmp = new object();
                    Console.WriteLine("to lock...... Data='{0}'  sleepTime:{1}", data, len);
                    lock (tmp)//tmp won't work, change tmp to gObject to see different output, which is good locking case)
                    {
                        Console.WriteLine("in lock...... Data='{0}'  sleepTime:{1}", data, len);

                        Thread.Sleep(  len* 1000);
                        Console.WriteLine("Static thread procedure. Data='{0}'  sleepTime:{1}", data, len);
                    }
                }

            }
        }

    **Lock temp variable,will output:**
    to lock...... Data='1'  sleepTime:1
    in lock...... Data='1'  sleepTime:1
    to lock...... Data='2'  sleepTime:2
    in lock...... Data='2'  sleepTime:2
    to lock...... Data='0'  sleepTime:0
    in lock...... Data='0'  sleepTime:0
    Static thread procedure. Data='0'  sleepTime:0
    to lock...... Data='3'  sleepTime:0
    in lock...... Data='3'  sleepTime:0
    Static thread procedure. Data='3'  sleepTime:0
    to lock...... Data='4'  sleepTime:1
    in lock...... Data='4'  sleepTime:1
    to lock...... Data='5'  sleepTime:2
    in lock...... Data='5'  sleepTime:2
    to lock...... Data='6'  sleepTime:0
    in lock...... Data='6'  sleepTime:0
    Static thread procedure. Data='6'  sleepTime:0
    to lock...... Data='7'  sleepTime:1
    in lock...... Data='7'  sleepTime:1
    to lock...... Data='8'  sleepTime:2
    in lock...... Data='8'  sleepTime:2
    to lock...... Data='9'  sleepTime:0
    in lock...... Data='9'  sleepTime:0
    Static thread procedure. Data='9'  sleepTime:0
    Static thread procedure. Data='1'  sleepTime:1
    Static thread procedure. Data='4'  sleepTime:1
    Static thread procedure. Data='7'  sleepTime:1
    Static thread procedure. Data='2'  sleepTime:2
    Static thread procedure. Data='5'  sleepTime:2
    Static thread procedure. Data='8'  sleepTime:2

    **Then lock gObject, will print:**
    to lock...... Data='0'  sleepTime:0
    in lock...... Data='0'  sleepTime:0
    to lock...... Data='1'  sleepTime:1
    to lock...... Data='2'  sleepTime:2
    Static thread procedure. Data='0'  sleepTime:0
    in lock...... Data='1'  sleepTime:1
    to lock...... Data='3'  sleepTime:0
    to lock...... Data='4'  sleepTime:1
    to lock...... Data='5'  sleepTime:2
    to lock...... Data='6'  sleepTime:0
    to lock...... Data='7'  sleepTime:1
    to lock...... Data='8'  sleepTime:2
    to lock...... Data='9'  sleepTime:0
    Static thread procedure. Data='1'  sleepTime:1
    in lock...... Data='5'  sleepTime:2
    Static thread procedure. Data='5'  sleepTime:2
    in lock...... Data='9'  sleepTime:0
    Static thread procedure. Data='9'  sleepTime:0
    in lock...... Data='2'  sleepTime:2
    Static thread procedure. Data='2'  sleepTime:2
    in lock...... Data='8'  sleepTime:2
    Static thread procedure. Data='8'  sleepTime:2
    in lock...... Data='7'  sleepTime:1
    Static thread procedure. Data='7'  sleepTime:1
    in lock...... Data='4'  sleepTime:1
    Static thread procedure. Data='4'  sleepTime:1
    in lock...... Data='3'  sleepTime:0
    Static thread procedure. Data='3'  sleepTime:0
    in lock...... Data='6'  sleepTime:0
    Static thread procedure. Data='6'  sleepTime:0