Implement shared_mutex re: N4508

[mclow.lists]

Summary

N45088 adds shared_mutex to the set of mutex classes. This is exactly the same as shared_timed_mutex, without the timed bits.

Create a new class __shared_mutex_base with all the non-timed bits in it, and make shared_timed_mutex and shared_mutex use it. Don't inherit from it, because ABI break. Also, leave some stubs in the dylib for the functions that were renamed, because ABI break.

Duplicate the shared_timed_mutex tests, strip out the timed bits, and rename them to use shared_mutex

[EricWF]

Why not just implement shared_mutex in terms of shared_timed_mutex instead of factoring out common functionality. I agree that this version is better code but it has a larger ABI impact.

include/shared_mutex

175

Why > 14 and not >= 17? I don't have a preference, just a question.

196

Why are these commented out/not getting committed?

[mclow.lists]

Why not just implement shared_mutex in terms of shared_timed_mutex instead of factoring out common functionality?

I'd rather do it the other way, since shared_timed_mutex is a superset of the functionality of shared_mutex. But I can't, due to ABI concerns.

include/shared_mutex

175

Because we don't know that the next version of the standard will be "17". It could slip to 2018 (say). All you know is that it is post-c++14. FWIW, if you specify -std=c++1z in clang, you get a _LIBCPP_STD_VER that is "15".

196

In the standard, they are optional.

[EricWF]

Can apple take this patch since it adds new symbols to the dylib?

include/shared_mutex

196

Why comment them out though? Either we should ship them or remove them.

[mclow.lists]

Can apple take this patch since it adds new symbols to the dylib?

I believe that they're OK with that; I'll give them a heads-up.

include/shared_mutex

196

I'm fine with that.

[EricWF]

LGTM modulo the concerns with Apple. It seems to me that this could be implemented with no ABI changes if need be, but this change looks good.

[mclow.lists]

I talked to Doug last night; they haven't shipped a dylib with shared_mutex in it yet, so they have no ABI concerns here.

[mclow.lists]

Committed as r241067

[theraven]

I missed this when it went in and coming across the code now I'm quite surprised that it did. Why is shared_mutex not implemented as a wrapper around rwlocks (pthreads and Windows both provide this abstraction)? The current implementation looks a lot less efficient than the system version on any operating system that I'm familiar with and has the added effect that native_handle isn't possible to implement, meaning that these can't easily interoperate with C APIs either.

I don't see a way of fixing this without an ABI break, unfortunately. Perhaps we can provide a better implementation in the __v2 namespace and let users (and platforms that don't care about binary compatibility with older libc++) opt in?

[kjcamann]

I have recently benchmarked this (platform was RHEL 7 and Haswell), and the libc++ version has about twice the latency of a pthread_rwlock for both classes of locking. libc++ is also about twice the latency of the libstdc++ implementation, because it is just a lightweight wrapper around pthread_rwlocks.

