Is this usage of condition variable wrong?

Question

In the chapter 6 of C++ Concurrency in Action, it implements a thread-safe queue. Here is the complete code. But I find there may be something wrong with its use of condition variable.

  std::unique_lock<std::mutex> wait_for_data()
  {
    std::unique_lock<std::mutex> head_lock(head_mutex);
    data_cond.wait(head_lock, [&] {return head.get() != queue::get_tail(); });
    return std::move(head_lock);
  }

  void push(T new_value)
  {
    std::shared_ptr<T> new_data(
      std::make_shared<T>(std::move(new_value)));
    std::unique_ptr<node> p(new node);
    {
      std::lock_guard<std::mutex> tail_lock(tail_mutex);
      tail->data = new_data;
      node *const new_tail = p.get();
      tail->next = std::move(p);
      tail = new_tail;
    }
    data_cond.notify_one();
  }

The consuming part locks head_mutex, but the producing part locks tail_mutex, possibly causing the consumer to miss notifications. Am I rihgt?

Dacoity

The get_tail() member function locks tail_mutex – Garrott

The implementation is confusing because of the use of two mutexes, where seemingly one will do. Calling notify_one like this is fine. The worst case is a redundant wakeup of a thread waiting on the head_mutex. For the life of me though, I cannot see the benefit of this implementation's use of 2 mutexes. They both get locked during a consume and one is locked during a store. So it seems to me that it would be more efficient to have only one mutex. – Ethban

Isn’t that redundant wakeup a problem? Wait_for_data() doesn’t notice and retry the spurious wakeup, so the caller gets a failure. Or does it? This seems to follow Thompson’s critique of C++ of "a lot of things half well”... – Delhi