threading_support: introduce __libcpp_recursive_mutex_t

[compnerd]

Summary

Introduce the new __libcpp_recursive_mutex_t which differentiates
between a recursive and non-recursive mutex. This is motivated by
windows, where a much lighter wait alternative exists for non-recursive
locks. On pthreads, the same lock type is available with a different
typename and no static initializer value.

Introduce an internal __libcpp_mutex_reference to provide an
implicitly move constructed with type-erasure to let the condition
variable handling get a type-erased discriminated-union which we can
handle appropriately.

[compnerd]

add more context

[alexander-shaposhnikov]

include/__threading_support

99

indentation looks a bit strange

[EricWF]

include/__threading_support

96

This uses C++11 features but is limited to C++03.

108

Wouldn't a static_cast be valid here?

127

This uses C++11 features but is limited to C++03. Although we can technically cheat and use Clang extended rvalue references here.

129

Won't we ever want to pass an lvalue here?

[majnemer]

Is __libcpp_mutex_reference all that useful? There is no real dynamism for these mutexes. I'd just add functions for the recursive locks just like we have a separate function for __libcpp_recursive_mutex_init

[compnerd]

The dynamic behavior only is used for Windows, not pthreads. So, we dont see it here, but it becomes apparent in the windows support. I was trying to minimize the changes to libc++ itself to avoid having it to consider the recursive vs non-recursive cases. If the libc++ maintainers would prefer to expand the interfaces rather than use the dynamic behavior, Im fine with that approach too.

[majnemer]

In D28226#634282, @compnerd wrote:

The dynamic behavior only is used for Windows, not pthreads. So, we dont see it here, but it becomes apparent in the windows support. I was trying to minimize the changes to libc++ itself to avoid having it to consider the recursive vs non-recursive cases. If the libc++ maintainers would prefer to expand the interfaces rather than use the dynamic behavior, Im fine with that approach too.

Another argument that I forgot to mention: __libcpp_condvar_wait and __libcpp_condvar_timedwait are only expected to work with non-recursive mutexes. I feel that this overwidens the API surface. Flattening it out removes the dynamic behavior and makes it clear what operations needs to support what types of mutexes.

[compnerd]

include/__threading_support

108

Yeah.

127

Switched to pass-by-value.

129

Just changed it to a pass-by-value, which is a l-value.

[compnerd]

Use l-values, static_cast.

[compnerd]

Im not tied to this approach. I really want to get @mclow.lists or @EricWF to weigh in as to which approach they prefer (new interfaces or the wrapper, or even a new approach).

[EricWF]

Yeah, I think I agree with @majnemer that I would rather use an expanded interface than this type-erased mutex type. Thank you for working on this.

[compnerd]

Superceded by https://reviews.llvm.org/D28338

include/__threading_support

 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) || \
     defined(_LIBCPP_HAS_THREAD_API_EXTERNAL_PTHREAD)
 // Mutex
 typedef pthread_mutex_t __libcpp_mutex_t;
 #define _LIBCPP_MUTEX_INITIALIZER PTHREAD_MUTEX_INITIALIZER
 
+typedef pthread_mutex_t __libcpp_recursive_mutex_t;
+
 // Condition Variable
 typedef pthread_cond_t __libcpp_condvar_t;
 #define _LIBCPP_CONDVAR_INITIALIZER PTHREAD_COND_INITIALIZER
 
 // Execute once
 typedef pthread_once_t __libcpp_exec_once_flag;
 #define _LIBCPP_EXEC_ONCE_INITIALIZER PTHREAD_ONCE_INIT
 
 // Thread id
 typedef pthread_t __libcpp_thread_id;
 
 // Thread
 typedef pthread_t __libcpp_thread_t;
 
 // Thrad Local Storage
 typedef pthread_key_t __libcpp_tls_key;
 #endif
 
+struct __libcpp_mutex_reference {
+  const bool __recursive;
+  void * const __value;
+
+  _LIBCPP_CONSTEXPR
+  __libcpp_mutex_reference(__libcpp_mutex_t* m) _NOEXCEPT
+      : __recursive(false), __value(m)
+  {
+  }
+
+  template <typename _Mutex = __libcpp_mutex_t,
+            typename _RecursiveMutex = __libcpp_recursive_mutex_t,
+            typename = typename _VSTD::enable_if<!_VSTD::is_same<_Mutex,
+                                                                 _RecursiveMutex>::value>::type>
+  _LIBCPP_CONSTEXPR
+  __libcpp_mutex_reference(__libcpp_recursive_mutex_t* m) _NOEXCEPT
+      : __recursive(true), __value(m)
+  {
+  }
+
+  operator __libcpp_mutex_t*() const _NOEXCEPT
+  {
+    return static_cast<__libcpp_mutex_t*>(__value);
+  }
+
+  template <typename _Mutex = __libcpp_mutex_t,
+            typename _RecursiveMutex = __libcpp_recursive_mutex_t,
+            typename = typename _VSTD::enable_if<!_VSTD::is_same<_Mutex,
+                                                                 _RecursiveMutex>::value>::type>
+  operator __libcpp_recursive_mutex_t*() const _NOEXCEPT
+  {
+    return static_cast<__libcpp_recursive_mutex_t*>(__value);
+  }
+};
+
 // Mutex
 _LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_recursive_mutex_init(__libcpp_mutex_t *__m);
+int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m);
 
 _LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_mutex_lock(__libcpp_mutex_t *__m);
+int __libcpp_mutex_lock(__libcpp_mutex_reference __m);
 
 _LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_mutex_trylock(__libcpp_mutex_t *__m);
+int __libcpp_mutex_trylock(__libcpp_mutex_reference __m);
 
 _LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_mutex_unlock(__libcpp_mutex_t *__m);
