1 files changed, 47 insertions, 30 deletions

diff --git a/absl/synchronization/internal/waiter.cc b/absl/synchronization/internal/waiter.cc
index 2f41aa5f2b44..8e7137705325 100644
--- a/absl/synchronization/internal/waiter.cc
+++ b/absl/synchronization/internal/waiter.cc
@@ -122,10 +122,12 @@ class Futex {
   }
 };
 
-void Waiter::Init() {
+Waiter::Waiter() {
   futex_.store(0, std::memory_order_relaxed);
 }
 
+Waiter::~Waiter() = default;
+
 bool Waiter::Wait(KernelTimeout t) {
   // Loop until we can atomically decrement futex from a positive
   // value, waiting on a futex while we believe it is zero.
@@ -199,7 +201,7 @@ class PthreadMutexHolder {
   pthread_mutex_t *mu_;
 };
 
-void Waiter::Init() {
+Waiter::Waiter() {
   const int err = pthread_mutex_init(&mu_, 0);
   if (err != 0) {
     ABSL_RAW_LOG(FATAL, "pthread_mutex_init failed: %d", err);
@@ -214,6 +216,18 @@ void Waiter::Init() {
   wakeup_count_ = 0;
 }
 
+Waiter::~Waiter() {
+  const int err = pthread_mutex_destroy(&mu_);
+  if (err != 0) {
+    ABSL_RAW_LOG(FATAL, "pthread_mutex_destroy failed: %d", err);
+  }
+
+  const int err2 = pthread_cond_destroy(&cv_);
+  if (err2 != 0) {
+    ABSL_RAW_LOG(FATAL, "pthread_cond_destroy failed: %d", err2);
+  }
+}
+
 bool Waiter::Wait(KernelTimeout t) {
   struct timespec abs_timeout;
   if (t.has_timeout()) {
@@ -274,13 +288,19 @@ void Waiter::InternalCondVarPoke() {
 
 #elif ABSL_WAITER_MODE == ABSL_WAITER_MODE_SEM
 
-void Waiter::Init() {
+Waiter::Waiter() {
   if (sem_init(&sem_, 0, 0) != 0) {
     ABSL_RAW_LOG(FATAL, "sem_init failed with errno %d\n", errno);
   }
   wakeups_.store(0, std::memory_order_relaxed);
 }
 
+Waiter::~Waiter() {
+  if (sem_destroy(&sem_) != 0) {
+    ABSL_RAW_LOG(FATAL, "sem_destroy failed with errno %d\n", errno);
+  }
+}
+
 bool Waiter::Wait(KernelTimeout t) {
   struct timespec abs_timeout;
   if (t.has_timeout()) {
@@ -388,39 +408,32 @@ class LockHolder {
   SRWLOCK* mu_;
 };
 
-void Waiter::Init() {
+Waiter::Waiter() {
   auto *mu = ::new (static_cast<void *>(&mu_storage_)) SRWLOCK;
   auto *cv = ::new (static_cast<void *>(&cv_storage_)) CONDITION_VARIABLE;
   InitializeSRWLock(mu);
   InitializeConditionVariable(cv);
-  waiter_count_.store(0, std::memory_order_relaxed);
-  wakeup_count_.store(0, std::memory_order_relaxed);
+  waiter_count_ = 0;
+  wakeup_count_ = 0;
 }
 
+// SRW locks and condition variables do not need to be explicitly destroyed.
+// https://docs.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-initializesrwlock
+// https://stackoverflow.com/questions/28975958/why-does-windows-have-no-deleteconditionvariable-function-to-go-together-with
+Waiter::~Waiter() = default;
+
 bool Waiter::Wait(KernelTimeout t) {
   SRWLOCK *mu = WinHelper::GetLock(this);
   CONDITION_VARIABLE *cv = WinHelper::GetCond(this);
 
   LockHolder h(mu);
-  waiter_count_.fetch_add(1, std::memory_order_relaxed);
+  ++waiter_count_;
 
   // Loop until we find a wakeup to consume or timeout.
   // Note that, since the thread ticker is just reset, we don't need to check
   // whether the thread is idle on the very first pass of the loop.
   bool first_pass = true;
-  while (true) {
-    int x = wakeup_count_.load(std::memory_order_relaxed);
-    if (x != 0) {
-      if (!wakeup_count_.compare_exchange_weak(x, x - 1,
-                                               std::memory_order_acquire,
-                                               std::memory_order_relaxed)) {
-        continue;  // Raced with someone, retry.
-      }
-      // Successfully consumed a wakeup, we're done.
-      waiter_count_.fetch_sub(1, std::memory_order_relaxed);
-      return true;
-    }
-
+  while (wakeup_count_ == 0) {
     if (!first_pass) MaybeBecomeIdle();
     // No wakeups available, time to wait.
     if (!SleepConditionVariableSRW(cv, mu, t.InMillisecondsFromNow(), 0)) {
@@ -429,7 +442,7 @@ bool Waiter::Wait(KernelTimeout t) {
       // initialization guarantees this is not a narrowing conversion.
       const unsigned long err{GetLastError()};  // NOLINT(runtime/int)
       if (err == ERROR_TIMEOUT) {
-        waiter_count_.fetch_sub(1, std::memory_order_relaxed);
+        --waiter_count_;
         return false;
       } else {
         ABSL_RAW_LOG(FATAL, "SleepConditionVariableSRW failed: %lu", err);
@@ -437,23 +450,27 @@ bool Waiter::Wait(KernelTimeout t) {
     }
     first_pass = false;
   }
+  // Consume a wakeup and we're done.
+  --wakeup_count_;
+  --waiter_count_;
+  return true;
 }
 
 void Waiter::Post() {
-  wakeup_count_.fetch_add(1, std::memory_order_release);
-  Poke();
+  LockHolder h(WinHelper::GetLock(this));
+  ++wakeup_count_;
+  InternalCondVarPoke();
 }
 
 void Waiter::Poke() {
-  if (waiter_count_.load(std::memory_order_relaxed) == 0) {
-    return;
-  }
-  // Potentially a waiter. Take the lock and check again.
   LockHolder h(WinHelper::GetLock(this));
-  if (waiter_count_.load(std::memory_order_relaxed) == 0) {
-    return;
+  InternalCondVarPoke();
+}
+
+void Waiter::InternalCondVarPoke() {
+  if (waiter_count_ != 0) {
+    WakeConditionVariable(WinHelper::GetCond(this));
   }
-  WakeConditionVariable(WinHelper::GetCond(this));
 }
 
 #else