9

I have been learning about locking on threads and I have not found an explanation for why creating a typical System.Object, locking it and carrying out whatever actions are required during the lock provides the thread safety?

Example

object obj = new object()
lock (obj) {
  //code here
}

At first I thought that it was just being used as a place holder in examples and meant to be swapped out with the Type you are dealing with. But I find examples such as Dennis Phillips points out, doesn't appear to be anything different than actually using an instance of Object.

So taking an example of needing to update a private dictionary, what does locking an instance of System.Object do to provide thread safety as opposed to actually locking the dictionary (I know locking the dictionary in this case could case synchronization issues)? What if the dictionary was public?

//what if this was public?
private Dictionary<string, string> someDict = new Dictionary<string, string>();

var obj = new Object();
lock (obj) {
  //do something with the dictionary
}
Community
  • 1
  • 1
pghtech
  • 3,642
  • 11
  • 48
  • 73
  • If you're worried about the `dictionary` being public, write (or copy) a `Dictionary` wrapper wrap all the functionality in a `lock`. See [What's the best way of implementing a thread-safe Dictionary in .NET?](http://stackoverflow.com/questions/157933) for examples and explanation. Or just use a [`ConcurrentDictionary`](http://msdn.microsoft.com/en-us/library/dd287191.aspx) – Brian Jan 11 '12 at 16:22

5 Answers5

9

The lock itself provides no safety whatsoever for the Dictionary<TKey, TValue> type. What a lock does is essentially

For every use of lock(objInstance) only one thread will ever be in the body of the lock statement for a given object (objInstance)

If every use of a given Dictionary<TKey, TValue> instance occurs inside a lock. And every one of those lock uses the same object then you know that only one thread at a time is ever accessing / modifying the dictionary. This is critical to preventing multiple threads from reading and writing to it at the same time and corrupting its internal state.

There is one giant problem with this approach though: You have to make sure every use of the dictionary occurs inside a lock and it uses the same object. If you forget even one then you've created a potential race condition, there will be no compiler warnings and likely the bug will remain undiscovered for some time.

In the second sample you showed you're using a local object instance (var indicates a method local) as a lock parameter for an object field. This is almost certainly the wrong thing to do. The local will live only for the lifetime of the method. Hence 2 calls to the method will use lock on different locals and hence all methods will be able to simultaneously enter the lock.

JaredPar
  • 733,204
  • 149
  • 1,241
  • 1,454
8

It used to be common practice to lock on the shared data itself:

private Dictionary<string, string> someDict = new Dictionary<string, string>();

lock (someDict ) 
{
  //do something with the dictionary
}

But the (somewhat theoretical) objection is that other code, outside of your control, could also lock on someDict and then you might have a deadlock.

So it is recommended to use a (very) private object, declared in 1-to-1 correspondence with the data, to use as a stand-in for the lock. As long as all code that accesses the dictionary locks on on obj the tread-safety is guaranteed.

// the following 2 lines belong together!!
private Dictionary<string, string> someDict = new Dictionary<string, string>();
private object obj = new Object();


// multiple code segments like this
lock (obj) 
{
  //do something with the dictionary
}

So the purpose of obj is to act as a proxy for the dictionary, and since its Type doesn't matter we use the simplest type, System.Object.

What if the dictionary was public?

Then all bets are off, any code could access the Dictionary and code outside the containing class is not even able to lock on the guard object. And before you start looking for fixes, that simply is not an sustainable pattern. Use a ConcurrentDictionary or keep a normal one private.

H H
  • 263,252
  • 30
  • 330
  • 514
  • +1 for mentioning that other code could use locks you are not aware of. Such as Dictionary locking on itself internally. – Roger Lindsjö Jan 11 '12 at 14:34
  • By putting the object instance at the instance member level, does that lock the entire class? – pghtech Jan 11 '12 at 15:30
  • Also, it seems that you can still get deadlocks at the method level where you are locking on obj if two threads are trying to call a method. If I am correct, is there a way to manage request at the method level? Like a request queue? – pghtech Jan 11 '12 at 15:37
  • @pghtech, a) the lock should be at the same level as the Dictionary. b) Yes you can still have a deadlock but it would be all your code so you can also fix it. – H H Jan 11 '12 at 16:55
