Fixing mutex to not use unique_lock

port/win/port_win.cc

@@ -43,53 +43,31 @@ void gettimeofday(struct timeval* tv, struct timezone* /* tz */) {
   tv->tv_usec = usNow.count() - duration_cast<microseconds>(secNow).count();
 }
 
-Mutex::Mutex(bool adaptive) : lock(m_mutex, std::defer_lock) {}
-
 Mutex::~Mutex() {}
 
-void Mutex::Lock() {
-  lock.lock();
-#ifndef NDEBUG
-  locked_ = true;
-#endif
-}
-
-void Mutex::Unlock() {
-#ifndef NDEBUG
-  locked_ = false;
-#endif
-  lock.unlock();
-}
-
-void Mutex::AssertHeld() {
-#ifndef NDEBUG
-  assert(locked_);
-#endif
-}
-
-CondVar::CondVar(Mutex* mu) : mu_(mu) {}
-
 CondVar::~CondVar() {}
 
 void CondVar::Wait() {
+  // Caller must ensure that mutex is held prior to calling this method
+  std::unique_lock<std::mutex> lk(mu_->getLock(), std::adopt_lock);
 #ifndef NDEBUG
   mu_->locked_ = false;
 #endif
-  cv_.wait(mu_->getLock());
+  cv_.wait(lk);
 #ifndef NDEBUG
   mu_->locked_ = true;
 #endif
+  // Release ownership of the lock as we don't want it to be unlocked when
+  // it goes out of scope (as we adopted the lock and didn't lock it ourselves)
+  lk.release();
 }
 
 bool CondVar::TimedWait(uint64_t abs_time_us) {
-#ifndef NDEBUG
-  mu_->locked_ = false;
-#endif
 
   using namespace std::chrono;
 
   // MSVC++ library implements wait_until in terms of wait_for so
-  // there is not an absolute wait anyway.
+  // we need to convert absoulte wait into relative wait.
   microseconds usAbsTime(abs_time_us);
 
   microseconds usNow(
@@ -97,11 +75,18 @@ bool CondVar::TimedWait(uint64_t abs_time_us) {
   microseconds relTimeUs =
     (usAbsTime > usNow) ? (usAbsTime - usNow) : microseconds::zero();
 
-  std::cv_status cvStatus = cv_.wait_for(mu_->getLock(), relTimeUs);
-
+  // Caller must ensure that mutex is held prior to calling this method
+  std::unique_lock<std::mutex> lk(mu_->getLock(), std::adopt_lock);
+#ifndef NDEBUG
+  mu_->locked_ = false;
+#endif
+  std::cv_status cvStatus = cv_.wait_for(lk, relTimeUs);
 #ifndef NDEBUG
   mu_->locked_ = true;
 #endif
+  // Release ownership of the lock as we don't want it to be unlocked when
+  // it goes out of scope (as we adopted the lock and didn't lock it ourselves)
+  lk.release();
 
   if (cvStatus == std::cv_status::timeout) {
     return true;

port/win/port_win.h

@@ -113,29 +113,50 @@ class CondVar;
 
 class Mutex {
  public:
-  /* implicit */ Mutex(bool adaptive = false);
+
+   /* implicit */ Mutex(bool adaptive = false) : locked_(false) {
+  }
+
   ~Mutex();
 
-  void Lock();
-  void Unlock();
+  void Lock() {
+    mutex_.lock();
+#ifndef NDEBUG
+    locked_ = true;
+#endif
+  }
+
+  void Unlock() {
+#ifndef NDEBUG
+    locked_ = false;
+#endif
+    mutex_.unlock();
+  }
 
   // this will assert if the mutex is not locked
   // it does NOT verify that mutex is held by a calling thread
-  void AssertHeld();
+  void AssertHeld() {
+#ifndef NDEBUG
+    assert(locked_);
+#endif
+  }
 
-  std::unique_lock<std::mutex>& getLock() { return lock; }
+  // Mutex is move only with lock ownership transfer
+  Mutex(const Mutex&) = delete;
+  void operator=(const Mutex&) = delete;
 
  private:
+
   friend class CondVar;
-  std::mutex m_mutex;
-  std::unique_lock<std::mutex> lock;
+
+  std::mutex& getLock() {
+    return mutex_;
+  }
+
+  std::mutex mutex_;
 #ifndef NDEBUG
   bool locked_;
 #endif
-
-  // No copying
-  Mutex(const Mutex&);
-  void operator=(const Mutex&);
 };
 
 class RWMutex {
@@ -162,13 +183,22 @@ class RWMutex {
 
 class CondVar {
  public:
-  explicit CondVar(Mutex* mu);
+  explicit CondVar(Mutex* mu) : mu_(mu) {
+  }
+
   ~CondVar();
   void Wait();
   bool TimedWait(uint64_t expiration_time);
   void Signal();
   void SignalAll();
 
+  // Condition var is not copy/move constructible
+  CondVar(const CondVar&) = delete;
+  CondVar& operator=(const CondVar&) = delete;
+
+  CondVar(CondVar&&) = delete;
+  CondVar& operator=(CondVar&&) = delete;
+
  private:
   std::condition_variable cv_;
   Mutex* mu_;