4

In the following example, the method foo() gets called, where it acquires ownership of a mutex, and locks it. It then calls check(), which acquires ownership, but assumes that the mutex is already locked, and so simply adopts it using std::adopt_lock.

But when check() finishes, the mutex gets unlocked. So when foo() continues, the section I was trying to guard is actually no longer guarded.

#include <mutex>
static std::mutex sessionLock;

bool check();

void foo() {
  std::lock_guard<std::mutex> guard(sessionLock);
  if (check()) {
    // Do transaction
    // Wait... the mutex is unlocked here!
  }
}

bool check() {
  std::lock_guard<std::mutex> guard(sessionLock, std::adopt_lock);
  // Critical section
  return true;
}

int main() {
  foo();
  return 0;
}

I find this behaviour very unintuitive. If a sub-method decides to take ownership of a lock using std::adopt_lock (ie. it doesn't call lock()), shouldn't it also release ownership without calling unlock()? The standard says otherwise, but I'm curious if this was an oversight or if there is a particular reason this is expected.

This could be rewritten using std::recursive_mutex, though in this case where a regular std::mutex is used, is there a proper way inside check() to ensure its critical section is guarded?

Michael Oliver
  • 1,392
  • 8
  • 22
  • 3
    `adopt_lock` means "hey, I've locked this mutex (e.g., via `std::lock`), make sure that it's unlocked when I'm done". A `lock_guard` that does nothing on construction and nothing on destruction would be pretty useless. – T.C. Dec 09 '15 at 02:08

2 Answers2

1

...though in this case where a regular std::mutex is used, is there a proper way inside check() to ensure its critical section is guarded?

Yes. Use a unique_lock<std::mutex> in foo instead of lock_guard, and pass a const& to that unique_lock as an argument to check so that it can validate the proper mutex is held:

bool check(const std::unique_lock<std::mutex>& guard) {
  assert(guard.owns_lock());             // guard holds *some* mutex...
  assert(guard.mutex() == &sessionLock); // ...it is in fact sessionLock
  // Critical section
  return true;
}

void foo() {
  std::unique_lock<std::mutex> guard(sessionLock);
  if (check(guard)) {
    // Do transaction - guard is still locked.
  }
}
Casey
  • 41,449
  • 7
  • 95
  • 125
0

I know this Question is outdated but you can also use a std::recursive_mutex that allows multiple level of lock acquisition by the same thread...So every time it gets locked ,it adds a new level of hierarchial ownership and unlocking it decreases the level.:)