2

The object which is used for locking does not stand in relation to the objects that are modified during the lock. It could be anything, but should be private and no string, as public objects could be modified externally and strings could be used by two locks by mistake.

MatthiasG
  • 4,434
  • 3
  • 27
  • 47
1

So far as I understand it, the use of a generic object is simply to have something to lock (as an internally lockable object). To better explain this; say you have two methods within a class, both access the Dictionary, but may be running on different threads. To prevent both methods from modifying the Dictionary at the same time (and potentially causing deadlock), you can lock some object to control the flow. This is better illustrated by the following example:

private readonly object mLock = new object();

public void FirstMethod()
{
    while (/* Running some operations */)
    {
        // Get the lock
        lock (mLock)
        {
            // Add to the dictionary
            mSomeDictionary.Add("Key", "Value");
        }
    }
}

public void SecondMethod()
{
    while (/* Running some operation */)
    {
        // Get the lock
        lock (mLock)
        {
            // Remove from dictionary
            mSomeDictionary.Remove("Key");
        }
    }
}

The use of the lock(...) statement in both methods on the same object prevents the two methods from accessing the resource at the same time.

Samuel Slade
  • 8,405
  • 6
  • 33
  • 55
0

The important rules for the object you lock on are:

  1. It must be an object visible only to the code that needs to lock on it. This avoids other code also locking on it.

    1. This rules out strings that could be interned, and Type objects.
    2. This rules out this in most cases, and the exceptions are too few and offer little in exploiting, so just don't use this.
    3. Note also that some cases internal to the framework lock on Types and this, so while "it's okay as long as nobody else does it" is true, but it's already too late.
  2. It must be static to protect static static operations, it may be instance to protect instance operations (including those internal to a instance that is held in a static).

  3. You don't want to lock on a value-type. If you really wanted too you could lock on a particular boxing of it, but I can't think of anything that this would gain beyond proving that it's technically possible - it's still going to lead to the code being less clear as to just what locks on what.

  4. You don't want to lock on a field that you may change during the lock being held, as you'll no longer have the lock on what you appear to have the lock on (it's just about plausible that there's a practical use for the effect of this, but there's going to be an impedance between what the code appears to do at first read and what it really does, which is never good).

  5. The same object must be used to lock on all operations that may conflict with each other.

  6. While you can have correctness with overly-broad locks, you can get better performance with finer. E.g. if you had a lock that was protecting 6 operations, and realised that 2 of those operations couldn't interfere with the other 4, so you changed to having 2 lock objects, then you can gain by having better coherency (or crash-and-burn if you were wrong in that analysis!)

The first point rules out locking on anything that is either visible or which could be made visible (e.g. a private instance that is returned by a protected or public member should be considered public as far as this analysis goes, anything captured by a delegate could end up elsewhere, and so on).

The last two points can mean that there's no obvious "type you are dealing with" as you put it, because locks don't protect objects, the protect operations done on objects and you may either have more than one object affected, or the same object affected by more than one group of operations that must be locked.

Hence it can be good practice to have an object that exists purely to lock on. Since it's doing nothing else, it can't get mixed up with other semantics or written over when you don't expect. And since it does nothing else it may as well be the lightest reference type that exists in .NET; System.Object.

Personally, I do prefer to lock on an object related to an operation when it does clearly fit the bill of the "type you are dealing with", and none of the other concerns apply, as it seems to me to be quite self-documenting, but to others the risk of doing it wrong out-weighs that benefit.

Jon Hanna
  • 110,372
  • 10
  • 146
  • 251