C# thread locking and using a lock object; what should the correct scope be for the lock object?

Is there a difference between using a lock object that is开发者_StackOverflow declared as a field of a class as opposed to local scope?

For example: Is there a difference between using lockObject1 or lockObject2 in the below example?

public class Test()
{
   private Object lockObject1 = new Object();

   public void FooBar()
   {
       lock(lockObject1)
       {
          //Thread safe area 1
       }

       var lockObject2 = new Object();
       lock(lockObject2)
       {
           //Thread safe area 2
       }
   }
}

It seems like most examples always seem to glaze over the importance of scoping of the chosen lock object.

The local lock object will not really be providing any thread safety since multiple threads running FooBar will each have their own object rather than sharing a single object for locking. (Sadly, I have seen this very thing in code headed toward production before I raised the issue and got it corrected in time.)

You should always use a private static member variable if you want to lock all instances of an object. Use a non-static private member if you want to lock a specific instance.

Never use a publicly visible member, or 'this', as you never know when someone else is going to use it for their own lock, and possibly cause deadlocks.

public class Test()
{
   private static Object lockObjectAll = new Object();
   private Object lockObjectInstance = new Object();

   public void FooBar()
   {
       lock (lockObjectAll)
       {
          // Thread safe for all instances
       }

       lock (lockObjectInstance)
       {
          // Thread safe for 'this' instance
       }
   }
}

your lockObject2 won't protect anything since, it's not going to be locked anywhere else. Do note that multiple calls to FooBar by different threads will not be protected since it's a new instance each time FooBar is called. Also, do note that lockObject1 isn't much better. It allows locking between calls on the same instance. This is good if it's only member variables being accessed. But if some global resource is being accessed, lockObject1 won't protect fully since each instance of the class has its own instance. As suggested by @Haoest, making lockObject1 static should fix this problem.

Make lockObject1 static and you should be alright.

lockObject2 should also be static field to be effective.