Overhaul condition_variable (#4720)

stl/inc/__msvc_threads_core.hpp

@@ -52,6 +52,13 @@ struct _Mtx_internal_imp_t {
 
 using _Mtx_t = _Mtx_internal_imp_t*;
 
+struct _Stl_condition_variable {
+    void* _Unused = nullptr; // TRANSITION, ABI: was the vptr
+    void* _Win_cv = nullptr;
+};
+
+#pragma warning(push)
+#pragma warning(disable : 26495) // Variable 'meow' is uninitialized. Always initialize a member variable (type.6).
 struct _Cnd_internal_imp_t {
 #if defined(_CRT_WINDOWS) // for Windows-internal code
     static constexpr size_t _Cnd_internal_imp_size = 2 * sizeof(void*);
@@ -61,8 +68,12 @@ struct _Cnd_internal_imp_t {
     static constexpr size_t _Cnd_internal_imp_size = 40;
 #endif // ^^^ ordinary 32-bit code ^^^
 
-    _STD _Aligned_storage_t<_Cnd_internal_imp_size, alignof(void*)> _Cv_storage;
+    union {
+        _Stl_condition_variable _Stl_cv{};
+        _STD _Aligned_storage_t<_Cnd_internal_imp_size, alignof(void*)> _Cv_storage;
+    };
 };
+#pragma warning(pop)
 
 using _Cnd_t = _Cnd_internal_imp_t*;
 } // extern "C"

stl/inc/condition_variable

@@ -52,12 +52,12 @@ private:
 _EXPORT_STD class condition_variable_any { // class for waiting for conditions with any kind of mutex
 public:
     condition_variable_any() : _Myptr{_STD make_shared<mutex>()} {
+#ifdef _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
         _Cnd_init_in_situ(_Mycnd());
+#endif // ^^^ defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) ^^^
     }
 
-    ~condition_variable_any() noexcept {
-        _Cnd_destroy_in_situ(_Mycnd());
-    }
+    ~condition_variable_any() noexcept = default;
 
     condition_variable_any(const condition_variable_any&)            = delete;
     condition_variable_any& operator=(const condition_variable_any&) = delete;
@@ -218,7 +218,7 @@ public:
 private:
     shared_ptr<mutex> _Myptr;
 
-    _Cnd_internal_imp_t _Cnd_storage;
+    _Cnd_internal_imp_t _Cnd_storage{};
 
     _NODISCARD _Cnd_t _Mycnd() noexcept {
         return &_Cnd_storage;

stl/inc/mutex

@@ -531,13 +531,15 @@ _EXPORT_STD enum class cv_status { // names for wait returns
 
 _EXPORT_STD class condition_variable { // class for waiting for conditions
 public:
+#ifdef _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
     condition_variable() noexcept /* strengthened */ {
         _Cnd_init_in_situ(_Mycnd());
     }
+#else // ^^^ defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) / !defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) vvv
+    condition_variable() noexcept /* strengthened */ = default;
+#endif // ^^^ !defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) ^^^
 
-    ~condition_variable() noexcept {
-        _Cnd_destroy_in_situ(_Mycnd());
-    }
+    ~condition_variable() noexcept = default;
 
     condition_variable(const condition_variable&)            = delete;
     condition_variable& operator=(const condition_variable&) = delete;
@@ -629,7 +631,7 @@ public:
     }
 
 private:
-    _Cnd_internal_imp_t _Cnd_storage;
+    _Cnd_internal_imp_t _Cnd_storage{};
 
     _Cnd_t _Mycnd() noexcept {
         return &_Cnd_storage;

stl/inc/xthreads.h

@@ -35,11 +35,9 @@ enum { // mutex types
     _Mtx_recursive = 0x100
 };
 
-#ifdef _CRTBLD
-_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_init(_Mtx_t*, int) noexcept;
-_CRTIMP2_PURE void __cdecl _Mtx_destroy(_Mtx_t) noexcept;
-#endif // _CRTBLD
+#if defined(_CRTBLD) || defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR)
 _CRTIMP2_PURE void __cdecl _Mtx_init_in_situ(_Mtx_t, int) noexcept;
+#endif // ^^^ defined(_CRTBLD) || defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) ^^^
 _CRTIMP2_PURE int __cdecl _Mtx_current_owns(_Mtx_t) noexcept;
 _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_lock(_Mtx_t) noexcept;
 _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_trylock(_Mtx_t) noexcept;
@@ -55,12 +53,9 @@ void __cdecl _Smtx_unlock_exclusive(_Smtx_t*) noexcept;
 void __cdecl _Smtx_unlock_shared(_Smtx_t*) noexcept;
 
 // condition variables
-#ifdef _CRTBLD
-_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_init(_Cnd_t*) noexcept;
-_CRTIMP2_PURE void __cdecl _Cnd_destroy(_Cnd_t) noexcept;
-#endif // _CRTBLD
+#ifdef _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
 _CRTIMP2_PURE void __cdecl _Cnd_init_in_situ(_Cnd_t) noexcept;
-_CRTIMP2_PURE void __cdecl _Cnd_destroy_in_situ(_Cnd_t) noexcept;
+#endif // ^^^ defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) ^^^
 _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(_Cnd_t, _Mtx_t) noexcept; // TRANSITION, ABI: Always succeeds
 _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_broadcast(_Cnd_t) noexcept; // TRANSITION, ABI: Always succeeds
 _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_signal(_Cnd_t) noexcept; // TRANSITION, ABI: Always succeeds

stl/src/cond.cpp

@@ -12,13 +12,15 @@
 
 extern "C" {
 
-
+// TRANSITION, ABI: preserved for binary compatibility (and _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR)
 _CRTIMP2_PURE void __cdecl _Cnd_init_in_situ(const _Cnd_t cond) noexcept { // initialize condition variable in situ
-    new (Concurrency::details::_Get_cond_var(cond)) Concurrency::details::stl_condition_variable_win7;
+    new (cond) _Cnd_internal_imp_t;
 }
 
+// TRANSITION, ABI: preserved for binary compatibility
 _CRTIMP2_PURE void __cdecl _Cnd_destroy_in_situ(_Cnd_t) noexcept {} // destroy condition variable in situ
 
+// TRANSITION, ABI: preserved for binary compatibility
 _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_init(_Cnd_t* const pcond) noexcept { // initialize
     *pcond = nullptr;
 
@@ -32,9 +34,9 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_init(_Cnd_t* const pcond) noexcept { //
     return _Thrd_result::_Success;
 }
 
+// TRANSITION, ABI: preserved for binary compatibility
 _CRTIMP2_PURE void __cdecl _Cnd_destroy(const _Cnd_t cond) noexcept { // clean up
     if (cond) { // something to do, do it
-        _Cnd_destroy_in_situ(cond);
         _free_crt(cond);
     }
 }
@@ -52,9 +54,8 @@ _CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t mtx) noexcept { // set owner
 }
 
 _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx) noexcept { // wait until signaled
-    const auto cs = &mtx->_Critical_section;
     _Mtx_clear_owner(mtx);
-    Concurrency::details::_Get_cond_var(cond)->wait(cs);
+    _Primitive_wait(cond, mtx);
     _Mtx_reset_owner(mtx);
     return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds
 }
@@ -63,17 +64,15 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx
 _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_timedwait(
     const _Cnd_t cond, const _Mtx_t mtx, const _timespec64* const target) noexcept {
     _Thrd_result res = _Thrd_result::_Success;
-    const auto cs    = &mtx->_Critical_section;
     if (target == nullptr) { // no target time specified, wait on mutex
         _Mtx_clear_owner(mtx);
-        Concurrency::details::_Get_cond_var(cond)->wait(cs);
+        _Primitive_wait(cond, mtx);
         _Mtx_reset_owner(mtx);
     } else { // target time specified, wait for it
         _timespec64 now;
         _Timespec64_get_sys(&now);
         _Mtx_clear_owner(mtx);
-        if (!Concurrency::details::_Get_cond_var(cond)->wait_for(
-                cs, _Xtime_diff_to_millis2(target, &now))) { // report timeout
+        if (!_Primitive_wait_for(cond, mtx, _Xtime_diff_to_millis2(target, &now))) { // report timeout
             _Timespec64_get_sys(&now);
             if (_Xtime_diff_to_millis2(target, &now) == 0) {
                 res = _Thrd_result::_Timedout;
@@ -85,12 +84,12 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_timedwait(
 }
 
 _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_signal(const _Cnd_t cond) noexcept { // release one waiting thread
-    Concurrency::details::_Get_cond_var(cond)->notify_one();
+    _Primitive_notify_one(cond);
     return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds
 }
 
 _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_broadcast(const _Cnd_t cond) noexcept { // release all waiting threads
-    Concurrency::details::_Get_cond_var(cond)->notify_all();
+    _Primitive_notify_all(cond);
     return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds
 }
 

stl/src/cthread.cpp

@@ -113,10 +113,11 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Thrd_create(_Thrd_t* thr, _Thrd_start_t func
     _Thrd_result res;
     _Thrd_binder b;
     int started = 0;
-    _Cnd_t cond;
-    _Mtx_t mtx;
-    _Cnd_init(&cond);
-    _Mtx_init(&mtx, _Mtx_plain);
+    _Cnd_internal_imp_t cond_var{};
+    _Cnd_t cond = &cond_var;
+    _Mtx_internal_imp_t mtx_var{};
+    _Mtx_t mtx = &mtx_var;
+    _Mtx_init_in_situ(mtx, _Mtx_plain);
     b.func    = func;
     b.data    = d;
     b.cond    = &cond;
@@ -129,8 +130,6 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Thrd_create(_Thrd_t* thr, _Thrd_start_t func
         }
     }
     _Mtx_unlock(mtx);
-    _Cnd_destroy(cond);
-    _Mtx_destroy(mtx);
     return res;
 }
 

stl/src/mutex.cpp

@@ -10,8 +10,6 @@
 #include <xthreads.h>
 #include <xtimec.h>
 
-#include "primitives.hpp"
-
 extern "C" {
 
 // TRANSITION, ABI: exported only for ABI compat
@@ -37,7 +35,7 @@ _CRTIMP2 void __cdecl __set_stl_sync_api_mode(__stl_sync_api_modes_enum) noexcep
     return reinterpret_cast<PSRWLOCK>(&mtx->_Critical_section._M_srw_lock);
 }
 
-// TRANSITION, only used when constexpr mutex constructor is not enabled
+// TRANSITION, ABI: preserved for binary compatibility (and _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR)
 _CRTIMP2_PURE void __cdecl _Mtx_init_in_situ(_Mtx_t mtx, int type) noexcept { // initialize mutex in situ
     new (&mtx->_Critical_section) _Stl_critical_section;
     mtx->_Thread_id = -1;
@@ -51,6 +49,7 @@ _CRTIMP2_PURE void __cdecl _Mtx_destroy_in_situ(_Mtx_t mtx) noexcept { // destro
     (void) mtx;
 }
 
+// TRANSITION, ABI: preserved for binary compatibility
 _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_init(_Mtx_t* mtx, int type) noexcept { // initialize mutex
     *mtx = nullptr;
 

stl/src/primitives.hpp

@@ -5,47 +5,27 @@
 
 #include <__msvc_threads_core.hpp>
 #include <cstdlib>
-#include <type_traits>
 
 #include <Windows.h>
 
-namespace Concurrency {
-    namespace details {
-        class stl_condition_variable_win7 {
-        public:
-            stl_condition_variable_win7() = default;
-
-            ~stl_condition_variable_win7()                                             = delete;
-            stl_condition_variable_win7(const stl_condition_variable_win7&)            = delete;
-            stl_condition_variable_win7& operator=(const stl_condition_variable_win7&) = delete;
-
-            void wait(_Stl_critical_section* lock) {
-                if (!wait_for(lock, INFINITE)) {
-                    _CSTD abort();
-                }
-            }
-
-            bool wait_for(_Stl_critical_section* lock, unsigned int timeout) {
-                return SleepConditionVariableSRW(
-                           &m_condition_variable, reinterpret_cast<PSRWLOCK>(&lock->_M_srw_lock), timeout, 0)
-                    != 0;
-            }
-
-            void notify_one() {
-                WakeConditionVariable(&m_condition_variable);
-            }
-
-            void notify_all() {
-                WakeAllConditionVariable(&m_condition_variable);
-            }
-
-        private:
-            void* unused                            = nullptr; // TRANSITION, ABI: was the vptr
-            CONDITION_VARIABLE m_condition_variable = CONDITION_VARIABLE_INIT;
-        };
-
-        [[nodiscard]] inline stl_condition_variable_win7* _Get_cond_var(::_Cnd_internal_imp_t* _Cond) noexcept {
-            return reinterpret_cast<stl_condition_variable_win7*>(&_Cond->_Cv_storage);
-        }
-    } // namespace details
-} // namespace Concurrency
+inline bool _Primitive_wait_for(const _Cnd_t cond, const _Mtx_t mtx, unsigned int timeout) noexcept {
+    const auto pcv  = reinterpret_cast<PCONDITION_VARIABLE>(&cond->_Stl_cv._Win_cv);
+    const auto psrw = reinterpret_cast<PSRWLOCK>(&mtx->_Critical_section._M_srw_lock);
+    return SleepConditionVariableSRW(pcv, psrw, timeout, 0) != 0;
+}
+
+inline void _Primitive_wait(const _Cnd_t cond, const _Mtx_t mtx) noexcept {
+    if (!_Primitive_wait_for(cond, mtx, INFINITE)) {
+        _CSTD abort();
+    }
+}
+
+inline void _Primitive_notify_one(const _Cnd_t cond) noexcept {
+    const auto pcv = reinterpret_cast<PCONDITION_VARIABLE>(&cond->_Stl_cv._Win_cv);
+    WakeConditionVariable(pcv);
+}
+
+inline void _Primitive_notify_all(const _Cnd_t cond) noexcept {
+    const auto pcv = reinterpret_cast<PCONDITION_VARIABLE>(&cond->_Stl_cv._Win_cv);
+    WakeAllConditionVariable(pcv);
+}

stl/src/sharedmutex.cpp

@@ -43,14 +43,13 @@ void __stdcall _Thrd_sleep_for(const unsigned long ms) noexcept { // suspend cur
 
 _Thrd_result __stdcall _Cnd_timedwait_for(const _Cnd_t cond, const _Mtx_t mtx, const unsigned int target_ms) noexcept {
     _Thrd_result res    = _Thrd_result::_Success;
-    const auto cs       = &mtx->_Critical_section;
     const auto start_ms = GetTickCount64();
 
     // TRANSITION: replace with _Mtx_clear_owner(mtx);
     mtx->_Thread_id = -1;
     --mtx->_Count;
 
-    if (!Concurrency::details::_Get_cond_var(cond)->wait_for(cs, target_ms)) { // report timeout
+    if (!_Primitive_wait_for(cond, mtx, target_ms)) { // report timeout
         const auto end_ms = GetTickCount64();
         if (end_ms - start_ms >= target_ms) {
             res = _Thrd_result::_Timedout;

tests/libcxx/expected_results.txt

@@ -26,6 +26,11 @@ std/time/time.syn/formatter.year_month_weekday.pass.cpp:1 FAIL
 std/utilities/memory/specialized.algorithms/uninitialized.copy/uninitialized_copy.pass.cpp FAIL
 std/utilities/memory/specialized.algorithms/uninitialized.move/uninitialized_move.pass.cpp FAIL
 
+# LLVM-94907: [libc++][test] Avoid -Wunused-variable warnings in mutex tests
+std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/default.pass.cpp:2 FAIL
+std/thread/thread.mutex/thread.mutex.requirements/thread.timedmutex.requirements/thread.timedmutex.class/default.pass.cpp:2 FAIL
+std/thread/thread.mutex/thread.mutex.requirements/thread.timedmutex.requirements/thread.timedmutex.recursive/default.pass.cpp:2 FAIL
+
 # Non-Standard regex behavior.
 # "It seems likely that the test is still non-conforming due to how libc++ handles the 'w' character class."
 std/re/re.traits/lookup_classname.pass.cpp FAIL