include/shared_mutex

 namespace std
 {
 
+class shared_mutex      // C++17
+{
+public:
+    shared_mutex();
+    ~shared_mutex();
+
+    shared_mutex(const shared_mutex&) = delete;
+    shared_mutex& operator=(const shared_mutex&) = delete;
+
+    // Exclusive ownership
+    void lock(); // blocking
+    bool try_lock();
+    void unlock();
+
+    // Shared ownership
+    void lock_shared(); // blocking
+    bool try_lock_shared();
+    void unlock_shared();
+
+    typedef implementation-defined native_handle_type; // See 30.2.3
+    native_handle_type native_handle(); // See 30.2.3
+};
+
 class shared_timed_mutex
 {
 public:

 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-class _LIBCPP_TYPE_VIS shared_timed_mutex
+struct _LIBCPP_TYPE_VIS __shared_mutex_base
 {
     mutex               __mut_;
     condition_variable  __gate1_;

 
     static const unsigned __write_entered_ = 1U << (sizeof(unsigned)*__CHAR_BIT__ - 1);
     static const unsigned __n_readers_ = ~__write_entered_;
+
+    __shared_mutex_base();
+    _LIBCPP_INLINE_VISIBILITY ~__shared_mutex_base() = default;
+
+    __shared_mutex_base(const __shared_mutex_base&) = delete;
+    __shared_mutex_base& operator=(const __shared_mutex_base&) = delete;
+
+    // Exclusive ownership
+    void lock(); // blocking
+    bool try_lock();
+    void unlock();
+
+    // Shared ownership
+    void lock_shared(); // blocking
+    bool try_lock_shared();
+    void unlock_shared();
+
+//     typedef implementation-defined native_handle_type; // See 30.2.3
+//     native_handle_type native_handle(); // See 30.2.3
+};
+
+
+#if _LIBCPP_STD_VER > 14
+class _LIBCPP_TYPE_VIS shared_mutex
+{
+  __shared_mutex_base __base;
 public:
+    shared_mutex() : __base() {}
+    _LIBCPP_INLINE_VISIBILITY ~shared_mutex() = default;
+
+    shared_mutex(const shared_mutex&) = delete;
+    shared_mutex& operator=(const shared_mutex&) = delete;
+
+    // Exclusive ownership
+    _LIBCPP_INLINE_VISIBILITY void lock()     { return __base.lock(); }
+    _LIBCPP_INLINE_VISIBILITY bool try_lock() { return __base.try_lock(); }
+    _LIBCPP_INLINE_VISIBILITY void unlock()   { return __base.unlock(); }
+
+    // Shared ownership
+    _LIBCPP_INLINE_VISIBILITY void lock_shared()     { return __base.lock_shared(); }
+    _LIBCPP_INLINE_VISIBILITY bool try_lock_shared() { return __base.try_lock_shared(); }
+    _LIBCPP_INLINE_VISIBILITY void unlock_shared()   { return __base.unlock_shared(); }
+
+//     typedef __shared_mutex_base::native_handle_type native_handle_type;
+//     _LIBCPP_INLINE_VISIBILITY native_handle_type native_handle() { return __base::unlock_shared(); }
+};
+#endif
+
+
+class _LIBCPP_TYPE_VIS shared_timed_mutex
+{
+  __shared_mutex_base __base;
+public:
     shared_timed_mutex();
     _LIBCPP_INLINE_VISIBILITY ~shared_timed_mutex() = default;
 

 shared_timed_mutex::try_lock_until(
                         const chrono::time_point<_Clock, _Duration>& __abs_time)
 {
-    unique_lock<mutex> __lk(__mut_);
-    if (__state_ & __write_entered_)
+    unique_lock<mutex> __lk(__base.__mut_);
+    if (__base.__state_ & __base.__write_entered_)
     {
         while (true)
         {
-            cv_status __status = __gate1_.wait_until(__lk, __abs_time);
-            if ((__state_ & __write_entered_) == 0)
+            cv_status __status = __base.__gate1_.wait_until(__lk, __abs_time);
+            if ((__base.__state_ & __base.__write_entered_) == 0)
                 break;
             if (__status == cv_status::timeout)
                 return false;
         }
     }
-    __state_ |= __write_entered_;
-    if (__state_ & __n_readers_)
+    __base.__state_ |= __base.__write_entered_;
+    if (__base.__state_ & __base.__n_readers_)
     {
         while (true)
         {
-            cv_status __status = __gate2_.wait_until(__lk, __abs_time);
-            if ((__state_ & __n_readers_) == 0)
+            cv_status __status = __base.__gate2_.wait_until(__lk, __abs_time);
+            if ((__base.__state_ & __base.__n_readers_) == 0)
                 break;
             if (__status == cv_status::timeout)
             {
-                __state_ &= ~__write_entered_;
-                __gate1_.notify_all();
+                __base.__state_ &= ~__base.__write_entered_;
+                __base.__gate1_.notify_all();
                 return false;
             }
         }

 shared_timed_mutex::try_lock_shared_until(
                         const chrono::time_point<_Clock, _Duration>& __abs_time)
 {
-    unique_lock<mutex> __lk(__mut_);
-    if ((__state_ & __write_entered_) || (__state_ & __n_readers_) == __n_readers_)
+    unique_lock<mutex> __lk(__base.__mut_);
+    if ((__base.__state_ & __base.__write_entered_) || (__base.__state_ & __base.__n_readers_) == __base.__n_readers_)
     {
         while (true)
         {
-            cv_status status = __gate1_.wait_until(__lk, __abs_time);
-            if ((__state_ & __write_entered_) == 0 &&
-                                       (__state_ & __n_readers_) < __n_readers_)
+            cv_status status = __base.__gate1_.wait_until(__lk, __abs_time);
+            if ((__base.__state_ & __base.__write_entered_) == 0 &&
+                                       (__base.__state_ & __base.__n_readers_) < __base.__n_readers_)
                 break;
             if (status == cv_status::timeout)
                 return false;
         }
     }
-    unsigned __num_readers = (__state_ & __n_readers_) + 1;
-    __state_ &= ~__n_readers_;
-    __state_ |= __num_readers;
+    unsigned __num_readers = (__base.__state_ & __base.__n_readers_) + 1;
+    __base.__state_ &= ~__base.__n_readers_;
+    __base.__state_ |= __num_readers;
     return true;
 }
 

src/shared_mutex.cpp

 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-shared_timed_mutex::shared_timed_mutex()
+// Shared Mutex Base
+__shared_mutex_base::__shared_mutex_base()
     : __state_(0)
 {
 }

 // Exclusive ownership
 
 void
-shared_timed_mutex::lock()
+__shared_mutex_base::lock()
 {
     unique_lock<mutex> lk(__mut_);
     while (__state_ & __write_entered_)

 }
 
 bool
-shared_timed_mutex::try_lock()
+__shared_mutex_base::try_lock()
 {
     unique_lock<mutex> lk(__mut_);
     if (__state_ == 0)

 }
 
 void
-shared_timed_mutex::unlock()
+__shared_mutex_base::unlock()
 {
     lock_guard<mutex> _(__mut_);
     __state_ = 0;

 // Shared ownership
 
 void
-shared_timed_mutex::lock_shared()
+__shared_mutex_base::lock_shared()
 {
     unique_lock<mutex> lk(__mut_);
     while ((__state_ & __write_entered_) || (__state_ & __n_readers_) == __n_readers_)

 }
 
 bool
-shared_timed_mutex::try_lock_shared()
+__shared_mutex_base::try_lock_shared()
 {
     unique_lock<mutex> lk(__mut_);
     unsigned num_readers = __state_ & __n_readers_;

 }
 
 void
-shared_timed_mutex::unlock_shared()
+__shared_mutex_base::unlock_shared()
 {
     lock_guard<mutex> _(__mut_);
     unsigned num_readers = (__state_ & __n_readers_) - 1;

 }
 
 
+// Shared Timed Mutex
+// These routines are here for ABI stability
+shared_timed_mutex::shared_timed_mutex() : __base() {}
+void shared_timed_mutex::lock()     { return __base.lock(); }
+bool shared_timed_mutex::try_lock() { return __base.try_lock(); }
+void shared_timed_mutex::unlock()   { return __base.unlock(); }
+void shared_timed_mutex::lock_shared() { return __base.lock_shared(); }
+bool shared_timed_mutex::try_lock_shared() { return __base.try_lock_shared(); }
+void shared_timed_mutex::unlock_shared() { return __base.unlock_shared(); }
+
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // !_LIBCPP_HAS_NO_THREADS

test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/nothing_to_do.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+int main()
+{
+}

test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.fail.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// <shared_mutex>
+
+// class shared_mutex;
+
+// shared_mutex& operator=(const shared_mutex&) = delete;
+
+#include <shared_mutex>
+
+#include "test_macros.h"
+
+int main()
+{
+#if TEST_STD_VER > 14
+    std::shared_mutex m0;
+    std::shared_mutex m1;
+    m1 = m0;
+#else
+#   error
+#endif
+}

test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/copy.fail.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// <shared_mutex>
+
+// class shared_mutex;
+
+// shared_mutex(const shared_mutex&) = delete;
+
+#include <shared_mutex>
+
+#include "test_macros.h"
+
+int main()
+{
+#if TEST_STD_VER > 14
+    std::shared_mutex m0;
+    std::shared_mutex m1(m0);
+#else
+#   error
+#endif
+}

test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/default.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// UNSUPPORTED: libcpp-has-no-threads
+// UNSUPPORTED: c++03, c++98, c++11, c++14
+
+// <shared_mutex>
+
+// class shared_mutex;
+
+// shared_mutex();
+
+#include <shared_mutex>
+
+int main()
+{
+    std::shared_mutex m;
+}

test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// UNSUPPORTED: libcpp-has-no-threads
+// UNSUPPORTED: c++03, c++98, c++11, c++14
+
+// <shared_mutex>
+
+// class shared_mutex;
+
+// void lock();
+
+#include <shared_mutex>
+#include <thread>
+#include <cstdlib>
+#include <cassert>
+
+
+std::shared_mutex m;
+
+typedef std::chrono::system_clock Clock;
+typedef Clock::time_point time_point;
+typedef Clock::duration duration;
+typedef std::chrono::milliseconds ms;
+typedef std::chrono::nanoseconds ns;
+
+void f()
+{
+    time_point t0 = Clock::now();
+    m.lock();
+    time_point t1 = Clock::now();
+    m.unlock();
+    ns d = t1 - t0 - ms(250);
+    assert(d < ms(50));  // within 50ms
+}
+
+int main()
+{
+    m.lock();
+    std::thread t(f);
+    std::this_thread::sleep_for(ms(250));
+    m.unlock();
+    t.join();
+}

test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock_shared.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// UNSUPPORTED: libcpp-has-no-threads
+// UNSUPPORTED: c++03, c++98, c++11, c++14
+
+// <shared_mutex>
+
+// class shared_mutex;
+
+// void lock_shared();
+
+#include <shared_mutex>
+#include <thread>
+#include <vector>
+#include <cstdlib>
+#include <cassert>
+
+std::shared_mutex m;
+
+typedef std::chrono::system_clock Clock;
+typedef Clock::time_point time_point;
+typedef Clock::duration duration;
+typedef std::chrono::milliseconds ms;
+typedef std::chrono::nanoseconds ns;
+
+void f()
+{
+    time_point t0 = Clock::now();
+    m.lock_shared();
+    time_point t1 = Clock::now();
+    m.unlock_shared();
+    ns d = t1 - t0 - ms(250);
+    assert(d < ms(50));  // within 50ms
+}
+
+void g()
+{
+    time_point t0 = Clock::now();
+    m.lock_shared();
+    time_point t1 = Clock::now();
+    m.unlock_shared();
+    ns d = t1 - t0;
+    assert(d < ms(50));  // within 50ms
+}
+
+
+int main()
+{
+    m.lock();
+    std::vector<std::thread> v;
+    for (int i = 0; i < 5; ++i)
+        v.push_back(std::thread(f));
+    std::this_thread::sleep_for(ms(250));
+    m.unlock();
+    for (auto& t : v)
+        t.join();
+    m.lock_shared();
+    for (auto& t : v)
+        t = std::thread(g);
+    std::thread q(f);
+    std::this_thread::sleep_for(ms(250));
+    m.unlock_shared();
+    for (auto& t : v)
+        t.join();
+    q.join();
+}

test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// UNSUPPORTED: libcpp-has-no-threads
+// UNSUPPORTED: c++03, c++98, c++11, c++14
+
+// <shared_mutex>
+
+// class shared_mutex;
+
+// bool try_lock();
+
+#include <shared_mutex>
+#include <thread>
+#include <cstdlib>
+#include <cassert>
+
+std::shared_mutex m;
+
+typedef std::chrono::system_clock Clock;
+typedef Clock::time_point time_point;
+typedef Clock::duration duration;
+typedef std::chrono::milliseconds ms;
+typedef std::chrono::nanoseconds ns;
+
+void f()
+{
+    time_point t0 = Clock::now();
+    assert(!m.try_lock());
+    assert(!m.try_lock());
+    assert(!m.try_lock());
+    while(!m.try_lock())
+        ;
+    time_point t1 = Clock::now();
+    m.unlock();
+    ns d = t1 - t0 - ms(250);
+    assert(d < ms(200));  // within 200ms
+}
+
+int main()
+{
+    m.lock();
+    std::thread t(f);
+    std::this_thread::sleep_for(ms(250));
+    m.unlock();
+    t.join();
+}

test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock_shared.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// UNSUPPORTED: libcpp-has-no-threads
+// UNSUPPORTED: c++03, c++98, c++11, c++14
+
+// <shared_mutex>
+
+// class shared_mutex;
+
+// bool try_lock_shared();
+
+#include <shared_mutex>
+#include <thread>
+#include <vector>
+#include <cstdlib>
+#include <cassert>
+
+std::shared_mutex m;
+
+typedef std::chrono::system_clock Clock;
+typedef Clock::time_point time_point;
+typedef Clock::duration duration;
+typedef std::chrono::milliseconds ms;
+typedef std::chrono::nanoseconds ns;
+
+void f()
+{
+    time_point t0 = Clock::now();
+    assert(!m.try_lock_shared());
+    assert(!m.try_lock_shared());
+    assert(!m.try_lock_shared());
+    while(!m.try_lock_shared())
+        ;
+    time_point t1 = Clock::now();
+    m.unlock_shared();
+    ns d = t1 - t0 - ms(250);
+    assert(d < ms(200));  // within 200ms
+}
+
+
+int main()
+{
+    m.lock();
+    std::vector<std::thread> v;
+    for (int i = 0; i < 5; ++i)
+        v.push_back(std::thread(f));
+    std::this_thread::sleep_for(ms(250));
+    m.unlock();
+    for (auto& t : v)
+        t.join();
+}