Ensure that a thread doesn't lock a mutex twice?

Question

Say I have a thread running a member method like runController in the example below:

class SomeClass { public: SomeClass() { // Start controller thread mControllerThread = std::thread(&SomeClass::runController, this) } ~SomeClass() { // Stop controller thread mIsControllerThreadInterrupted = true; // wait for thread to die. std::unique_lock<std:::mutex> lk(mControllerThreadAlive); } // Both controller and external client threads might call this void modifyObject() { std::unique_lock<std::mutex> lock(mObjectMutex); mObject.doSomeModification(); } //... private: std::mutex mObjectMutex; Object mObject; std::thread mControllerThread; std::atomic<bool> mIsControllerInterrupted; std::mutex mControllerThreadAlive; void runController() { std::unique_lock<std::mutex> aliveLock(mControllerThreadAlive); while(!mIsControllerInterruped) { // Say I need to synchronize on mObject for all of these calls std::unique_lock<std::mutex> lock(mObjectMutex); someMethodA(); modifyObject(); // but calling modifyObject will then lock mutex twice someMethodC(); } } //... };

And some (or all) of the subroutines in runController need to modify data that is shared between threads and guarded by a mutex. Some (or all) of them, might also be called by other threads that need to modify this shared data.

With all the glory of C++11 at my disposal, how can I ensure that no thread ever locks a mutex twice?

Right now, I'm passing unique_lock references into the methods as parameters as below. But this seems clunky, difficult to maintain, potentially disastrous, etc...

void modifyObject(std::unique_lock<std::mutex>& objectLock) { // We don't even know if this lock manages the right mutex... // so let's waste some time checking that. if(objectLock.mutex() != &mObjectMutex) throw std::logic_error(); // Lock mutex if not locked by this thread bool wasObjectLockOwned = objectLock.owns_lock(); if(!wasObjectLockOwned) objectLock.lock(); mObject.doSomeModification(); // restore previous lock state if(!wasObjectLockOwned) objectLock.unlock(); }

Thanks!

I am a little confused. Do you have unchecked try_to_locks? How can a thread lock a mutex and then forget about it? — Red Alert
– Red Alert, Commented Apr 23, 2014 at 3:25
I don't understand the nature of the problem. You lock the mutex right before reading or modifying shared data, and unlock right after. You would normally have an instance of unique_lock on the stack, not passed in as a parameter. You must be doing something quite unorthodox if locking the same mutex twice is an actual risk in your design. Can you show exactly how this may happen for you? — Igor Tandetnik
– Igor Tandetnik, Commented Apr 23, 2014 at 3:43
@RedAlert Ah, perhaps std::try_to_lock is the answer I was looking for? Can I just call that from every function that needs a lock instead of creating a local unique_lock or passing locks around? — mxdubois
– mxdubois, Commented Apr 23, 2014 at 3:44
@IgorTandetnik I updated the question with a more explicit example, but let me know if it's still unclear. — mxdubois
– mxdubois, Commented Apr 23, 2014 at 4:04
@mxdubois no, then you'd lose mutual exclusion with other threads. You need to resolve this at the design level. In your example, a good fix would be to completely remove mutexes from modifyObject(), and instead have only the functions that call it handle the access to the shared resource. — Red Alert
– Red Alert, Commented Apr 23, 2014 at 4:24

nosid · Accepted Answer · 2014-04-23 22:42:55Z

There are several ways to avoid this kind of programming error. I recommend doing it on a class design level:

separate between public and private member functions,
only public member functions lock the mutex,
and public member functions are never called by other member functions.

If a function is needed both internally and externally, create two variants of the function, and delegate from one to the other:

public: // intended to be used from the outside int foobar(int x, int y) { std::unique_lock<std::mutex> lock(mControllerThreadAlive); return _foobar(x, y); } private: // intended to be used from other (public or private) member functions int _foobar(int x, int y) { // ... code that requires locking }

And then there are class private methods that you bind to this and pass along as callbacks. Then you don't know whether that method should be prefixed by an underscore as all the other private methods, or not. It gets messy.

Collectives™ on Stack Overflow

Ensure that a thread doesn't lock a mutex twice?

1 Answer 1

1 Comment

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Linked

Related