33

I'm using C# & .NEt 3.5. What is the difference between the OptionA and OptionB ?

class MyClass
{
    private object m_Locker = new object();
    private Dicionary<string, object> m_Hash = new Dictionary<string, object>();

    public void OptionA()
    {
        lock(m_Locker){ 
          // Do something with the dictionary
        }
    }

    public void OptionB()
    {
        lock(m_Hash){ 
          // Do something with the dictionary
        }
    }       
}

I'm starting to dabble in threading (primarly for creating a cache for a multi-threaded app, NOT using the HttpCache class, since it's not attached to a web site), and I see the OptionA syntax in a lot of the examples I see online, but I don't understand what, if any, reason that is done over OptionB.

Matt
  • 41,216
  • 30
  • 109
  • 147

8 Answers8

28

Option B uses the object to be protected to create a critical section. In some cases, this more clearly communicates the intent. If used consistently, it guarantees only one critical section for the protected object will be active at a time:

lock (m_Hash)
{
    // Across all threads, I can be in one and only one of these two blocks
    // Do something with the dictionary
}
lock (m_Hash)
{
    // Across all threads, I can be in one and only one of these two blocks
    // Do something with the dictionary
}

Option A is less restrictive. It uses a secondary object to create a critical section for the object to be protected. If multiple secondary objects are used, it's possible to have more than one critical section for the protected object active at a time.

private object m_LockerA = new object();
private object m_LockerB = new object();

lock (m_LockerA)
{
    // It's possible this block is active in one thread
    // while the block below is active in another
    // Do something with the dictionary
}
lock (m_LockerB)
{
    // It's possible this block is active in one thread
    // while the block above is active in another
    // Do something with the dictionary
}

Option A is equivalent to Option B if you use only one secondary object. As far as reading code, Option B's intent is clearer. If you're protecting more than one object, Option B isn't really an option.

Corbin March
  • 25,526
  • 6
  • 73
  • 100
  • It is _probably_ fine to use OptionB because `m_Hash` is private. OptionA is more common because it is generally considered dangerous to lock on an object that is not private / that you hand out references to. It might be hard or impossible to track (at the time of writing your code) where that instance might end up, and thus who else might lock on it. (If you really want to send the object around and possibly lock it in other classes too, then you should probably write a thread safe wrapper around your object and share a wrapper instance instead.) – AnorZaken Mar 03 '19 at 15:54
11

It's important to understand that lock(m_Hash) does NOT prevent other code from using the hash. It only prevents other code from running that is also using m_Hash as its locking object.

One reason to use option A is because classes are likely to have private variables that you will use inside the lock statement. It is much easier to just use one object which you use to lock access to all of them instead of trying to use finer grain locks to lock access to just the members you will need. If you try to go with the finer grained method you will probably have to take multiple locks in some situations and then you need to make sure you are always taking them in the same order to avoid deadlocks.

Another reason to use option A is because it is possible that the reference to m_Hash will be accessible outside your class. Perhaps you have a public property which supplies access to it, or maybe you declare it as protected and derived classes can use it. In either case once external code has a reference to it, it is possible that the external code will use it for a lock. This also opens up the possibility of deadlocks since you have no way to control or know what order the lock will be taken in.

Daniel Plaisted
  • 16,674
  • 4
  • 44
  • 56
10

Actually, it is not good idea to lock on object if you are using its members. Jeffrey Richter wrote in his book "CLR via C#" that there is no guarantee that a class of object that you are using for synchronization will not use lock(this) in its implementation (It's interesting, but it was a recommended way for synchronization by Microsoft for some time... Then, they found that it was a mistake), so it is always a good idea to use a special separate object for synchronization. So, as you can see OptionB will not give you a guarantee of deadlock - safety. So, OptionA is much safer that OptionB.

D.P.
  • 230
  • 3
  • 11
3

It's not what you're "Locking", its the code that's contained between the lock { ... } thats important and that you're preventing from being executed.

If one thread takes out a lock() on any object, it prevents other threads from obtaining a lock on the same object, and hence prevents the second thread from executing the code between the braces.

So that's why most people just create a junk object to lock on, it prevents other threads from obtaining a lock on that same junk object.

Eoin Campbell
  • 43,500
  • 17
  • 101
  • 157
  • 1
    I don't see how the second and third paragraphs follow the first, which claims that what you're locking on isn't important. – Jon Skeet Oct 23 '08 at 19:30
2

I think the scope of the variable you "pass" in will determine the scope of the lock. i.e. An instance variable will be in respect of the instance of the class whereas a static variable will be for the whole AppDomain.

Looking at the implementation of the collections (using Reflector), the pattern seems to follow that an instance variable called SyncRoot is declared and used for all locking operations in respect of the instance of the collection.

VirtualStaticVoid
  • 1,656
  • 3
  • 15
  • 21
1

Well, it depends on what you wanted to lock(be made threadsafe).

Normally I would choose OptionB to provide threadsafe access to m_Hash ONLY. Where as OptionA, I would used for locking value type, which can't be used with the lock, or I had a group of objects that need locking concurrently, but I don't what to lock the whole instance by using lock(this)

faulty
  • 8,117
  • 12
  • 44
  • 61
0

Locking the object that you're using is simply a matter of convenience. An external lock object can make things simpler, and is also needed if the shared resource is private, like with a collection (in which case you use the ICollection.SyncRoot object).

Mark Cidade
  • 98,437
  • 31
  • 224
  • 236
0

OptionA is the way to go here as long as in all your code, when accessing the m_hash you use the m_Locker to lock on it.

Now Imagine this case. You lock on the object. And that object in one of the functions you call has a lock(this) code segment. In this case that is a sure unrecoverable deadlock

John Demetriou
  • 4,093
  • 6
  • 52
  • 88