4

I'm hunting down a bug we have with some messy thread/condition variable classes being updated to use C++11 threads. During the course of the hunt, I've come across the following in the GCC codebase:

  template<typename _Lock>
  void
  wait(_Lock& __lock)
  {
    unique_lock<mutex> __my_lock(_M_mutex);
    _Unlock<_Lock> __unlock(__lock);
    // _M_mutex must be unlocked before re-locking __lock so move
    // ownership of _M_mutex lock to an object with shorter lifetime.
    unique_lock<mutex> __my_lock2(std::move(__my_lock));
    _M_cond.wait(__my_lock2);
  }

Despite the comment, I'm having difficulty understanding the purpose of the move constructor here to __my_lock2. Why is __my_lock moved to __my_lock2 here?

Improve this question
1
  • 2
    I suspect it has to do with __my_lock's destructor being called after __unlock's, whereas __my_lock2 will be destroyed first. It's basically hacking around FILO lifetime rules. Commented Mar 4, 2014 at 4:16

3 Answers 3

Reset to default
10

This does not look like condition_variable code. It looks like condition_variable_any code. The latter has a templated wait function. The former does not.

N2406 may shed light on what you are seeing. In N2406, condition_variable_any is instead named gen_cond_var, but otherwise, the types are identical. This paper describes a deadlock scenario whereupon deadlock is achieved unless great care is taken to assure that _M_mutex and __lock are locked and unlocked in a very specific order.

While the code you show, and the code at N2406, is not the same, I strongly suspect that they are both built to lock and unlock these two locks in an order so as to avoid the deadlock described in N2406. Without further insight into the definition of _Unlock, I can not be absolutely certain though. However a strong clue is the final comment in N2406 of the this wait function:

}  // mut_.unlock(), external.lock()

I.e. the member mutex must be unlocked prior to locking the external __lock. By moving __my_lock to a local variable with scope less than that of __unlock, it is highly likely that the correct ordering of 

}  // _M_mutex.unlock(), __lock.lock()

has been achieved. To understand why this ordering is so critically important, you have to read the associated section of N2406 (it prevents deadlock).

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

3

The function (which I believe to be std::condition_variable_any<_Lock>::wait) is avoiding deadlock by establishing a lock ordering invariant that __lock must be locked before acquiring _M_mutex. While doing so, it must somehow still guarantee that __lock is not held while waiting on the internal condition variable _M_cond. Note that _Unlock<_Lock> is an RAII unlocking object. It performs the opposite function of the usual lock guards by unlocking in its constructor and locking in the destructor. The necessary ordering of events is:

  1. acquire __lock (pre-condition of the wait call)
  2. acquire _M_mutex (in the __my_lock constructor)
  3. release __lock (in the __unlock constructor)
  4. atomically release _M_mutex and wait on _M_cond (happens in _M_cond.wait)
  5. reacquire _M_mutex on wakeup (also in _M_cond.wait)
  6. release _M_mutex (in the __my_lock2 destructor)
  7. reacquire __lock (in the __unlock destructor)

The move from __my_lock to __my_lock2 is necessary so that __my_lock2 will be destroyed before __unlock ensuring that event 6 happens before 7 instead of after.

Improve this answer

Comments

1

It looks like it is to reorder the destruction of the __lock and __my_lock variables.

The calls should look like this:

construct __my_lock // locks   _M_mutex
construct __unlock  // unlocks __lock
construct __my_lock2 // Does nothing as its a move.

_M_cond.wait(__my_lock2);

destroy __mylock2 // unlocks __M_mutex
destroy __unlock // locks __lock again
destroy __mylock // does nothing as its been moved

Without the move the order would be

construct __my_lock // locks   _M_mutex
construct __unlock  // unlocks __lock

_M_cond.wait(__my_lock);

destroy __unlock // locks __lock
destroy __mylock // unlocks _M_mutex

Which can result in deadlock as mentioned in the other answer

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.