Don't use pthread_spinlock_t in TimedRWMutex

Summary: `TimedMutex` was already using `folly::SpinLock`, so switch `TimedRWMutex` to do the same.

Reviewed By: andriigrynenko

Differential Revision: D4888005

fbshipit-source-id: 6e782347bc22dc186ed41f2e77c6614b8444bae2

diff --git a/folly/fibers/TimedMutex-inl.h b/folly/fibers/TimedMutex-inl.h
index c11871aff68fb8ae8f4f1902a4d5e879607fea1f..1c64a0ebb0646fcda77413ade6af8d2fca8a791f 100644 (file)
--- a/folly/fibers/TimedMutex-inl.h
+++ b/folly/fibers/TimedMutex-inl.h
@@ -151,11 +151,11 @@ inline void TimedMutex::unlock() {
 
 template <typename BatonType>
 void TimedRWMutex<BatonType>::read_lock() {
 
 template <typename BatonType>
 void TimedRWMutex<BatonType>::read_lock() {

-  pthread_spin_lock(&lock_);
+  lock_.lock();

   if (state_ == State::WRITE_LOCKED) {
     MutexWaiter waiter;
     read_waiters_.push_back(waiter);
   if (state_ == State::WRITE_LOCKED) {
     MutexWaiter waiter;
     read_waiters_.push_back(waiter);

-    pthread_spin_unlock(&lock_);
+    lock_.unlock();

     waiter.baton.wait();
     assert(state_ == State::READ_LOCKED);
     return;
     waiter.baton.wait();
     assert(state_ == State::READ_LOCKED);
     return;


@@ -166,18 +166,18 @@ void TimedRWMutex<BatonType>::read_lock() {
   assert(read_waiters_.empty());
   state_ = State::READ_LOCKED;
   readers_ += 1;
   assert(read_waiters_.empty());
   state_ = State::READ_LOCKED;
   readers_ += 1;

-  pthread_spin_unlock(&lock_);
+  lock_.unlock();

 }
 
 template <typename BatonType>
 template <typename Rep, typename Period>
 bool TimedRWMutex<BatonType>::timed_read_lock(
     const std::chrono::duration<Rep, Period>& duration) {
 }
 
 template <typename BatonType>
 template <typename Rep, typename Period>
 bool TimedRWMutex<BatonType>::timed_read_lock(
     const std::chrono::duration<Rep, Period>& duration) {

-  pthread_spin_lock(&lock_);
+  lock_.lock();

   if (state_ == State::WRITE_LOCKED) {
     MutexWaiter waiter;
     read_waiters_.push_back(waiter);
   if (state_ == State::WRITE_LOCKED) {
     MutexWaiter waiter;
     read_waiters_.push_back(waiter);

-    pthread_spin_unlock(&lock_);
+    lock_.unlock();

 
     if (!waiter.baton.timed_wait(duration)) {
       // We timed out. Two cases:
 
     if (!waiter.baton.timed_wait(duration)) {
       // We timed out. Two cases:


@@ -185,13 +185,13 @@ bool TimedRWMutex<BatonType>::timed_read_lock(
       // 2. We're not in the waiter list anymore. This could happen if the baton
       //    times out but the mutex is unlocked before we reach this code. In
       //    this case we'll pretend we got the lock on time.
       // 2. We're not in the waiter list anymore. This could happen if the baton
       //    times out but the mutex is unlocked before we reach this code. In
       //    this case we'll pretend we got the lock on time.

-      pthread_spin_lock(&lock_);
+      lock_.lock();

       if (waiter.hook.is_linked()) {
         read_waiters_.erase(read_waiters_.iterator_to(waiter));
       if (waiter.hook.is_linked()) {
         read_waiters_.erase(read_waiters_.iterator_to(waiter));

-        pthread_spin_unlock(&lock_);
+        lock_.unlock();

         return false;
       }
         return false;
       }

-      pthread_spin_unlock(&lock_);
+      lock_.unlock();

     }
     return true;
   }
     }
     return true;
   }


@@ -201,13 +201,13 @@ bool TimedRWMutex<BatonType>::timed_read_lock(
   assert(read_waiters_.empty());
   state_ = State::READ_LOCKED;
   readers_ += 1;
   assert(read_waiters_.empty());
   state_ = State::READ_LOCKED;
   readers_ += 1;

-  pthread_spin_unlock(&lock_);
+  lock_.unlock();

   return true;
 }
 
 template <typename BatonType>
 bool TimedRWMutex<BatonType>::try_read_lock() {
   return true;
 }
 
 template <typename BatonType>
 bool TimedRWMutex<BatonType>::try_read_lock() {

-  pthread_spin_lock(&lock_);
+  lock_.lock();

   if (state_ != State::WRITE_LOCKED) {
     assert(
         (state_ == State::UNLOCKED && readers_ == 0) ||
   if (state_ != State::WRITE_LOCKED) {
     assert(
         (state_ == State::UNLOCKED && readers_ == 0) ||


@@ -215,25 +215,25 @@ bool TimedRWMutex<BatonType>::try_read_lock() {
     assert(read_waiters_.empty());
     state_ = State::READ_LOCKED;
     readers_ += 1;
     assert(read_waiters_.empty());
     state_ = State::READ_LOCKED;
     readers_ += 1;

-    pthread_spin_unlock(&lock_);
+    lock_.unlock();

     return true;
   }
     return true;
   }

-  pthread_spin_unlock(&lock_);
+  lock_.unlock();

   return false;
 }
 
 template <typename BatonType>
 void TimedRWMutex<BatonType>::write_lock() {
   return false;
 }
 
 template <typename BatonType>
 void TimedRWMutex<BatonType>::write_lock() {

-  pthread_spin_lock(&lock_);
+  lock_.lock();

   if (state_ == State::UNLOCKED) {
     verify_unlocked_properties();
     state_ = State::WRITE_LOCKED;
   if (state_ == State::UNLOCKED) {
     verify_unlocked_properties();
     state_ = State::WRITE_LOCKED;

-    pthread_spin_unlock(&lock_);
+    lock_.unlock();

     return;
   }
   MutexWaiter waiter;
   write_waiters_.push_back(waiter);
     return;
   }
   MutexWaiter waiter;
   write_waiters_.push_back(waiter);

-  pthread_spin_unlock(&lock_);
+  lock_.unlock();

   waiter.baton.wait();
 }
 
   waiter.baton.wait();
 }
 


@@ -241,16 +241,16 @@ template <typename BatonType>
 template <typename Rep, typename Period>
 bool TimedRWMutex<BatonType>::timed_write_lock(
     const std::chrono::duration<Rep, Period>& duration) {
 template <typename Rep, typename Period>
 bool TimedRWMutex<BatonType>::timed_write_lock(
     const std::chrono::duration<Rep, Period>& duration) {

-  pthread_spin_lock(&lock_);
+  lock_.lock();

   if (state_ == State::UNLOCKED) {
     verify_unlocked_properties();
     state_ = State::WRITE_LOCKED;
   if (state_ == State::UNLOCKED) {
     verify_unlocked_properties();
     state_ = State::WRITE_LOCKED;

-    pthread_spin_unlock(&lock_);
+    lock_.unlock();

     return true;
   }
   MutexWaiter waiter;
   write_waiters_.push_back(waiter);
     return true;
   }
   MutexWaiter waiter;
   write_waiters_.push_back(waiter);

-  pthread_spin_unlock(&lock_);
+  lock_.unlock();

 
   if (!waiter.baton.timed_wait(duration)) {
     // We timed out. Two cases:
 
   if (!waiter.baton.timed_wait(duration)) {
     // We timed out. Two cases:


@@ -258,13 +258,13 @@ bool TimedRWMutex<BatonType>::timed_write_lock(
     // 2. We're not in the waiter list anymore. This could happen if the baton
     //    times out but the mutex is unlocked before we reach this code. In
     //    this case we'll pretend we got the lock on time.
     // 2. We're not in the waiter list anymore. This could happen if the baton
     //    times out but the mutex is unlocked before we reach this code. In
     //    this case we'll pretend we got the lock on time.

-    pthread_spin_lock(&lock_);
+    lock_.lock();

     if (waiter.hook.is_linked()) {
       write_waiters_.erase(write_waiters_.iterator_to(waiter));
     if (waiter.hook.is_linked()) {
       write_waiters_.erase(write_waiters_.iterator_to(waiter));

-      pthread_spin_unlock(&lock_);
+      lock_.unlock();

       return false;
     }
       return false;
     }

-    pthread_spin_unlock(&lock_);
+    lock_.unlock();

   }
   assert(state_ == State::WRITE_LOCKED);
   return true;
   }
   assert(state_ == State::WRITE_LOCKED);
   return true;


@@ -272,20 +272,20 @@ bool TimedRWMutex<BatonType>::timed_write_lock(
 
 template <typename BatonType>
 bool TimedRWMutex<BatonType>::try_write_lock() {
 
 template <typename BatonType>
 bool TimedRWMutex<BatonType>::try_write_lock() {

-  pthread_spin_lock(&lock_);
+  lock_.lock();

   if (state_ == State::UNLOCKED) {
     verify_unlocked_properties();
     state_ = State::WRITE_LOCKED;
   if (state_ == State::UNLOCKED) {
     verify_unlocked_properties();
     state_ = State::WRITE_LOCKED;

-    pthread_spin_unlock(&lock_);
+    lock_.unlock();

     return true;
   }
     return true;
   }

-  pthread_spin_unlock(&lock_);
+  lock_.unlock();

   return false;
 }
 
 template <typename BatonType>
 void TimedRWMutex<BatonType>::unlock() {
   return false;
 }
 
 template <typename BatonType>
 void TimedRWMutex<BatonType>::unlock() {

-  pthread_spin_lock(&lock_);
+  lock_.lock();

   assert(state_ != State::UNLOCKED);
   assert(
       (state_ == State::READ_LOCKED && readers_ > 0) ||
   assert(state_ != State::UNLOCKED);
   assert(
       (state_ == State::READ_LOCKED && readers_ > 0) ||


@@ -322,12 +322,12 @@ void TimedRWMutex<BatonType>::unlock() {
   } else {
     assert(state_ == State::READ_LOCKED);
   }
   } else {
     assert(state_ == State::READ_LOCKED);
   }

-  pthread_spin_unlock(&lock_);
+  lock_.unlock();

 }
 
 template <typename BatonType>
 void TimedRWMutex<BatonType>::downgrade() {
 }
 
 template <typename BatonType>
 void TimedRWMutex<BatonType>::downgrade() {

-  pthread_spin_lock(&lock_);
+  lock_.lock();

   assert(state_ == State::WRITE_LOCKED && readers_ == 0);
   state_ = State::READ_LOCKED;
   readers_ += 1;
   assert(state_ == State::WRITE_LOCKED && readers_ == 0);
   state_ = State::READ_LOCKED;
   readers_ += 1;


@@ -341,7 +341,7 @@ void TimedRWMutex<BatonType>::downgrade() {
       to_wake.baton.post();
     }
   }
       to_wake.baton.post();
     }
   }

-  pthread_spin_unlock(&lock_);
+  lock_.unlock();

 }
 }
 }
 }
 }
 }

diff --git a/folly/fibers/TimedMutex.h b/folly/fibers/TimedMutex.h
index 7c42b8b0397791ca58ea691c86209e4400290d0a..671409a0c39364b1f08da8fb7d69e8d434c78619 100644 (file)
--- a/folly/fibers/TimedMutex.h
+++ b/folly/fibers/TimedMutex.h
@@ -15,8 +15,6 @@
  */
 #pragma once
 
  */
 #pragma once
 

-#include <pthread.h>
-

 #include <folly/IntrusiveList.h>
 #include <folly/SpinLock.h>
 #include <folly/fibers/GenericBaton.h>
 #include <folly/IntrusiveList.h>
 #include <folly/SpinLock.h>
 #include <folly/fibers/GenericBaton.h>


@@ -93,13 +91,8 @@ class TimedMutex {
 template <typename BatonType>
 class TimedRWMutex {
  public:
 template <typename BatonType>
 class TimedRWMutex {
  public:

-  TimedRWMutex() {
-    pthread_spin_init(&lock_, PTHREAD_PROCESS_PRIVATE);
-  }
-
-  ~TimedRWMutex() {
-    pthread_spin_destroy(&lock_);
-  }
+  TimedRWMutex() = default;
+  ~TimedRWMutex() = default;

 
   TimedRWMutex(const TimedRWMutex& rhs) = delete;
   TimedRWMutex& operator=(const TimedRWMutex& rhs) = delete;
 
   TimedRWMutex(const TimedRWMutex& rhs) = delete;
   TimedRWMutex& operator=(const TimedRWMutex& rhs) = delete;


@@ -223,7 +216,7 @@ class TimedRWMutex {
       boost::intrusive::constant_time_size<true>>
       MutexWaiterList;
 
       boost::intrusive::constant_time_size<true>>
       MutexWaiterList;
 

-  pthread_spinlock_t lock_; //< lock protecting the internal state
+  folly::SpinLock lock_; //< lock protecting the internal state

   // (state_, read_waiters_, etc.)
   State state_ = State::UNLOCKED;
 
   // (state_, read_waiters_, etc.)
   State state_ = State::UNLOCKED;