Locking via inheritance rather than composition

Question

In most code I have written or reviewed, locking is achieved via composition, where a class owns a critical section or mutex:

class MyClass
{
    Mutex mMutex;
};

and when mutable members can potentially be accessed via multiple threads, we acquire and release the lock via RAII like this:

void MyClass::Method()
{
    Lock lock(mMutex);
    // ...
}

Today I reviewed some code where the code was implementing locking via inheritance, like this:

class MyClass : public Mutex
{
    // ...
};

And locking is performed by the class locking "itself":

void MyClass::Method()
{
    Lock lock(this);
    // ...
}

Are there any advantages or disadvantages of this approach? Or is this just a style issue?

Answer 1

Private inheritance might make a little more sense for this, since private inheritance is more or less a way to implement composition. however, I unless there's a need to inherit from Mutex (like if Mutex had protected members that needed to be accessed), I think that the more standard composition approach of making it a member would probably cause less confusion (like people wondering why inheritance was used instead of making Mutex a member).

One situation where it might make sense to inherit publicly is where you want clients to be able to lock on that object for some reason, then they can simply treat MyClass as a Mutex. That might not be a great design choice, since you open up more opportunities for deadlock if the object were locked in unanticipated ways. If the Mutex remains private (whether by inheritance or by standard composition) the class can be more certain of exactly how the lock is used.

I wouldn't be surprised if the person who introduced this design was influenced by .NET where any object is 'lockable'. Note that in .NET it used to be common to lock(this), but that idiom has fallen into disfavor, and now it's considered more correct to have a specific private object member to use for locking.

Answer 2

This almost never makes sense. Is MyClass some kind of synchronization object that extends Mutex? If not, then inheritance is almost certainly the wrong choice because it doesn't make sense to use MyClass as a Mutex (the relationship of MyClass to Mutex is not an "is-a" relationship).

It's also dangerous:

MyClass x;
Lock lock(x);
x.Method();   // uh oh.