From: Dan Melnic <dmm@fb.com>
Date: Mon, 3 Apr 2017 21:09:47 +0000 (-0700)
Subject: Remove/make private the default ***Holder constructor to allow compile time detection... 
X-Git-Tag: v2017.04.10.00~25
X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=a130b23ab3fc1c0895c29f3d73597ea465f20bc3

Remove/make private the default ***Holder constructor to allow compile time detection of ***Holder(mutexPtr_) constructs

Summary: Avoid unintended C++ stuff - this is not the same as the "fleeting rvalue" - this change might break the build but I will fix it

Reviewed By: yfeldblum

Differential Revision: D4814104

fbshipit-source-id: 058a0eac44893c573062fcf5665d4fd022ee64a0
---

diff --git a/folly/RWSpinLock.h b/folly/RWSpinLock.h
index 63dac2f5..cc4f480a 100644
--- a/folly/RWSpinLock.h
+++ b/folly/RWSpinLock.h
@@ -307,7 +307,7 @@ class RWSpinLock {
 
   class ReadHolder {
    public:
-    explicit ReadHolder(RWSpinLock* lock = nullptr) : lock_(lock) {
+    explicit ReadHolder(RWSpinLock* lock) : lock_(lock) {
       if (lock_) lock_->lock_shared();
     }
 
@@ -360,7 +360,7 @@ class RWSpinLock {
 
   class UpgradedHolder {
    public:
-    explicit UpgradedHolder(RWSpinLock* lock = nullptr) : lock_(lock) {
+    explicit UpgradedHolder(RWSpinLock* lock) : lock_(lock) {
       if (lock_) lock_->lock_upgrade();
     }
 
@@ -409,7 +409,7 @@ class RWSpinLock {
 
   class WriteHolder {
    public:
-    explicit WriteHolder(RWSpinLock* lock = nullptr) : lock_(lock) {
+    explicit WriteHolder(RWSpinLock* lock) : lock_(lock) {
       if (lock_) lock_->lock();
     }
 
@@ -692,8 +692,7 @@ class RWTicketSpinLockT {
     ReadHolder(ReadHolder const&) = delete;
     ReadHolder& operator=(ReadHolder const&) = delete;
 
-    explicit ReadHolder(RWSpinLock *lock = nullptr) :
-      lock_(lock) {
+    explicit ReadHolder(RWSpinLock* lock) : lock_(lock) {
       if (lock_) lock_->lock_shared();
     }
 
@@ -732,7 +731,7 @@ class RWTicketSpinLockT {
     WriteHolder(WriteHolder const&) = delete;
     WriteHolder& operator=(WriteHolder const&) = delete;
 
-    explicit WriteHolder(RWSpinLock *lock = nullptr) : lock_(lock) {
+    explicit WriteHolder(RWSpinLock* lock) : lock_(lock) {
       if (lock_) lock_->lock();
     }
     explicit WriteHolder(RWSpinLock &lock) : lock_ (&lock) {
diff --git a/folly/SharedMutex.h b/folly/SharedMutex.h
index 49dd29c6..571af4a7 100644
--- a/folly/SharedMutex.h
+++ b/folly/SharedMutex.h
@@ -1133,10 +1133,15 @@ class SharedMutexImpl {
 
  public:
   class ReadHolder {
-   public:
     ReadHolder() : lock_(nullptr) {}
 
-    explicit ReadHolder(const SharedMutexImpl* lock) : ReadHolder(*lock) {}
+   public:
+    explicit ReadHolder(const SharedMutexImpl* lock)
+        : lock_(const_cast<SharedMutexImpl*>(lock)) {
+      if (lock_) {
+        lock_->lock_shared(token_);
+      }
+    }
 
     explicit ReadHolder(const SharedMutexImpl& lock)
         : lock_(const_cast<SharedMutexImpl*>(&lock)) {
@@ -1190,10 +1195,14 @@ class SharedMutexImpl {
   };
 
   class UpgradeHolder {
-   public:
     UpgradeHolder() : lock_(nullptr) {}
 
-    explicit UpgradeHolder(SharedMutexImpl* lock) : UpgradeHolder(*lock) {}
+   public:
+    explicit UpgradeHolder(SharedMutexImpl* lock) : lock_(lock) {
+      if (lock_) {
+        lock_->lock_upgrade();
+      }
+    }
 
     explicit UpgradeHolder(SharedMutexImpl& lock) : lock_(&lock) {
       lock_->lock_upgrade();
@@ -1236,10 +1245,14 @@ class SharedMutexImpl {
   };
 
   class WriteHolder {
-   public:
     WriteHolder() : lock_(nullptr) {}
 
-    explicit WriteHolder(SharedMutexImpl* lock) : WriteHolder(*lock) {}
+   public:
+    explicit WriteHolder(SharedMutexImpl* lock) : lock_(lock) {
+      if (lock_) {
+        lock_->lock();
+      }
+    }
 
     explicit WriteHolder(SharedMutexImpl& lock) : lock_(&lock) {
       lock_->lock();
diff --git a/folly/detail/ThreadLocalDetail.cpp b/folly/detail/ThreadLocalDetail.cpp
index 12a02e46..67eb0565 100644
--- a/folly/detail/ThreadLocalDetail.cpp
+++ b/folly/detail/ThreadLocalDetail.cpp
@@ -45,7 +45,7 @@ void StaticMetaBase::onThreadExit(void* ptr) {
   };
 
   {
-    SharedMutex::ReadHolder rlock;
+    SharedMutex::ReadHolder rlock(nullptr);
     if (meta.strict_) {
       rlock = SharedMutex::ReadHolder(meta.accessAllThreadsLock_);
     }
@@ -103,7 +103,7 @@ void StaticMetaBase::destroy(EntryID* ent) {
     std::vector<ElementWrapper> elements;
 
     {
-      SharedMutex::WriteHolder wlock;
+      SharedMutex::WriteHolder wlock(nullptr);
       if (meta.strict_) {
         /*
          * In strict mode, the logic guarantees per-thread instances are
diff --git a/folly/test/SharedMutexTest.cpp b/folly/test/SharedMutexTest.cpp
index d18e3b6e..9e04edc7 100644
--- a/folly/test/SharedMutexTest.cpp
+++ b/folly/test/SharedMutexTest.cpp
@@ -98,7 +98,7 @@ void runBasicHoldersTest() {
     EXPECT_FALSE(lock.try_lock_shared(token));
 
     // move ownership to another write holder via assign operator
-    typename Lock::WriteHolder holder3;
+    typename Lock::WriteHolder holder3(nullptr);
     holder3 = std::move(holder2);
     EXPECT_FALSE(lock.try_lock());
     EXPECT_FALSE(lock.try_lock_shared(token));