Wait on condition variables independently on system time (#4457)

Co-authored-by: Stephan T. Lavavej <[email protected]>
microsoft · Mar 16, 2024 · 4b80c7c · 4b80c7c
1 parent 79e79a2
commit 4b80c7c
Show file tree

Hide file tree

Showing 13 changed files with 184 additions and 139 deletions.
diff --git a/stl/CMakeLists.txt b/stl/CMakeLists.txt
@@ -19,6 +19,7 @@ set(HEADERS
     ${CMAKE_CURRENT_LIST_DIR}/inc/__msvc_print.hpp
     ${CMAKE_CURRENT_LIST_DIR}/inc/__msvc_sanitizer_annotate_container.hpp
     ${CMAKE_CURRENT_LIST_DIR}/inc/__msvc_system_error_abi.hpp
+    ${CMAKE_CURRENT_LIST_DIR}/inc/__msvc_threads_core.hpp
     ${CMAKE_CURRENT_LIST_DIR}/inc/__msvc_tzdb.hpp
     ${CMAKE_CURRENT_LIST_DIR}/inc/__msvc_xlocinfo_types.hpp
     ${CMAKE_CURRENT_LIST_DIR}/inc/algorithm

diff --git a/stl/inc/__msvc_chrono.hpp b/stl/inc/__msvc_chrono.hpp
@@ -715,30 +715,6 @@ namespace chrono {
     _EXPORT_STD using high_resolution_clock = steady_clock;
 } // namespace chrono
 
-template <class _Rep, class _Period>
-_NODISCARD bool _To_timespec64_sys_10_day_clamped(
-    _timespec64& _Ts64, const _CHRONO duration<_Rep, _Period>& _Rel_time) noexcept(is_arithmetic_v<_Rep>) {
-    // Convert duration to _timespec64 representing system time, maximum 10 days from now, returns whether clamping
-    // occurred. If clamped, timeouts will be transformed into spurious non-timeout wakes, due to ABI restrictions where
-    // the other side of the DLL boundary overflows int32_t milliseconds.
-    // Every function calling this one is TRANSITION, ABI
-    constexpr _CHRONO nanoseconds _Ten_days{_CHRONO hours{24} * 10};
-    constexpr _CHRONO duration<double> _Ten_days_d{_Ten_days};
-    _CHRONO nanoseconds _Tx0 = _CHRONO system_clock::duration{_Xtime_get_ticks()};
-    const bool _Clamped      = _Ten_days_d < _Rel_time;
-    if (_Clamped) {
-        _Tx0 += _Ten_days;
-    } else {
-        _Tx0 += _CHRONO duration_cast<_CHRONO nanoseconds>(_Rel_time);
-    }
-
-    const auto _Whole_seconds = _CHRONO duration_cast<_CHRONO seconds>(_Tx0);
-    _Ts64.tv_sec              = _Whole_seconds.count();
-    _Tx0 -= _Whole_seconds;
-    _Ts64.tv_nsec = static_cast<long>(_Tx0.count());
-    return _Clamped;
-}
-
 inline namespace literals {
     inline namespace chrono_literals {
         _EXPORT_STD _NODISCARD constexpr _CHRONO hours operator""h(unsigned long long _Val) noexcept

diff --git a/stl/inc/__msvc_threads_core.hpp b/stl/inc/__msvc_threads_core.hpp
@@ -0,0 +1,103 @@
+// __msvc_threads_core.hpp internal header (core)
+
+// Copyright (c) Microsoft Corporation.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef __MSVC_THREADS_CORE_HPP
+#define __MSVC_THREADS_CORE_HPP
+#include <yvals_core.h>
+#if _STL_COMPILER_PREPROCESSOR
+#include <type_traits>
+
+#pragma pack(push, _CRT_PACKING)
+#pragma warning(push, _STL_WARNING_LEVEL)
+#pragma warning(disable : _STL_DISABLED_WARNINGS)
+_STL_DISABLE_CLANG_WARNINGS
+#pragma push_macro("new")
+#undef new
+
+extern "C" {
+using _Thrd_id_t = unsigned int;
+struct _Thrd_t { // thread identifier for Win32
+    void* _Hnd; // Win32 HANDLE
+    _Thrd_id_t _Id;
+};
+
+using _Smtx_t = void*;
+
+enum class _Thrd_result : int { _Success, _Nomem, _Timedout, _Busy, _Error };
+
+struct _Stl_critical_section {
+    void* _Unused       = nullptr; // TRANSITION, ABI: was the vptr
+    _Smtx_t _M_srw_lock = nullptr;
+};
+
+struct _Mtx_internal_imp_t {
+#if defined(_CRT_WINDOWS) || defined(UNDOCKED_WINDOWS_UCRT)
+#ifdef _WIN64
+    static constexpr size_t _Critical_section_size = 16;
+#else // ^^^ defined(_WIN64) / !defined(_WIN64) vvv
+    static constexpr size_t _Critical_section_size = 8;
+#endif // ^^^ !defined(_WIN64) ^^^
+#else // ^^^ Windows private STL / public STL vvv
+#ifdef _WIN64
+    static constexpr size_t _Critical_section_size = 64;
+#else // ^^^ defined(_WIN64) / !defined(_WIN64) vvv
+    static constexpr size_t _Critical_section_size = 36;
+#endif // ^^^ !defined(_WIN64) ^^^
+#endif // ^^^ public STL ^^^
+
+    static constexpr size_t _Critical_section_align = alignof(void*);
+
+    int _Type{};
+    union {
+        _Stl_critical_section _Critical_section{};
+        _STD _Aligned_storage_t<_Critical_section_size, _Critical_section_align> _Cs_storage;
+    };
+    long _Thread_id{};
+    int _Count{};
+};
+
+// Size and alignment for _Cnd_internal_imp_t
+#if defined(_CRT_WINDOWS) // for Windows-internal code
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 2 * sizeof(void*);
+#elif defined(_WIN64) // ordinary 64-bit code
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 72;
+#else // vvv ordinary 32-bit code vvv
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 40;
+#endif // ^^^ ordinary 32-bit code ^^^
+
+_INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = alignof(void*);
+
+using _Mtx_t = _Mtx_internal_imp_t*;
+
+#ifdef _M_CEE // avoid warning LNK4248: unresolved typeref token for '_Cnd_internal_imp_t'; image may not run
+using _Cnd_t = void*;
+#else // ^^^ defined(_M_CEE) / !defined(_M_CEE) vvv
+struct _Cnd_internal_imp_t;
+using _Cnd_t = _Cnd_internal_imp_t*;
+#endif // ^^^ !defined(_M_CEE) ^^^
+} // extern "C"
+
+#pragma pop_macro("new")
+_STL_RESTORE_CLANG_WARNINGS
+#pragma warning(pop)
+#pragma pack(pop)
+#endif // _STL_COMPILER_PREPROCESSOR
+#endif // __MSVC_THREADS_CORE_HPP
+
+/*
+ * This file is derived from software bearing the following
+ * restrictions:
+ *
+ * (c) Copyright William E. Kempf 2001
+ *
+ * Permission to use, copy, modify, distribute and sell this
+ * software and its documentation for any purpose is hereby
+ * granted without fee, provided that the above copyright
+ * notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting
+ * documentation. William E. Kempf makes no representations
+ * about the suitability of this software for any purpose.
+ * It is provided "as is" without express or implied warranty.
+ */
diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable
@@ -124,12 +124,9 @@ public:
             return cv_status::timeout;
         }
 
-        // TRANSITION, ABI: The standard says that we should use a steady clock,
-        // but unfortunately our ABI relies on the system clock.
-        _timespec64 _Tgt;
-        const bool _Clamped     = _To_timespec64_sys_10_day_clamped(_Tgt, _Rel_time);
-        const cv_status _Result = _Wait_until_sys_time(_Lck, &_Tgt);
-        if (_Clamped) {
+        const auto _Rel_time_ms = _Clamped_rel_time_ms_count(_Rel_time);
+        const cv_status _Result = _Wait_for_ms_count(_Lck, _Rel_time_ms._Count);
+        if (_Rel_time_ms._Clamped) {
             return cv_status::no_timeout;
         }
 
@@ -206,12 +203,8 @@ public:
                 break;
             }
 
-            const auto _Rel_time = _Abs_time - _Now;
-            // TRANSITION, ABI: The standard says that we should use a steady clock,
-            // but unfortunately our ABI relies on the system clock.
-            _timespec64 _Tgt;
-            (void) _To_timespec64_sys_10_day_clamped(_Tgt, _Rel_time);
-            (void) _Cnd_timedwait(_Mycnd(), _Myptr->_Mymtx(), &_Tgt);
+            const unsigned long _Rel_ms_count = _Clamped_rel_time_ms_count(_Abs_time - _Now)._Count;
+            (void) _Cnd_timedwait_for(_Mycnd(), _Myptr->_Mymtx(), _Rel_ms_count);
             _Guard_unlocks_before_locking_outer.unlock();
         } // relock
 
@@ -234,12 +227,12 @@ private:
     }
 
     template <class _Lock>
-    cv_status _Wait_until_sys_time(_Lock& _Lck, const _timespec64* const _Abs_time) {
+    cv_status _Wait_for_ms_count(_Lock& _Lck, const unsigned int _Rel_ms_count) {
         // wait for signal with timeout
         const shared_ptr<mutex> _Ptr = _Myptr; // for immunity to *this destruction
         unique_lock<mutex> _Guard{*_Ptr};
         _Unlock_guard<_Lock> _Unlock_outer{_Lck};
-        const _Thrd_result _Res = _Cnd_timedwait(_Mycnd(), _Ptr->_Mymtx(), _Abs_time);
+        const _Thrd_result _Res = _Cnd_timedwait_for(_Mycnd(), _Ptr->_Mymtx(), _Rel_ms_count);
         _Guard.unlock();
 
         if (_Res == _Thrd_result::_Success) {

diff --git a/stl/inc/header-units.json b/stl/inc/header-units.json
@@ -17,6 +17,7 @@
         "__msvc_print.hpp",
         "__msvc_sanitizer_annotate_container.hpp",
         "__msvc_system_error_abi.hpp",
+        "__msvc_threads_core.hpp",
         "__msvc_tzdb.hpp",
         "__msvc_xlocinfo_types.hpp",
         "algorithm",

diff --git a/stl/inc/mutex b/stl/inc/mutex
@@ -601,11 +601,9 @@ public:
                 return cv_status::timeout;
             }
 
-            // TRANSITION, ABI: should use a steady clock
-            _timespec64 _Tgt;
-            (void) _To_timespec64_sys_10_day_clamped(_Tgt, _Abs_time - _Now);
-            // Nothing to do to comply with LWG-2135 because std::mutex lock/unlock are nothrow
-            const _Thrd_result _Res = _Cnd_timedwait(_Mycnd(), _Lck.mutex()->_Mymtx(), &_Tgt);
+            const unsigned long _Rel_ms_count = _Clamped_rel_time_ms_count(_Abs_time - _Now)._Count;
+
+            const _Thrd_result _Res = _Cnd_timedwait_for(_Mycnd(), _Lck.mutex()->_Mymtx(), _Rel_ms_count);
             if (_Res == _Thrd_result::_Success) {
                 return cv_status::no_timeout;
             }

diff --git a/stl/inc/thread b/stl/inc/thread
@@ -179,6 +179,24 @@ _NODISCARD auto _To_absolute_time(const chrono::duration<_Rep, _Period>& _Rel_ti
     return _Abs_time;
 }
 
+struct _Clamped_rel_time_ms_count_result {
+    unsigned long _Count;
+    bool _Clamped;
+};
+
+template <class _Duration>
+_NODISCARD _Clamped_rel_time_ms_count_result _Clamped_rel_time_ms_count(const _Duration& _Rel) {
+    // _Clamp must be less than 2^32 - 1 (INFINITE) milliseconds, but is otherwise arbitrary.
+    constexpr chrono::milliseconds _Clamp{chrono::hours{24}};
+
+    if (_Rel > _Clamp) {
+        return {static_cast<unsigned long>(_Clamp.count()), true};
+    } else {
+        const auto _Rel_ms = chrono::ceil<chrono::milliseconds>(_Rel);
+        return {static_cast<unsigned long>(_Rel_ms.count()), false};
+    }
+}
+
 namespace this_thread {
     _EXPORT_STD _NODISCARD thread::id get_id() noexcept;
 
@@ -197,16 +215,8 @@ namespace this_thread {
                 return;
             }
 
-            // _Clamp must be less than 2^32 - 1 (INFINITE) milliseconds, but is otherwise arbitrary.
-            constexpr chrono::milliseconds _Clamp{chrono::hours{24}};
-
-            const auto _Rel = _Abs_time - _Now;
-            if (_Rel >= _Clamp) {
-                _Thrd_sleep_for(static_cast<unsigned long>(_Clamp.count()));
-            } else {
-                const auto _Rel_ms = chrono::ceil<chrono::milliseconds>(_Rel);
-                _Thrd_sleep_for(static_cast<unsigned long>(_Rel_ms.count()));
-            }
+            const unsigned long _Rel_ms_count = _Clamped_rel_time_ms_count(_Abs_time - _Now)._Count;
+            _Thrd_sleep_for(_Rel_ms_count);
         }
     }
 

diff --git a/stl/inc/xthreads.h b/stl/inc/xthreads.h
@@ -7,8 +7,8 @@
 #define _THR_XTHREADS_H
 #include <yvals_core.h>
 #if _STL_COMPILER_PREPROCESSOR
+#include <__msvc_threads_core.hpp>
 #include <climits>
-#include <type_traits>
 #include <xtimec.h>
 
 #pragma pack(push, _CRT_PACKING)
@@ -19,67 +19,6 @@ _STL_DISABLE_CLANG_WARNINGS
 #undef new
 
 extern "C" {
-using _Thrd_id_t = unsigned int;
-struct _Thrd_t { // thread identifier for Win32
-    void* _Hnd; // Win32 HANDLE
-    _Thrd_id_t _Id;
-};
-
-using _Smtx_t = void*;
-
-struct _Stl_critical_section {
-    void* _Unused       = nullptr; // TRANSITION, ABI: was the vptr
-    _Smtx_t _M_srw_lock = nullptr;
-};
-
-struct _Mtx_internal_imp_t {
-#if defined(_CRT_WINDOWS) || defined(UNDOCKED_WINDOWS_UCRT)
-#ifdef _WIN64
-    static constexpr size_t _Critical_section_size = 16;
-#else // ^^^ defined(_WIN64) / !defined(_WIN64) vvv
-    static constexpr size_t _Critical_section_size = 8;
-#endif // ^^^ !defined(_WIN64) ^^^
-#else // ^^^ Windows private STL / public STL vvv
-#ifdef _WIN64
-    static constexpr size_t _Critical_section_size = 64;
-#else // ^^^ defined(_WIN64) / !defined(_WIN64) vvv
-    static constexpr size_t _Critical_section_size = 36;
-#endif // ^^^ !defined(_WIN64) ^^^
-#endif // ^^^ public STL ^^^
-
-    static constexpr size_t _Critical_section_align = alignof(void*);
-
-    int _Type{};
-    union {
-        _Stl_critical_section _Critical_section{};
-        _STD _Aligned_storage_t<_Critical_section_size, _Critical_section_align> _Cs_storage;
-    };
-    long _Thread_id{};
-    int _Count{};
-};
-
-// Size and alignment for _Cnd_internal_imp_t
-#if defined(_CRT_WINDOWS) // for Windows-internal code
-_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 2 * sizeof(void*);
-#elif defined(_WIN64) // ordinary 64-bit code
-_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 72;
-#else // vvv ordinary 32-bit code vvv
-_INLINE_VAR constexpr size_t _Cnd_internal_imp_size = 40;
-#endif // ^^^ ordinary 32-bit code ^^^
-
-_INLINE_VAR constexpr size_t _Cnd_internal_imp_alignment = alignof(void*);
-
-using _Mtx_t = _Mtx_internal_imp_t*;
-
-#ifdef _M_CEE // avoid warning LNK4248: unresolved typeref token for '_Cnd_internal_imp_t'; image may not run
-using _Cnd_t = void*;
-#else // ^^^ defined(_M_CEE) / !defined(_M_CEE) vvv
-struct _Cnd_internal_imp_t;
-using _Cnd_t = _Cnd_internal_imp_t*;
-#endif // ^^^ !defined(_M_CEE) ^^^
-
-enum class _Thrd_result : int { _Success, _Nomem, _Timedout, _Busy, _Error };
-
 // threads
 _CRTIMP2_PURE _Thrd_result __cdecl _Thrd_detach(_Thrd_t) noexcept;
 _CRTIMP2_PURE _Thrd_result __cdecl _Thrd_join(_Thrd_t, int*) noexcept;
@@ -123,12 +62,12 @@ _CRTIMP2_PURE void __cdecl _Cnd_destroy(_Cnd_t) noexcept;
 _CRTIMP2_PURE void __cdecl _Cnd_init_in_situ(_Cnd_t) noexcept;
 _CRTIMP2_PURE void __cdecl _Cnd_destroy_in_situ(_Cnd_t) noexcept;
 _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(_Cnd_t, _Mtx_t) noexcept; // TRANSITION, ABI: Always succeeds
-_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_timedwait(_Cnd_t, _Mtx_t, const _timespec64*) noexcept;
 _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
 _CRTIMP2_PURE void __cdecl _Cnd_register_at_thread_exit(_Cnd_t, _Mtx_t, int*) noexcept;
 _CRTIMP2_PURE void __cdecl _Cnd_unregister_at_thread_exit(_Mtx_t) noexcept;
 _CRTIMP2_PURE void __cdecl _Cnd_do_broadcast_at_thread_exit() noexcept;
+_Thrd_result __stdcall _Cnd_timedwait_for(_Cnd_t, _Mtx_t, unsigned int) noexcept;
 } // extern "C"
 
 _STD_BEGIN

diff --git a/stl/src/cond.cpp b/stl/src/cond.cpp
@@ -3,6 +3,7 @@
 
 #include <cstdlib>
 #include <internal_shared.h>
+#include <new>
 #include <type_traits>
 #include <xthreads.h>
 #include <xtimec.h>
@@ -11,17 +12,9 @@
 
 extern "C" {
 
-struct _Cnd_internal_imp_t {
-    typename std::_Aligned_storage<_Cnd_internal_imp_size, _Cnd_internal_imp_alignment>::type cv;
-
-    [[nodiscard]] Concurrency::details::stl_condition_variable_win7* _get_cv() noexcept {
-        // get pointer to implementation
-        return reinterpret_cast<Concurrency::details::stl_condition_variable_win7*>(&cv);
-    }
-};
 
 _CRTIMP2_PURE void __cdecl _Cnd_init_in_situ(const _Cnd_t cond) noexcept { // initialize condition variable in situ
-    Concurrency::details::create_stl_condition_variable(cond->_get_cv());
+    new (cond->_get_cv()) Concurrency::details::stl_condition_variable_win7;
 }
 
 _CRTIMP2_PURE void __cdecl _Cnd_destroy_in_situ(_Cnd_t) noexcept {} // destroy condition variable in situ
@@ -66,7 +59,7 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx
     return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds
 }
 
-// wait until signaled or timeout
+// TRANSITION, ABI: preserved for compatibility; wait until signaled or timeout
 _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;