Question about lock objects and sub classes

Question

Hello,

So, I have a base class which has a private locking object like so:

class A
{
    private object mLock = new object();

    public virtual void myMethod()
    {
        lock(mLock)
        {
            // CS
        }
    }
}

This locking object is used for most of A's operations... because they need to be thread safe.

Now, lets say I inherit from A like so:

class B : public A
{
    public override void myMethod()
    {
        lock(???)
        {
            // CS of mymethod here

            // Call base (thread safe alread)
            base.myMethod();
        }
    }
}

What is the convention for making B thread safe? Should B also have a private locking object like A does? What if I need to call the base method like above?

I guess I'm just curious what the convention is for making subclasses thread safe when the base class is already thread safe. Thanks!

Edit: Some are asking what I mean by "thread safe"... I'll clarify by adding that I'm trying to achieve thread safety through mutual exclusion... Only one thread at a time should be executing code which may alter the state of the object.

Answer 1

You would potentially expose the lock used by A:

protected object SyncLock { get { return mLock; } }

If B had its own lock object, you could easily end up with:

• Different locks being used, potentially leading to race conditions. (It may be fine - if the operations occurring while holding lock B are orthogonal to those occurring while holding lock A, you may just get away with it.)
• Locks being taken out recursively in different orders, leading to deadlocks. (Again, the orthogonality argument applies.)

As locks are recursive in .NET (for better or worse), if your override locks on the same object as A's implementation, it's fine to call base.myMethod and recursively acquire/release the lock.

Having said all of this, I'm keen on making most classes non-thread safe or immutable (only classes which are about threading need threading knowledge) and most classes don't need to be designed for inheritance IMO.

Answer 2

I thought the appropriate thing to do here is use lock(this), instead of creating an instance of another object just for locking. That should work for both cases.

Answer 3

It depends really. If your subclass is only using the safe methods from your base class and doesn't add any extra unsafe state, than you don't have to do anything (preferred). If it add some extra state which is not correlated with the state of the base class, then you can create a separate lock object in the subclass and use that. If, on the other hand, you need to make sure that the new state and the state from the base class are changed in some sort "transactional" way, then I would make the lock object in the base class protected and use that.

Answer 4

What do you mean by "thread safe"? If this includes any ideas about reentrancy -- the idea that a method can only see the object in a good state, so only one method can be running at once -- then locking around methods may not do what you expect. For example, if your object ever calls out to another object in any way (an event?), and that object calls back in to the original object, the inner call will see the object in a "bad" state.

For this reason, when you're locking the whole object like this, you have to be careful about the code that runs inside the lock: "bad" code running inside the lock can compromise the correctness of your code.

Since subclassing introduces code that's outside the control of the original object, it's often seen as a dangerous way to work with self-locking classes like this, so one popular convention is "don't do it."

If, given the possible problems, you still want to take this route, then making the lockable object protected rather than private will allow you to include subclass operations inside the same base-class lock.

Answer 5

There's not really enough information to answer your question, but first things first.

Just using lock blocks does not make your code thread-safe.

All you're doing in A is making it impossible for more than one thread at a time to call any function whose body you enclose in the lock(mLock). Thread safety is a broad term, and preventing concurrent calls is just one aspect (and isn't always what you want or need). If that is what you need, then you've obviously taken the right approach. Just be sure about it.

Second, what you need to expose to your subclass is not evident from the code above. You have three scenarios:

1. B might call protected (or internal if it's in the same assembly as A) functions on A that are not enclosed in the lock(mLock) blocks
2. B will only call functions on A that are enclosed in the lock(mLock) blocks and doesn't provide any operations of its own that require you to prevent concurrent calls
3. B will only call functions on A that are enclosed in the lock(mLock) blocks and also provides operations of its own that require you to prevent concurrent calls.

Which really boils down into two unrelated questions:

1. Will B interact with A in a way that needs to be protected (in other words, in a way that isn't already protected)?
2. Will B expose functionality that needs to be protected, and, if so, should they use the same locking object?

If 1) is true of 2) is true and they should use the same locking object, then you'll need to expose your locking object via a protected property and use that in all of your locks (I would suggest using it within A as well for readability).

If neither is true, then don't worry about it. If 2) is true but they don't need to use the same locking object (in other words, it would be acceptable to have concurrent calls to A and B), then don't worry about exposing the locking object. Remember that any calls that B makes into a function on A that's been protected by the lock(mLock) is going to lock on that object anyway, regardless of any outer locks.