diff --git a/port/win/port_win.cc b/port/win/port_win.cc index 2aaeada924..42eff09a16 100644 --- a/port/win/port_win.cc +++ b/port/win/port_win.cc @@ -43,65 +43,50 @@ void gettimeofday(struct timeval* tv, struct timezone* /* tz */) { tv->tv_usec = usNow.count() - duration_cast(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 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( - duration_cast(system_clock::now().time_since_epoch())); + duration_cast(system_clock::now().time_since_epoch())); microseconds relTimeUs = - (usAbsTime > usNow) ? (usAbsTime - usNow) : microseconds::zero(); - - std::cv_status cvStatus = cv_.wait_for(mu_->getLock(), relTimeUs); + (usAbsTime > usNow) ? (usAbsTime - usNow) : microseconds::zero(); + // Caller must ensure that mutex is held prior to calling this method + std::unique_lock 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; diff --git a/port/win/port_win.h b/port/win/port_win.h index 1f517fb784..edee22e7a6 100644 --- a/port/win/port_win.h +++ b/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& 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 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_;