+int __libcpp_mutex_unlock(__libcpp_mutex_reference __m);
 
 _LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_mutex_destroy(__libcpp_mutex_t *__m);
+int __libcpp_mutex_destroy(__libcpp_mutex_reference __m);
 
 // Condition variable
 _LIBCPP_THREAD_ABI_VISIBILITY
 int __libcpp_condvar_signal(__libcpp_condvar_t* __cv);
 
 _LIBCPP_THREAD_ABI_VISIBILITY
 int __libcpp_condvar_broadcast(__libcpp_condvar_t* __cv);
 
 _LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_condvar_wait(__libcpp_condvar_t* __cv, __libcpp_mutex_t* __m);
+int __libcpp_condvar_wait(__libcpp_condvar_t* __cv,
+                          __libcpp_mutex_reference __m);
 
 _LIBCPP_THREAD_ABI_VISIBILITY
-int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m,
-                               timespec *__ts);
+int __libcpp_condvar_timedwait(__libcpp_condvar_t* __cv,
+                               __libcpp_mutex_reference __m, timespec* __ts);
 
 _LIBCPP_THREAD_ABI_VISIBILITY
 int __libcpp_condvar_destroy(__libcpp_condvar_t* __cv);
 
 // Execute once
 _LIBCPP_THREAD_ABI_VISIBILITY
 int __libcpp_execute_once(__libcpp_exec_once_flag *flag,
                           void (*init_routine)(void));

 void *__libcpp_tls_get(__libcpp_tls_key __key);
 
 _LIBCPP_THREAD_ABI_VISIBILITY
 int __libcpp_tls_set(__libcpp_tls_key __key, void *__p);
 
 #if defined(_LIBCPP_HAS_THREAD_API_PTHREAD) || \
     defined(_LIBCPP_BUILDING_THREAD_API_EXTERNAL_PTHREAD)
 
-int __libcpp_recursive_mutex_init(__libcpp_mutex_t *__m)
+int __libcpp_recursive_mutex_init(__libcpp_recursive_mutex_t *__m)
 {
   pthread_mutexattr_t attr;
   int __ec = pthread_mutexattr_init(&attr);
   if (__ec)
     return __ec;
   __ec = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
   if (__ec) {
     pthread_mutexattr_destroy(&attr);
     return __ec;
   }
   __ec = pthread_mutex_init(__m, &attr);
   if (__ec) {
     pthread_mutexattr_destroy(&attr);
     return __ec;
   }
   __ec = pthread_mutexattr_destroy(&attr);
   if (__ec) {
     pthread_mutex_destroy(__m);
     return __ec;
   }
   return 0;
 }
 
-int __libcpp_mutex_lock(__libcpp_mutex_t *__m)
+int __libcpp_mutex_lock(__libcpp_mutex_reference __m)
 {
   return pthread_mutex_lock(__m);
 }
 
-int __libcpp_mutex_trylock(__libcpp_mutex_t *__m)
+int __libcpp_mutex_trylock(__libcpp_mutex_reference __m)
 {
   return pthread_mutex_trylock(__m);
 }
 
-int __libcpp_mutex_unlock(__libcpp_mutex_t *__m)
+int __libcpp_mutex_unlock(__libcpp_mutex_reference __m)
 {
   return pthread_mutex_unlock(__m);
 }
 
-int __libcpp_mutex_destroy(__libcpp_mutex_t *__m)
+int __libcpp_mutex_destroy(__libcpp_mutex_reference __m)
 {
   return pthread_mutex_destroy(__m);
 }
 
 // Condition Variable
 int __libcpp_condvar_signal(__libcpp_condvar_t *__cv)
 {
   return pthread_cond_signal(__cv);
 }
 
 int __libcpp_condvar_broadcast(__libcpp_condvar_t *__cv)
 {
   return pthread_cond_broadcast(__cv);
 }
 
-int __libcpp_condvar_wait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m)
+int __libcpp_condvar_wait(__libcpp_condvar_t* __cv,
+                          __libcpp_mutex_reference __m)
 {
   return pthread_cond_wait(__cv, __m);
 }
 
-int __libcpp_condvar_timedwait(__libcpp_condvar_t *__cv, __libcpp_mutex_t *__m,
-                               timespec *__ts)
+int __libcpp_condvar_timedwait(__libcpp_condvar_t* __cv,
+                               __libcpp_mutex_reference __m, timespec* __ts)
 {
   return pthread_cond_timedwait(__cv, __m, __ts);
 }
 
 int __libcpp_condvar_destroy(__libcpp_condvar_t *__cv)
 {
   return pthread_cond_destroy(__cv);
 }

include/mutex

 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #ifndef _LIBCPP_HAS_NO_THREADS
 
 class _LIBCPP_TYPE_VIS recursive_mutex
 {
-    __libcpp_mutex_t __m_;
+    __libcpp_recursive_mutex_t __m_;
 
 public:
      recursive_mutex();
      ~recursive_mutex();
 
 private:
     recursive_mutex(const recursive_mutex&); // = delete;
     recursive_mutex& operator=(const recursive_mutex&); // = delete;
 
 public:
     void lock();
     bool try_lock() _NOEXCEPT;
     void unlock()  _NOEXCEPT;
 
-    typedef __libcpp_mutex_t* native_handle_type;
+    typedef __libcpp_recursive_mutex_t* native_handle_type;
     _LIBCPP_INLINE_VISIBILITY
     native_handle_type native_handle() {return &__m_;}
 };
 
 class _LIBCPP_TYPE_VIS timed_mutex
 {
     mutex              __m_;
     condition_variable __cv_;