locking c# using private variables

Question

according to Eric Gunnerson

Don’t

• Use lock(this)
• Use lock(typeof())

Do Lock on a private variable, not on something the user can see Use “object key = new object()” if you need a private key to lock on

what is the reason??

Answer 1

You should really search for old questions on this before posting a new one. Lock

Also Darin Dimitrov is wrong when he says locks on private variables are dangerous. Locks on private variables serve the purpose of synchronizing the resources of a particular instance of your class. It can happen when you have

// A Client which listens to several servers
public class Client
{
private static object logSync = new object();
private readonly Dictionary<string, Server> servers = new Dictionary<string, Server>();// .... some code for initialization ...

// Disposing a server. 
public void Dispose (string serverName)
{
    //  the lock needed here is on private variable. This purpose cannot be achieved with a      
    //  lock on private static object. Well you can achieve the purpose but you will block
    //  all Client instances when you do so, which is pointless.
    //  Also notice that services is readonly, which is convenient
    //  because that is the object we took a lock on. The lock is on the same object always
    //  there is no need to unnecessarily create objects for locks.  
    lock(services)
    {
        // ... Do something cleanup here ...
        Server server;
        if (servers.TryGetValue(serverName, out server))
        {
             server.Dispose();
             servers.Remove(serverName);
        }
    }
}

// on some message that has to be logged  
public void OnMessage(string message, Server server)
{
    // This makes sure that all clients log to the same sink and 
    // the messages are processed in the order of receipt
    lock (logSync)
    {
        Log(evt);
    }
}

}

Answer 2

what is the reason??

Because anything that is not private means that could be used from the outside to lock on by someone else or some code that is outside from your control leading to deadlocks.

The best practice is to lock on private static variables, like this:

private static object _syncRoot = new object();

and then:

lock(_syncRoot)
{
    ...
}

private instance variables could also be dangerous since the instance of your class is not something that you as implementer of the class own. It's the consumer of the class that owns the instance.