Fix definition of SynchronizedBase::RLockedPtr

Summary:
The definition of RLockedPtr should use a non-const Subclass type, and not a
const one.

Reviewed By: ot

Differential Revision: D15356827

fbshipit-source-id: b8ad41e263f0e15ffa25b0698aa85eab8ca2ccb8

folly/Synchronized.h

@@ -87,7 +87,7 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> {
   using ConstWLockedPtr =
       ::folly::LockedPtr<const Subclass, LockPolicyExclusive>;
 
-  using RLockedPtr = ::folly::LockedPtr<const Subclass, LockPolicyShared>;
+  using RLockedPtr = ::folly::LockedPtr<Subclass, LockPolicyShared>;
   using ConstRLockedPtr = ::folly::LockedPtr<const Subclass, LockPolicyShared>;
 
   using TryWLockedPtr = ::folly::LockedPtr<Subclass, LockPolicyTryExclusive>;
@@ -181,9 +181,13 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> {
    * validity.)
    */
   template <class Rep, class Period>
-  ConstLockedPtr rlock(
+  RLockedPtr rlock(const std::chrono::duration<Rep, Period>& timeout) {
+    return RLockedPtr(static_cast<Subclass*>(this), timeout);
+  }
+  template <class Rep, class Period>
+  ConstRLockedPtr rlock(
       const std::chrono::duration<Rep, Period>& timeout) const {
-    return ConstLockedPtr(static_cast<const Subclass*>(this), timeout);
+    return ConstRLockedPtr(static_cast<const Subclass*>(this), timeout);
   }
 
   /**

folly/test/SynchronizedTest.cpp

@@ -361,6 +361,26 @@ TEST_F(SynchronizedLockTest, TestCopyConstructibleValues) {
       std::is_copy_assignable<folly::Synchronized<CopyConstructible>>::value);
 }
 
+namespace {
+class Dummy {
+ public:
+  void foo() {}
+};
+} // namespace
+
+TEST_F(SynchronizedLockTest, ReadLockAsNonConstUnsafe) {
+  {
+    folly::Synchronized<Dummy> sync;
+    auto rlock = sync.rlock();
+    rlock.asNonConstUnsafe().foo();
+  }
+  {
+    folly::Synchronized<Dummy> sync;
+    auto rlock = sync.rlock(std::chrono::seconds{1});
+    rlock.asNonConstUnsafe().foo();
+  }
+}
+
 TEST_F(SynchronizedLockTest, UpgradeLocking) {
   folly::Synchronized<int, FakeAllPowerfulAssertingMutex> sync;
 

folly/test/SynchronizedTestLib-inl.h

@@ -285,11 +285,15 @@ testWithLock() {
     EXPECT_EQ(1006, lockedObj->size());
     EXPECT_EQ(16, lockedObj->back());
   });
-  obj.withRLockPtr([](typename SynchType::ConstLockedPtr&& lockedObj) {
+  obj.withRLockPtr([](typename SynchType::RLockedPtr&& lockedObj) {
+    EXPECT_TRUE(
+        (std::is_const<std::remove_reference_t<decltype(*lockedObj)>>{}));
     EXPECT_EQ(1006, lockedObj->size());
     EXPECT_EQ(16, lockedObj->back());
   });
-  constObj.withRLockPtr([](typename SynchType::ConstLockedPtr&& lockedObj) {
+  constObj.withRLockPtr([](typename SynchType::ConstRLockedPtr&& lockedObj) {
+    EXPECT_TRUE(
+        (std::is_const<std::remove_reference_t<decltype(*lockedObj)>>{}));
     EXPECT_EQ(1006, lockedObj->size());
     EXPECT_EQ(16, lockedObj->back());
   });