From a7e2c93ec43d38ddc3de02188ab5dac6091ba33c Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 14 Aug 2020 04:17:09 -0700 Subject: [PATCH 01/20] Implement `` and `jthread`. Implements P0660R10, P1869R1, resolves GH-32, and VSO-951574. / : Extract a new type _Locked_pointer which implements the low order bit tricks for atomic as a reusable component. yvals_core.h / *_feature_test_macros/test.cpp: Add feature test macro __cpp_lib_jthread and note that the feature is implemented. CMakeLists.txt: Add stop_token header. This will require copy_crt / setup changes in msvc. `` : New header. FIXME This needs a writeup after review. _Stop_callback_base forms a doubly-linked list of callbacks to invoke when stop is requested. It uses a function pointer rather than a virtual function for reduced RTTI codegen costs and to save an indirection of a vtbl access; the compiler being able to devirtualize this case is unlikely. (Using a function pointer rather than virtual was suggested by @lewissbaker ) _Stop_state is the shared state acting as the 'pipe' between stop sources and stop tokens. It has separate reference counts for sources and tokens, and all sources share 1 token count in the same way that all shared_ptrs share one weak count. The stop requested bit is merged into the sources counter to make atomically answering stop_possible possible without tricks. The counters are separate uint32_ts instead of one uint64_t to make operations manipulating them more efficient on 32 bit platforms, even though that means we might have to do a 1-2 more atomic ops on 64 bit platforms in some cases. The tricky part here is primarily how we implement the dtor effects for stop_callback. We can't use a straightforward bool in the stop_callback indicating whether the callback has been called, because once we invoke the callback the memory on which the stop_callback is stored might be gone. To resolve this, we have the thread processing request_stop publish the callback it is currently retiring, so a wait_callback destructor, upon seeing that another thread is executing the callback, can wait until the currently being executed callback is something else. ``: Extract the thread startup machinery from thread's ctor; notably, this introduces another layer of forwards in the constructing thread, but doesn't introduce any additional stack frames in the thread procedure, preserving the debugger experience. Implement jthread with a member thread and stop_source. Note that the standard currently makes the move assignment operator join on self move assign which is surprising; I asked L(E)WG if that is intentional. ``: Implement the new interruptable waits; this code is effectively copied from the standard. I filed a defect report because [thread.condvarany.intwait]/7 depicts a call to a nonexistent variable `cv`. --- stl/CMakeLists.txt | 1 + stl/inc/atomic | 53 +++ stl/inc/condition_variable | 55 +++ stl/inc/memory | 90 +--- stl/inc/stop_token | 398 +++++++++++++++++ stl/inc/thread | 107 ++++- stl/inc/yvals_core.h | 2 + tests/std/include/new_counter.hpp | 76 ++++ tests/std/test.lst | 3 + .../tests/P0660R10_jthread_and_cv_any/env.lst | 4 + .../P0660R10_jthread_and_cv_any/test.cpp | 243 ++++++++++ tests/std/tests/P0660R10_stop_token/env.lst | 4 + tests/std/tests/P0660R10_stop_token/test.cpp | 418 ++++++++++++++++++ .../tests/P0660R10_stop_token_death/env.lst | 4 + .../tests/P0660R10_stop_token_death/test.cpp | 48 ++ .../VSO_0157762_feature_test_macros/test.cpp | 14 + 16 files changed, 1448 insertions(+), 72 deletions(-) create mode 100644 stl/inc/stop_token create mode 100644 tests/std/include/new_counter.hpp create mode 100644 tests/std/tests/P0660R10_jthread_and_cv_any/env.lst create mode 100644 tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp create mode 100644 tests/std/tests/P0660R10_stop_token/env.lst create mode 100644 tests/std/tests/P0660R10_stop_token/test.cpp create mode 100644 tests/std/tests/P0660R10_stop_token_death/env.lst create mode 100644 tests/std/tests/P0660R10_stop_token_death/test.cpp diff --git a/stl/CMakeLists.txt b/stl/CMakeLists.txt index 3314aab65d3..71a99d4051d 100644 --- a/stl/CMakeLists.txt +++ b/stl/CMakeLists.txt @@ -176,6 +176,7 @@ set(HEADERS ${CMAKE_CURRENT_LIST_DIR}/inc/span ${CMAKE_CURRENT_LIST_DIR}/inc/sstream ${CMAKE_CURRENT_LIST_DIR}/inc/stack + ${CMAKE_CURRENT_LIST_DIR}/inc/stop_token ${CMAKE_CURRENT_LIST_DIR}/inc/stdexcept ${CMAKE_CURRENT_LIST_DIR}/inc/streambuf ${CMAKE_CURRENT_LIST_DIR}/inc/string diff --git a/stl/inc/atomic b/stl/inc/atomic index 40a8edc9e93..a34ce217570 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -2970,6 +2970,59 @@ inline void atomic_flag_notify_all(volatile atomic_flag* const _Flag) noexcept { inline void atomic_flag_notify_all(atomic_flag* const _Flag) noexcept { return _Flag->notify_all(); } + +template +class _Locked_pointer { + static_assert(alignof(_Ty) >= (1 << 2), "2 low order bits are needed by _Locked_pointer"); + static constexpr uintptr_t _Lock_mask = 3; + static constexpr uintptr_t _Not_locked = 0; + static constexpr uintptr_t _Locked_notify_not_needed = 1; + static constexpr uintptr_t _Locked_notify_needed = 2; + static constexpr uintptr_t _Ptr_value_mask = ~_Lock_mask; + +public: + [[nodiscard]] _Ty* _Lock_and_load() noexcept { + uintptr_t _Rep = _Storage.load(memory_order_relaxed); + for (;;) { + switch (_Rep & _Lock_mask) { + case _Not_locked: // Can try to lock now + if (_Storage.compare_exchange_weak(_Rep, _Rep | _Locked_notify_not_needed)) { + return reinterpret_cast<_Ty*>(_Rep); + } + _YIELD_PROCESSOR(); + break; + + case _Locked_notify_not_needed: // Try to set "notify needed" and wait + if (!_Storage.compare_exchange_weak(_Rep, (_Rep & _Ptr_value_mask) | _Locked_notify_needed)) { + // Failed to put notify needed flag on, try again + _YIELD_PROCESSOR(); + break; + } + _Rep = (_Rep & _Ptr_value_mask) | _Locked_notify_needed; + [[fallthrough]]; + + case _Locked_notify_needed: // "Notify needed" is already set, just wait + _Storage.wait(_Rep, memory_order_relaxed); + _Rep = _Storage.load(memory_order_relaxed); + break; + + default: // Unrecognized bit pattern + _CSTD abort(); + } + } + } + + void _Store_and_unlock(_Ty* const _Value) noexcept { + uintptr_t _Rep = _Storage.exchange(reinterpret_cast(_Value)); + if ((_Rep & _Lock_mask) == _Locked_notify_needed) { + // As we don't count waiters, every waiter is notified, and then some may re-request notification + _Storage.notify_all(); + } + } + +private: + atomic _Storage; +}; #endif // _HAS_CXX20 _STD_END diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index 0fdd0e13306..82dba8563ae 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -12,6 +12,9 @@ #include #include #include +#if _HAS_CXX20 +#include +#endif // _HAS_CXX20 #pragma pack(push, _CRT_PACKING) #pragma warning(push, _STL_WARNING_LEVEL) @@ -128,6 +131,58 @@ public: return true; } +#if _HAS_CXX20 +private: + struct _Cv_any_notify_all { + condition_variable_any* _This; + + _Cv_any_notify_all(condition_variable_any* _This_) : _This{_This_} {} + + _Cv_any_notify_all(const _Cv_any_notify_all&) = delete; + _Cv_any_notify_all& operator=(const _Cv_any_notify_all&) = delete; + + void operator()() const noexcept { + _This->notify_all(); + } + }; + +public: + template + bool wait_until(_Lock& _Lck, stop_token _Stoken, _Predicate _Pred) { + stop_callback<_Cv_any_notify_all> _Cb{_STD move(_Stoken), this}; + while (!_Stoken.stop_requested()) { + if (_Pred()) { + return true; + } + + wait(_Lck); + } + + return _Pred(); + } + + template + bool wait_until( + _Lock& _Lck, stop_token _Stoken, const chrono::time_point<_Clock, _Duration>& _Abs_time, _Predicate _Pred) { + stop_callback<_Cv_any_notify_all> _Cb{_STD move(_Stoken), this}; + while (!_Stoken.stop_requested()) { + if (_Pred()) { + return true; + } + + if (wait_until(_Lck, _Abs_time) == cv_status::timeout) { + return _Pred(); + } + } + return _Pred(); + } + + template + bool wait_for(_Lock& _Lck, stop_token _Stoken, const chrono::duration<_Rep, _Period>& _Rel_time, _Predicate _Pred) { + return wait_until(_Lck, _STD move(_Stoken), chrono::steady_clock::now() + _Rel_time, _STD move(_Pred)); + } +#endif // _HAS_CXX20 + private: shared_ptr _Myptr; diff --git a/stl/inc/memory b/stl/inc/memory index 69eed6da175..b2d7569efc7 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -3199,65 +3199,17 @@ _CXX20_DEPRECATE_OLD_SHARED_PTR_ATOMIC_SUPPORT bool atomic_compare_exchange_stro template class alignas(2 * sizeof(void*)) _Atomic_ptr_base { // overalignment is to allow potential future use of cmpxchg16b - - static_assert(alignof(_Ref_count_base) >= (1 << 2), "Two bits don't fit as low bits"); - - static constexpr uintptr_t _Lock_mask = 3; - static constexpr uintptr_t _Not_locked = 0; - static constexpr uintptr_t _Locked_notify_not_needed = 1; - static constexpr uintptr_t _Locked_notify_needed = 2; - static constexpr uintptr_t _Ptr_value_mask = ~_Lock_mask; - protected: constexpr _Atomic_ptr_base() noexcept = default; _Atomic_ptr_base(_Ty* const _Px, _Ref_count_base* const _Ref) noexcept : _Ptr(_Px), _Repptr(reinterpret_cast(_Ref)) {} - _NODISCARD _Ref_count_base* _Lock_and_load() const noexcept { - uintptr_t _Rep = _Repptr.load(memory_order_relaxed); - for (;;) { - switch (_Rep & _Lock_mask) { - case _Not_locked: // Can try to lock now - if (_Repptr.compare_exchange_weak(_Rep, _Rep | _Locked_notify_not_needed)) { - return reinterpret_cast<_Ref_count_base*>(_Rep); - } - _YIELD_PROCESSOR(); - break; - - case _Locked_notify_not_needed: // Try to set "notify needed" and wait - if (!_Repptr.compare_exchange_weak(_Rep, (_Rep & _Ptr_value_mask) | _Locked_notify_needed)) { - // Failed to put notify needed flag on, try again - _YIELD_PROCESSOR(); - break; - } - _Rep = (_Rep & _Ptr_value_mask) | _Locked_notify_needed; - [[fallthrough]]; - - case _Locked_notify_needed: // "Notify needed" is already set, just wait - _Repptr.wait(_Rep, memory_order_relaxed); - _Rep = _Repptr.load(memory_order_relaxed); - break; - - default: // Unrecognized bit pattern - _CSTD abort(); - } - } - } - - void _Store_and_unlock(_Ref_count_base* const _Value) const noexcept { - uintptr_t _Rep = _Repptr.exchange(reinterpret_cast(_Value)); - if ((_Rep & _Lock_mask) == _Locked_notify_needed) { - // As we don't count waiters, every waiter is notified, and then some may re-request notification - _Repptr.notify_all(); - } - } - void _Wait(_Ty* _Old, memory_order) const noexcept { for (;;) { - auto _Rep = _Lock_and_load(); + auto _Rep = _Repptr._Lock_and_load(); bool _Equal = _Ptr.load(memory_order_relaxed) == _Old; - _Store_and_unlock(_Rep); + _Repptr._Store_and_unlock(_Rep); if (!_Equal) { break; } @@ -3274,7 +3226,7 @@ protected: } atomic<_Ty*> _Ptr{nullptr}; - mutable atomic _Repptr{0}; + mutable _Locked_pointer<_Ref_count_base> _Repptr; }; template @@ -3293,22 +3245,22 @@ public: void store(shared_ptr<_Ty> _Value, const memory_order _Order = memory_order_seq_cst) noexcept { _Check_store_memory_order(_Order); - const auto _Rep = this->_Lock_and_load(); + const auto _Rep = this->_Repptr._Lock_and_load(); _Ty* const _Tmp = _Value._Ptr; _Value._Ptr = this->_Ptr.load(memory_order_relaxed); this->_Ptr.store(_Tmp, memory_order_relaxed); - this->_Store_and_unlock(_Value._Rep); + this->_Repptr._Store_and_unlock(_Value._Rep); _Value._Rep = _Rep; } _NODISCARD shared_ptr<_Ty> load(const memory_order _Order = memory_order_seq_cst) const noexcept { _Check_load_memory_order(_Order); shared_ptr<_Ty> _Result; - const auto _Rep = this->_Lock_and_load(); + const auto _Rep = this->_Repptr._Lock_and_load(); _Result._Ptr = this->_Ptr.load(memory_order_relaxed); _Result._Rep = _Rep; _Result._Incref(); - this->_Store_and_unlock(_Rep); + this->_Repptr._Store_and_unlock(_Rep); return _Result; } @@ -3319,10 +3271,10 @@ public: shared_ptr<_Ty> exchange(shared_ptr<_Ty> _Value, const memory_order _Order = memory_order_seq_cst) noexcept { _Check_memory_order(_Order); shared_ptr<_Ty> _Result; - _Result._Rep = this->_Lock_and_load(); + _Result._Rep = this->_Repptr._Lock_and_load(); _Result._Ptr = this->_Ptr.load(memory_order_relaxed); this->_Ptr.store(_Value._Ptr, memory_order_relaxed); - this->_Store_and_unlock(_Value._Rep); + this->_Repptr._Store_and_unlock(_Value._Rep); _Value._Ptr = nullptr; // ownership of _Value ref has been given to this, silence decrement _Value._Rep = nullptr; return _Result; @@ -3346,20 +3298,20 @@ public: bool compare_exchange_strong(shared_ptr<_Ty>& _Expected, shared_ptr<_Ty> _Desired, const memory_order _Order = memory_order_seq_cst) noexcept { _Check_memory_order(_Order); - auto _Rep = this->_Lock_and_load(); + auto _Rep = this->_Repptr._Lock_and_load(); if (this->_Ptr.load(memory_order_relaxed) == _Expected._Ptr && _Rep == _Expected._Rep) { _Ty* const _Tmp = _Desired._Ptr; _Desired._Ptr = this->_Ptr.load(memory_order_relaxed); this->_Ptr.store(_Tmp, memory_order_relaxed); _STD swap(_Rep, _Desired._Rep); - this->_Store_and_unlock(_Rep); + this->_Repptr._Store_and_unlock(_Rep); return true; } _Ref_count_base* _Expected_rep = _Expected._Rep; _Expected._Ptr = this->_Ptr.load(memory_order_relaxed); _Expected._Rep = _Rep; _Expected._Incref(); - this->_Store_and_unlock(_Rep); + this->_Repptr._Store_and_unlock(_Rep); if (_Expected_rep) { _Expected_rep->_Decref(); } @@ -3410,22 +3362,22 @@ public: void store(weak_ptr<_Ty> _Value, const memory_order _Order = memory_order_seq_cst) noexcept { _Check_store_memory_order(_Order); - const auto _Rep = this->_Lock_and_load(); + const auto _Rep = this->_Repptr._Lock_and_load(); _Ty* const _Tmp = _Value._Ptr; _Value._Ptr = this->_Ptr.load(memory_order_relaxed); this->_Ptr.store(_Tmp, memory_order_relaxed); - this->_Store_and_unlock(_Value._Rep); + this->_Repptr._Store_and_unlock(_Value._Rep); _Value._Rep = _Rep; } _NODISCARD weak_ptr<_Ty> load(const memory_order _Order = memory_order_seq_cst) const noexcept { _Check_load_memory_order(_Order); weak_ptr<_Ty> _Result; - const auto _Rep = this->_Lock_and_load(); + const auto _Rep = this->_Repptr._Lock_and_load(); _Result._Ptr = this->_Ptr.load(memory_order_relaxed); _Result._Rep = _Rep; _Result._Incwref(); - this->_Store_and_unlock(_Rep); + this->_Repptr._Store_and_unlock(_Rep); return _Result; } @@ -3436,10 +3388,10 @@ public: weak_ptr<_Ty> exchange(weak_ptr<_Ty> _Value, const memory_order _Order = memory_order_seq_cst) noexcept { _Check_memory_order(_Order); weak_ptr<_Ty> _Result; - _Result._Rep = this->_Lock_and_load(); + _Result._Rep = this->_Repptr._Lock_and_load(); _Result._Ptr = this->_Ptr.load(memory_order_relaxed); this->_Ptr.store(_Value._Ptr, memory_order_relaxed); - this->_Store_and_unlock(_Value._Rep); + this->_Repptr._Store_and_unlock(_Value._Rep); _Value._Ptr = nullptr; // ownership of _Value ref has been given to this, silence decrement _Value._Rep = nullptr; return _Result; @@ -3463,20 +3415,20 @@ public: bool compare_exchange_strong( weak_ptr<_Ty>& _Expected, weak_ptr<_Ty> _Desired, const memory_order _Order = memory_order_seq_cst) noexcept { _Check_memory_order(_Order); - auto _Rep = this->_Lock_and_load(); + auto _Rep = this->_Repptr._Lock_and_load(); if (this->_Ptr.load(memory_order_relaxed) == _Expected._Ptr && _Rep == _Expected._Rep) { _Ty* const _Tmp = _Desired._Ptr; _Desired._Ptr = this->_Ptr.load(memory_order_relaxed); this->_Ptr.store(_Tmp, memory_order_relaxed); _STD swap(_Rep, _Desired._Rep); - this->_Store_and_unlock(_Rep); + this->_Repptr._Store_and_unlock(_Rep); return true; } const auto _Expected_rep = _Expected._Rep; _Expected._Ptr = this->_Ptr.load(memory_order_relaxed); _Expected._Rep = _Rep; _Expected._Incwref(); - this->_Store_and_unlock(_Rep); + this->_Repptr._Store_and_unlock(_Rep); if (_Expected_rep) { _Expected_rep->_Decwref(); } diff --git a/stl/inc/stop_token b/stl/inc/stop_token new file mode 100644 index 00000000000..ca3afff39ee --- /dev/null +++ b/stl/inc/stop_token @@ -0,0 +1,398 @@ +// stop_token standard header + +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#pragma once +#ifndef _STOP_TOKEN_ +#define _STOP_TOKEN_ +#include +#if _STL_COMPILER_PREPROCESSOR + +#if !_HAS_CXX20 +#pragma message("The contents of are available only with C++20 or later.") +#else // ^^^ !_HAS_CXX20 / _HAS_CXX20 vvv + +#include +#include +#include + +#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 + +_STD_BEGIN +struct nostopstate_t { + explicit nostopstate_t() = default; +}; + +inline constexpr nostopstate_t nostopstate{}; + +struct _Stop_state; +class stop_token; + +class _Stop_callback_base { + friend _Stop_state; + +public: + using _Callback_fn = void(__cdecl*)(_Stop_callback_base*) _NOEXCEPT_FNPTR; + + explicit _Stop_callback_base(const _Callback_fn _Fn_) noexcept : _Fn{_Fn_} {} + _Stop_callback_base(const _Stop_callback_base&) = delete; + _Stop_callback_base(_Stop_callback_base&&) = delete; + _Stop_callback_base& operator=(const _Stop_callback_base&) = delete; + _Stop_callback_base& operator=(_Stop_callback_base&&) = delete; + + // if _Token is _Stop_requested, calls the callback; + // otherwise, inserts *this into the callback list if stop is possible + inline void _Attach(const stop_token& _Token) noexcept; + inline void _Attach(stop_token&& _Token) noexcept; + + // if *this is in a callback list, removes it + inline void _Detach() noexcept; + +private: + template + void _Do_attach(conditional_t<_Transfer_ownership, _Stop_state*&, _Stop_state* const> _State) noexcept; + +protected: + _Stop_state* _Parent = nullptr; + _Stop_callback_base* _Next = nullptr; + _Stop_callback_base* _Prev = nullptr; + _Callback_fn _Fn; +}; + +struct _Stop_state { + atomic _Stop_tokens = 1; // plus one shared by all stop_sources + atomic _Stop_sources = 2; // plus the low order bit is the stop requested bit + _Locked_pointer<_Stop_callback_base> _Callbacks; + // always uses relaxed operations; ordering provided by the _Callbacks lock + // (atomic just to get wait/notify support) + atomic _Current_callback = nullptr; + _Thrd_id_t _Stopping_thread = 0; + + [[nodiscard]] bool _Stop_requested() const noexcept { + return (_Stop_sources.load() & uint32_t{1}) != 0; + } + + [[nodiscard]] bool _Stop_possible() const noexcept { + return _Stop_sources.load() != 0; + } + + [[nodiscard]] bool _Request_stop() noexcept { + // Attempts to request stop and call callbacks, returns whether request was successful + if ((_Stop_sources.fetch_or(uint32_t{1}) & uint32_t{1}) != 0) { + // another thread already requested + return false; + } + + _Stopping_thread = _Thrd_id(); + for (;;) { + auto _Head = _Callbacks._Lock_and_load(); + _Current_callback.store(_Head, memory_order_relaxed); + _Current_callback.notify_one(); + if (_Head == nullptr) { + _Callbacks._Store_and_unlock(nullptr); + return true; + } + + const auto _Next = _STD exchange(_Head->_Next, nullptr); + _STL_INTERNAL_CHECK(_Head->_Prev == nullptr); + if (_Next != nullptr) { + _Next->_Prev = nullptr; + } + + _Callbacks._Store_and_unlock(_Next); // unlock before running _Head so other registrations + // can detach without blocking on the callback + + _Head->_Fn(_Head); // might destroy *_Head + } + } +}; + +class stop_source; + +class stop_token { + friend stop_source; + friend _Stop_callback_base; + +public: + stop_token() noexcept : _State{} {} + stop_token(const stop_token& _Other) noexcept : _State{_Other._State} { + const auto _Local = _State; + if (_Local != nullptr) { + _Local->_Stop_tokens.fetch_add(1, memory_order_relaxed); + } + } + + stop_token(stop_token&& _Other) noexcept : _State{_STD exchange(_Other._State, nullptr)} {} + stop_token& operator=(const stop_token& _Other) noexcept { + stop_token{_Other}.swap(*this); + return *this; + } + + stop_token& operator=(stop_token&& _Other) noexcept { + stop_token{_STD move(_Other)}.swap(*this); + return *this; + } + + ~stop_token() { + const auto _Local = _State; + if (_Local != nullptr) { + if (_Local->_Stop_tokens.fetch_sub(1, memory_order_acq_rel) == 1) { + delete _Local; + } + } + } + + void swap(stop_token& _Other) noexcept { + _STD swap(_State, _Other._State); + } + + [[nodiscard]] bool stop_requested() const noexcept { + const auto _Local = _State; + return _Local != nullptr && _Local->_Stop_requested(); + } + + [[nodiscard]] bool stop_possible() const noexcept { + const auto _Local = _State; + return _Local != nullptr && _Local->_Stop_possible(); + } + + [[nodiscard]] friend bool operator==(const stop_token& _Lhs, const stop_token& _Rhs) noexcept { + return _Lhs._State == _Rhs._State; + } + + friend void swap(stop_token& _Lhs, stop_token& _Rhs) noexcept { + _STD swap(_Lhs._State, _Rhs._State); + } + +private: + explicit stop_token(_Stop_state* const _State_) : _State{_State_} {} + + _Stop_state* _State; +}; + +class stop_source { +public: + stop_source() : _State{new _Stop_state} {} + explicit stop_source(nostopstate_t) noexcept : _State{} {} + stop_source(const stop_source& _Other) noexcept : _State{_Other._State} { + const auto _Local = _State; + if (_Local != nullptr) { + _Local->_Stop_sources.fetch_add(2, memory_order_relaxed); + } + } + + stop_source(stop_source&& _Other) noexcept : _State{_STD exchange(_Other._State, nullptr)} {} + stop_source& operator=(const stop_source& _Other) noexcept { + stop_source{_Other}.swap(*this); + return *this; + } + + stop_source& operator=(stop_source&& _Other) noexcept { + stop_source{_STD move(_Other)}.swap(*this); + return *this; + } + + ~stop_source() { + const auto _Local = _State; + if (_Local != nullptr) { + if ((_Local->_Stop_sources.fetch_sub(2, memory_order_relaxed) >> 1) == 1) { + if (_Local->_Stop_tokens.fetch_sub(1, memory_order_acq_rel) == 1) { + delete _Local; + } + } + } + } + + void swap(stop_source& _Other) noexcept { + _STD swap(_State, _Other._State); + } + + [[nodiscard]] stop_token get_token() const noexcept { + const auto _Local = _State; + if (_Local != nullptr) { + _Local->_Stop_tokens.fetch_add(1, memory_order_relaxed); + } + + return stop_token{_Local}; + } + + [[nodiscard]] bool stop_requested() const noexcept { + const auto _Local = _State; + return _Local != nullptr && _Local->_Stop_requested(); + } + + [[nodiscard]] bool stop_possible() const noexcept { + return _State != nullptr; + } + + bool request_stop() noexcept { + const auto _Local = _State; + return _Local && _Local->_Request_stop(); + } + + [[nodiscard]] friend bool operator==(const stop_source& _Lhs, const stop_source& _Rhs) noexcept { + return _Lhs._State == _Rhs._State; + } + + friend void swap(stop_source& _Lhs, stop_source& _Rhs) noexcept { + _STD swap(_Lhs._State, _Rhs._State); + } + +private: + _Stop_state* _State; +}; + +template +void _Stop_callback_base::_Do_attach( + conditional_t<_Transfer_ownership, _Stop_state*&, _Stop_state* const> _State_raw) noexcept { + auto _State = _State_raw; // avoid an indirection in all of the below + if (_State == nullptr) { + return; + } + + // fast path check if the state is already known + auto _Local_sources = _State->_Stop_sources.load(); + if ((_Local_sources & uint32_t{1}) != 0) { + // stop already requested + _Fn(this); + return; + } + + if (_Local_sources == 0) { + return; // stop not possible + } + + // fast path doesn't know, so try to insert + auto _Head = _State->_Callbacks._Lock_and_load(); + // recheck the state in case it changed while we were waiting to acquire the lock + _Local_sources = _State->_Stop_sources.load(); + if ((_Local_sources & uint32_t{1}) != 0) { + // stop already requested + _State->_Callbacks._Store_and_unlock(_Head); + _Fn(this); + return; + } + + if (_Local_sources != 0) { + // stop possible, do the insert + _Parent = _State; + _Next = _Head; + if constexpr (_Transfer_ownership) { + _State_raw = nullptr; + } else { + _State->_Stop_tokens.fetch_add(1, memory_order_relaxed); + } + + if (_Head != nullptr) { + _Head->_Prev = this; + } + + _Head = this; + } + + _State->_Callbacks._Store_and_unlock(_Head); +} + +inline void _Stop_callback_base::_Attach(const stop_token& _Token) noexcept { + this->_Do_attach(_Token._State); +} + +inline void _Stop_callback_base::_Attach(stop_token&& _Token) noexcept { + this->_Do_attach(_Token._State); +} + +inline void _Stop_callback_base::_Detach() noexcept { + stop_token _Token{_Parent}; // transfers ownership + if (_Token._State == nullptr) { + // callback was never inserted into the list + return; + } + + auto _Head = _Token._State->_Callbacks._Lock_and_load(); + if (this == _Head) { + // we are still in the list, so the callback is not being request_stop'd + const auto _Local_next = _Next; + if (_Local_next != nullptr) { + _Local_next->_Prev = nullptr; + } + + _STL_INTERNAL_CHECK(_Prev == nullptr); + _Token._State->_Callbacks._Store_and_unlock(_Next); + return; + } + + const auto _Local_prev = _Prev; + if (_Local_prev != nullptr) { + // we are still in the list, so the callback is not being request_stop'd, and there is at least one other + // callback still registered + const auto _Local_next = _Next; + _Next->_Prev = _Local_prev; + _Prev->_Next = _Local_next; + _Token._State->_Callbacks._Store_and_unlock(_Head); + return; + } + + // we aren't in the callback list even though we were added to it, so the stop requesting thread is attempting to + // call the callback + _STL_INTERNAL_CHECK((_Token._State->_Stop_sources.load() & uint32_t{1}) != 0); + if (_Token._State->_Current_callback.load(memory_order_relaxed) != this + || _Token._State->_Stopping_thread == _Thrd_id()) { + // the callback is done or the dtor is being recursively reentered, do not block + _Token._State->_Callbacks._Store_and_unlock(_Head); + return; + } + + // the callback is being executed by another thread, block until it is complete + _Token._State->_Callbacks._Store_and_unlock(_Head); + _Token._State->_Current_callback.wait(this, memory_order_relaxed); +} + +template +class stop_callback : public _Stop_callback_base { +public: + using callback_type = _Callback; + + template , int> = 0> + explicit stop_callback(const stop_token& _Token, _Cb_init_ty&& _Cb_) noexcept( + is_nothrow_constructible_v<_Callback, _Cb_init_ty>) + : _Stop_callback_base{_Invoke_by_stop}, _Cb(_STD forward<_Cb_init_ty>(_Cb_)) { + _Attach(_Token); + } + + template , int> = 0> + explicit stop_callback(stop_token&& _Token, _Cb_init_ty&& _Cb_) noexcept( + is_nothrow_constructible_v<_Callback, _Cb_init_ty>) + : _Stop_callback_base{_Invoke_by_stop}, _Cb(_STD forward<_Cb_init_ty>(_Cb_)) { + _Attach(_STD move(_Token)); + } + + ~stop_callback() { + _Detach(); + } + +private: + static void __cdecl _Invoke_by_stop(_Stop_callback_base* const _This) noexcept // enforces termination + { + _STD forward<_Callback>(static_cast(_This)->_Cb)(); + } + + _Callback _Cb; +}; + +template +stop_callback(stop_token, Callback) -> stop_callback; + +_STD_END +#pragma pop_macro("new") +_STL_RESTORE_CLANG_WARNINGS +#pragma warning(pop) +#pragma pack(pop) +#endif // _HAS_CXX20 +#endif // _STL_COMPILER_PREPROCESSOR +#endif // _STOP_TOKEN_ diff --git a/stl/inc/thread b/stl/inc/thread index 7a1d332d12b..8d64c186f8d 100644 --- a/stl/inc/thread +++ b/stl/inc/thread @@ -13,6 +13,9 @@ #include #include #include +#if _HAS_CXX20 +#include +#endif // _HAS_CXX20 #ifdef _M_CEE_PURE #error is not supported when compiling with /clr:pure. @@ -26,6 +29,10 @@ _STL_DISABLE_CLANG_WARNINGS #undef new _STD_BEGIN +#if _HAS_CXX20 +class jthread; +#endif // _HAS_CXX20 + class thread { // class for observing and managing threads public: class id; @@ -35,6 +42,8 @@ public: thread() noexcept : _Thr{} {} private: + friend jthread; + template static unsigned int __stdcall _Invoke(void* _RawVals) noexcept /* terminates */ { // adapt invoke of user's callable object to _beginthreadex's thread procedure @@ -50,9 +59,8 @@ private: return &_Invoke<_Tuple, _Indices...>; } -public: - template , thread>, int> = 0> - explicit thread(_Fn&& _Fx, _Args&&... _Ax) { + template + void _Start(_Fn&& _Fx, _Args&&... _Ax) { using _Tuple = tuple, decay_t<_Args>...>; auto _Decay_copied = _STD make_unique<_Tuple>(_STD forward<_Fn>(_Fx), _STD forward<_Args>(_Ax)...); constexpr auto _Invoker_proc = _Get_invoke<_Tuple>(make_index_sequence<1 + sizeof...(_Args)>{}); @@ -73,6 +81,12 @@ public: } } +public: + template , thread>, int> = 0> + explicit thread(_Fn&& _Fx, _Args&&... _Ax) { + _Start(_STD forward<_Fn>(_Fx), _STD forward<_Args>(_Ax)...); + } + ~thread() noexcept { if (joinable()) { _STD terminate(); @@ -240,6 +254,93 @@ struct hash { return _Hash_representation(_Keyval._Id); } }; + +#if _HAS_CXX20 +class jthread { +public: + using id = thread::id; + using native_handle_type = thread::native_handle_type; + + jthread() noexcept : _Impl{}, _Ssource{nostopstate} {} + + template , jthread>, int> = 0> + explicit jthread(_Fn&& _Fx, _Args&&... _Ax) { + if constexpr (is_invocable_v, stop_token, decay_t<_Args>...>) { + _Impl._Start(_STD forward<_Fn>(_Fx), _Ssource.get_token(), _STD forward<_Args>(_Ax)...); + } else { + _Impl._Start(_STD forward<_Fn>(_Fx), _STD forward<_Args>(_Ax)...); + } + } + + ~jthread() { + _Try_cancel_and_join(); + } + + jthread(const jthread&) = delete; + jthread(jthread&& _Other) noexcept = default; + jthread& operator=(const jthread&) = delete; + + jthread& operator=(jthread&& _Other) noexcept { + // note: the standard specifically disallows making self-move-assignment a no-op here + _Try_cancel_and_join(); + _Impl = _STD move(_Other._Impl); + _Ssource = _STD move(_Other._Ssource); + return *this; + } + + void swap(jthread& _Other) noexcept { + _Impl.swap(_Other._Impl); + _Ssource.swap(_Other._Ssource); + } + + [[nodiscard]] bool joinable() const noexcept { + return _Impl.joinable(); + } + + void join() { + _Impl.join(); + } + + void detach() { + _Impl.detach(); + } + + id get_id() const noexcept { + return _Impl.get_id(); + } + + [[nodiscard]] stop_source get_stop_source() noexcept { + return _Ssource; + } + + [[nodiscard]] stop_token get_stop_token() const noexcept { + return _Ssource.get_token(); + } + + bool request_stop() noexcept { + return _Ssource.request_stop(); + } + + friend void swap(jthread& _Lhs, jthread& _Rhs) noexcept { + _Lhs.swap(_Rhs); + } + + [[nodiscard]] static unsigned int hardware_concurrency() noexcept { + return thread::hardware_concurrency(); + } + +private: + void _Try_cancel_and_join() noexcept { + if (_Impl.joinable()) { + _Ssource.request_stop(); + _Impl.join(); + } + } + + thread _Impl; + stop_source _Ssource; +}; +#endif // _HAS_CXX20 _STD_END #pragma pop_macro("new") diff --git a/stl/inc/yvals_core.h b/stl/inc/yvals_core.h index 3043b0eec62..cb2f0e72ed8 100644 --- a/stl/inc/yvals_core.h +++ b/stl/inc/yvals_core.h @@ -159,6 +159,7 @@ // P0646R1 list/forward_list remove()/remove_if()/unique() Return size_type // P0653R2 to_address() // P0655R1 visit() +// P0661R10 and jthread // P0674R1 make_shared() For Arrays // P0718R2 atomic>, atomic> // P0758R1 is_nothrow_convertible @@ -1185,6 +1186,7 @@ #define __cpp_lib_interpolate 201902L #define __cpp_lib_is_constant_evaluated 201811L #define __cpp_lib_is_nothrow_convertible 201806L +#define __cpp_lib_jthread 201911L #define __cpp_lib_list_remove_return_type 201806L #define __cpp_lib_math_constants 201907L #define __cpp_lib_remove_cvref 201711L diff --git a/tests/std/include/new_counter.hpp b/tests/std/include/new_counter.hpp new file mode 100644 index 00000000000..c7d79861f30 --- /dev/null +++ b/tests/std/include/new_counter.hpp @@ -0,0 +1,76 @@ +#ifdef _STD_TEST_NEW_COUNTER +#error new_counter.hpp defines non-inline functions and thus must be used only once. +#else // ^^^ _STD_TEST_NEW_COUNTER // !_STD_TEST_NEW_COUNTER vvv +#define _STD_TEST_NEW_COUNTER +#endif // ^^^ !_STD_TEST_NEW_COUNTER + +#include +#include +#include + +#pragma once +#pragma warning(push) +#pragma warning(disable : 28251) // Inconsistent annotation for 'new': this instance has no annotations. + +namespace std_testing { + size_t g_total_news = 0; + size_t g_total_deletes = 0; + size_t g_maximum_news = 0; + + void reset_new_counters(size_t new_maximum_news) { + assert(g_total_news == g_total_deletes); + g_total_news = 0; + g_total_deletes = 0; + g_maximum_news = new_maximum_news; + } +} // namespace std_testing + +void* operator new(size_t size) { + void* const p = ::operator new(size, std::nothrow); + if (p == nullptr) { + throw std::bad_alloc(); + } + + return p; +} + +void* operator new(size_t size, const std::nothrow_t&) noexcept { + if (std_testing::g_total_news == std_testing::g_maximum_news) { + return nullptr; + } + if (size == 0) { + ++size; + } + ++std_testing::g_total_news; + return malloc(size); +} + +void operator delete(void* ptr) noexcept { + ::operator delete(ptr, std::nothrow); +} + +void operator delete(void* ptr, const std::nothrow_t&) noexcept { + if (ptr) { + ++std_testing::g_total_deletes; + assert(std_testing::g_total_deletes <= std_testing::g_total_news); + free(ptr); + } +} + +void* operator new[](size_t size) { + return ::operator new(size); +} + +void* operator new[](size_t size, const std::nothrow_t&) noexcept { + return ::operator new(size, std::nothrow); +} + +void operator delete[](void* ptr) noexcept { + ::operator delete(ptr); +} + +void operator delete[](void* ptr, const std::nothrow_t&) noexcept { + ::operator delete(ptr, std::nothrow); +} + +#pragma warning(pop) diff --git a/tests/std/test.lst b/tests/std/test.lst index f749fbff8d7..7e52c433da8 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -235,6 +235,9 @@ tests\P0595R2_is_constant_evaluated tests\P0607R0_inline_variables tests\P0616R0_using_move_in_numeric tests\P0631R8_numbers_math_constants +tests\P0660R10_jthread_and_cv_any +tests\P0660R10_stop_token +tests\P0660R10_stop_token_death tests\P0674R1_make_shared_for_arrays tests\P0718R2_atomic_smart_ptrs tests\P0758R1_is_nothrow_convertible diff --git a/tests/std/tests/P0660R10_jthread_and_cv_any/env.lst b/tests/std/tests/P0660R10_jthread_and_cv_any/env.lst new file mode 100644 index 00000000000..642f530ffad --- /dev/null +++ b/tests/std/tests/P0660R10_jthread_and_cv_any/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_latest_matrix.lst diff --git a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp new file mode 100644 index 00000000000..923c6f41224 --- /dev/null +++ b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp @@ -0,0 +1,243 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include +#include + +#define STATIC_ASSERT(...) static_assert(__VA_ARGS__, #__VA_ARGS__) + +#ifdef __clang__ +#pragma clang diagnostic ignored "-Wself-move" +#endif // __clang__ + +using namespace std; + +STATIC_ASSERT(is_same_v); +STATIC_ASSERT(is_same_v); + +int main() { + // dtor tested in lots of places here so no explicit tests for it + + { // default ctor + jthread default_constructed; + assert(default_constructed.get_id() == thread::id{}); + assert(!default_constructed.get_stop_source().stop_possible()); + } + + { // initializing ctor, traditional functor + jthread worker{[] {}}; + assert(worker.get_id() != thread::id{}); + assert(worker.joinable()); + assert(worker.get_stop_source().stop_possible()); + } + + { // also make sure that we don't delegate to std::thread's constructor which would try to move assign over the + // std::thread inside jthread rather than passing it to the functor + jthread worker{[](thread t) { t.join(); }, thread{[] {}}}; + assert(worker.get_id() != thread::id{}); + assert(worker.joinable()); + assert(worker.get_stop_source().stop_possible()); + } + + { // initializing ctor, token functor + bool called = false; + struct overload_detector { + bool* p_called; + void operator()(stop_token, int i) const { + assert(i == 1729); + *p_called = true; + } + void operator()(int) const { + assert(false); + } + }; + + { + jthread worker{overload_detector{&called}, 1729}; + (void) worker; + } + + assert(called); + } + + { // move ctor + jthread worker{[] {}}; + auto worker_source = worker.get_stop_source(); + { + jthread moved{move(worker)}; + assert(moved.get_stop_source() == worker_source); + assert(moved.joinable()); + assert(worker.get_stop_source() != worker_source); + assert(!worker.joinable()); + } + } + + { // move assign + jthread worker_a{[] {}}; + auto source_a = worker_a.get_stop_source(); + jthread worker_b{[] {}}; + auto id_b = worker_b.get_id(); + auto source_b = worker_b.get_stop_source(); + worker_a = move(worker_b); + assert(source_a.stop_requested()); + assert(id_b == worker_a.get_id()); + assert(!source_b.stop_requested()); + assert(worker_a.get_stop_source() == source_b); + assert(!worker_b.joinable()); + } + + { // self move assign, currently specified to try to cancel and join + jthread worker{[] {}}; + auto source = worker.get_stop_source(); + worker = move(worker); + assert(!worker.joinable()); + assert(source.stop_requested()); + } + + { // swaps + jthread worker_a{[] {}}; + auto id_a = worker_a.get_id(); + auto source_a = worker_a.get_stop_source(); + auto token_a = worker_a.get_stop_token(); + jthread worker_b{[] {}}; + auto id_b = worker_b.get_id(); + auto source_b = worker_b.get_stop_source(); + auto token_b = worker_b.get_stop_token(); + + assert(id_a != id_b); + assert(source_a != source_b); + assert(token_a != token_b); + + worker_a.swap(worker_b); + assert(worker_a.get_id() == id_b); + assert(worker_a.get_stop_source() == source_b); + assert(worker_b.get_id() == id_a); + assert(worker_b.get_stop_source() == source_a); + swap(worker_a, worker_b); + assert(worker_a.get_id() == id_a); + assert(worker_a.get_stop_source() == source_a); + assert(worker_b.get_id() == id_b); + assert(worker_b.get_stop_source() == source_b); + } + + { // join + jthread worker{[] {}}; + auto source = worker.get_stop_source(); + worker.join(); + assert(!worker.joinable()); + assert(worker.get_stop_source() == source); + assert(!source.stop_requested()); + assert(source.stop_possible()); + } + + // TRANSITION, MSFT-11107628 "_Exit allows cleanup in other DLLs" + // detach() is intentionally not tested + + // get_id, get_stop_source, get_stop_token tested above + + assert(jthread::hardware_concurrency() == thread::hardware_concurrency()); + + { // first wait_until overload; without the cancellation this would deadlock + jthread worker([](stop_token token) { + mutex m; + condition_variable_any cv; + unique_lock lck{m}; + assert(cv.wait_until(lck, move(token), [] { return false; }) == false); + }); + } + + { // ditto without the cancellation this would deadlock + jthread worker([](stop_token token) { + mutex m; + condition_variable_any cv; + unique_lock lck{m}; + assert(cv.wait_until(lck, move(token), chrono::steady_clock::time_point::max(), [] { return false; }) + == false); + }); + } + + { // ditto without the cancellation this would deadlock + jthread worker([](stop_token token) { + mutex m; + condition_variable_any cv; + unique_lock lck{m}; + assert(cv.wait_for(lck, move(token), chrono::steady_clock::duration::max(), [] { return false; }) == false); + }); + } + + // smoke test true-returning versions of the above + { + mutex m; + condition_variable_any cv; + bool b = false; + jthread worker([&](stop_token token) { + unique_lock lck{m}; + assert(cv.wait_until(lck, move(token), [] { return true; }) == true); + assert(cv.wait_until(lck, move(token), [&] { return b; }) == true); + }); + + { + lock_guard lck{m}; + b = true; + } + + cv.notify_all(); + } + + { + mutex m; + condition_variable_any cv; + bool b = false; + jthread worker([&](stop_token token) { + unique_lock lck{m}; + assert( + cv.wait_until(lck, move(token), chrono::steady_clock::time_point::max(), [] { return true; }) == true); + assert(cv.wait_until(lck, move(token), chrono::steady_clock::time_point::max(), [&] { return b; }) == true); + }); + + { + lock_guard lck{m}; + b = true; + } + + cv.notify_all(); + } + + { + mutex m; + condition_variable_any cv; + bool b = false; + jthread worker([&](stop_token token) { + unique_lock lck{m}; + assert(cv.wait_for(lck, move(token), chrono::steady_clock::duration::max(), [] { return true; }) == true); + assert(cv.wait_for(lck, move(token), chrono::steady_clock::duration::max(), [&] { return b; }) == true); + }); + + { + lock_guard lck{m}; + b = true; + } + + cv.notify_all(); + } + + // smoke test a timeout case: + { + jthread worker([] { + stop_source never_stopped; + mutex m; + condition_variable_any cv; + unique_lock lck{m}; + auto started_at = chrono::steady_clock::now(); + assert(cv.wait_for(lck, never_stopped.get_token(), 100ms, [] { return false; }) == false); + // not a timing assumption: the wait_for must wait at least that long + assert(started_at + 100ms <= chrono::steady_clock::now()); + }); + } + + puts("pass"); +} diff --git a/tests/std/tests/P0660R10_stop_token/env.lst b/tests/std/tests/P0660R10_stop_token/env.lst new file mode 100644 index 00000000000..642f530ffad --- /dev/null +++ b/tests/std/tests/P0660R10_stop_token/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_latest_matrix.lst diff --git a/tests/std/tests/P0660R10_stop_token/test.cpp b/tests/std/tests/P0660R10_stop_token/test.cpp new file mode 100644 index 00000000000..b27d8e6751d --- /dev/null +++ b/tests/std/tests/P0660R10_stop_token/test.cpp @@ -0,0 +1,418 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +using namespace std; +using namespace std_testing; + +#ifdef __clang__ +#pragma clang diagnostic ignored "-Wself-move" +#endif // __clang__ + +struct throwing_construction_functor { + throwing_construction_functor(int x) { + throw x; + } + + void operator()() const { + assert(false); + } +}; + +struct call_counting_functor { + atomic* state; + + call_counting_functor(atomic* state_) : state(state_) {} + + call_counting_functor(const call_counting_functor&) = delete; + call_counting_functor& operator=(const call_counting_functor&) = delete; + + void operator()() && { + ++*state; + } +}; + +struct cb_destroying_functor { + optional>& owner; + cb_destroying_functor(optional>& owner_) : owner(owner_) {} + + cb_destroying_functor(const cb_destroying_functor&) = delete; + cb_destroying_functor& operator=(const cb_destroying_functor&) = delete; + + void operator()() && { + owner.reset(); + } +}; + +int main() noexcept { + reset_new_counters(0); + { // all the following must not allocate and work with a nostopstate source; in rough synopsis order + stop_token token; + stop_token token_copy{token}; + stop_token token_moved{move(token)}; + token_copy = token; + token_moved = move(token); + token.swap(token_copy); + assert(!token.stop_requested()); + assert(!token.stop_possible()); + assert(token == token_copy); + swap(token, token_copy); + + stop_source source{nostopstate}; + stop_source copied_source{source}; + stop_source moved_source{move(source)}; + copied_source = source; + moved_source = move(source); + copied_source.swap(source); + + assert(!source.get_token().stop_possible()); + assert(!source.get_token().stop_requested()); + assert(!source.stop_possible()); + assert(!source.stop_requested()); + assert(!source.request_stop()); + + assert(source == copied_source); + assert(source == moved_source); + + swap(source, copied_source); + + stop_callback cb{token, [] { assert(false); }}; + stop_callback cb_moved{move(token), [] { assert(false); }}; + } + + // normal reference counted things state management; in rough synopsis order + reset_new_counters(2); + { // stop_source + stop_source empty{nostopstate}; + + // default ctor + stop_source source_a; + assert(source_a.stop_possible()); + assert(!source_a.stop_requested()); + stop_source source_b; + assert(source_b.stop_possible()); + assert(!source_b.stop_requested()); + assert(source_a != empty); + assert(source_a != source_b); + + // copy ctor + stop_source copied_source{source_a}; + assert(copied_source == source_a); + source_a.swap(source_b); + assert(copied_source == source_b); + swap(source_a, source_b); + assert(copied_source == source_a); + + // move ctor + stop_source moved_source{move(source_a)}; + assert(!source_a.stop_possible()); + assert(empty == source_a); + assert(moved_source != source_a); + moved_source = move(moved_source); + swap(moved_source, source_a); + + // copy assignment + copied_source = source_b; + assert(copied_source == source_b); + + // move assignment + moved_source = move(source_a); + assert(!source_a.stop_possible()); + assert(moved_source.stop_possible()); + + // swap member + moved_source.swap(source_a); + assert(source_a.stop_possible()); + assert(!moved_source.stop_possible()); + + // get_token tested with tokens below + // stop_possible tested above + // stop_requested tested below + assert(!empty.request_stop()); + assert(source_a.request_stop()); + assert(source_a.stop_requested()); + assert(!source_a.request_stop()); + assert(source_a.stop_requested()); + + assert(!source_b.stop_requested()); + assert(!copied_source.stop_requested()); + assert(copied_source.request_stop()); + assert(source_b.stop_requested()); + assert(copied_source.stop_requested()); + assert(!source_b.request_stop()); + } + + reset_new_counters(2); + { // stop_token + stop_source source_a; + stop_token token_a = source_a.get_token(); + assert(token_a.stop_possible()); + assert(!token_a.stop_requested()); + + stop_source source_b; + stop_token token_b = source_b.get_token(); + assert(token_a != token_b); + + stop_token empty; + + // default ctor tested above in the no-alloc block + + // copy ctor + stop_token copied_token{token_a}; + assert(copied_token == token_a); + + // move ctor + stop_token moved_token{move(token_a)}; + assert(moved_token == copied_token); + assert(moved_token != token_a); + assert(!token_a.stop_possible()); + assert(!token_a.stop_requested()); + moved_token.swap(token_a); + + // copy assign + copied_token = token_b; + assert(copied_token == token_b); + + // move assign + moved_token = move(token_b); + assert(moved_token == copied_token); + moved_token = move(moved_token); + assert(moved_token == copied_token); + assert(moved_token != token_a); + assert(!token_b.stop_possible()); + assert(!token_b.stop_requested()); + swap(token_b, moved_token); + + // stop_possible tested above and 1 special case below + + // stop_requested + assert(!copied_token.stop_requested()); + assert(source_b.request_stop()); + assert(!token_a.stop_requested()); + assert(token_b.stop_requested()); + assert(copied_token.stop_requested()); + + // equals and swap tested above + } + + // the stop_possible special cases + reset_new_counters(1); + { // all sources are gone + stop_token token; + { + stop_source source; + token = source.get_token(); + assert(token.stop_possible()); + assert(!token.stop_requested()); + } // destroy source + + assert(!token.stop_possible()); + assert(!token.stop_requested()); + stop_callback cb{token, [] { assert(false); }}; + (void) cb; + } + + reset_new_counters(1); + { // all sources are gone but stop happened first + stop_token token; + { + stop_source source; + token = source.get_token(); + assert(token.stop_possible()); + assert(!token.stop_requested()); + assert(source.request_stop()); + assert(token.stop_possible()); + assert(token.stop_requested()); + } // destroy source + + assert(token.stop_possible()); + assert(token.stop_requested()); + } + + // empty assign special cases + reset_new_counters(1); + { + stop_source source; + stop_source empty{nostopstate}; + source = empty; // lvalue + assert(!source.stop_possible()); + } + + reset_new_counters(1); + { + stop_source source; + source = stop_source{nostopstate}; // rvalue + assert(!source.stop_possible()); + } + + reset_new_counters(1); + { + stop_source source; + auto token = source.get_token(); + stop_token empty; + token = empty; // lvalue + assert(!token.stop_possible()); + } + + reset_new_counters(1); + { + stop_source source; + auto token = source.get_token(); + token = stop_token{}; // rvalue + assert(!token.stop_possible()); + } + + // callback calling in the ctor + reset_new_counters(1); + { + atomic calls{0}; + stop_source source; + source.request_stop(); + assert(calls.load() == 0); + stop_callback cb{source.get_token(), &calls}; + (void) cb; + assert(calls.load() == 1); + } + + reset_new_counters(1); + { + atomic calls{0}; + stop_token token; + + { + stop_source source; + token = source.get_token(); + source.request_stop(); + } // destroy source + + assert(calls.load() == 0); + stop_callback cb{token, &calls}; + (void) cb; + assert(calls.load() == 1); + } + + // callback calling on cancel + reset_new_counters(1); + { + atomic calls{0}; + stop_source source; + assert(calls.load() == 0); + stop_callback cb{source.get_token(), &calls}; + assert(calls.load() == 0); + source.request_stop(); + assert(calls.load() == 1); + } + + // if the callback is executing on the current thread it does not block for the callback to finish executing + reset_new_counters(1); + { + stop_source source; + auto token = source.get_token(); + optional> cb; + cb.emplace(token, cb); + source.request_stop(); // if we don't do what the standard says, this will deadlock + } + + // if the callback is executing on another thread it blocks for the callback to finish executing + reset_new_counters(2); // nonstandard assumption that our std::thread allocates exactly once + { + static constexpr chrono::milliseconds callback_wait_length = 5s; + static constexpr chrono::milliseconds request_wait_length = 500ms; + stop_source source; + atomic block_request_stop{false}; + // block_destroy makes it more likely that the timing assumption above is correct because the timer doesn't + // start until we know the worker thread is actively running trying to request_stop + atomic block_destroy{false}; + std::thread worker{[&] { + // run the callbacks in the worker thread + block_request_stop.wait(false); + block_destroy.store(true); + block_destroy.notify_one(); + assert("request_wait_length TIMING ASSUMPTION" && source.request_stop()); + }}; + + + auto worker_id = worker.get_id(); + chrono::steady_clock::time_point started_at; + { + // timing assumption that the main thread will try to destroy cb within request_wait_length + stop_callback cb{source.get_token(), [&] { + this_thread::sleep_for(callback_wait_length); + assert("request_wait_length TIMING ASSUMPTION" && this_thread::get_id() == worker_id); + }}; + started_at = chrono::steady_clock::now(); + block_request_stop.store(true); + block_request_stop.notify_one(); + block_destroy.wait(false); // wait for the other thread to start stopping + // timing assumption that worker enters request_stop before we try to destroy cb here; + // if that assumption is wrong then we merely don't test the case in which we're interested (because cb will + // run on this thread so we won't have to block for destruction) + this_thread::sleep_for(request_wait_length); + assert("request_wait_length TIMING ASSUMPTION" && !source.request_stop()); + } // destroy cb + + worker.join(); + + // not a timing assumption: we must have waited at least as long as the sleep_for in the cancellation callback + // (that's the point of this test) + auto stopped_at = chrono::steady_clock::now(); + assert(started_at + callback_wait_length <= stopped_at); + } + + // more than one callback in the list and the first callback unregisters one of the others + // (this tests edge cases in the callback linked list management) + for (int idx = 0; idx < 5; ++idx) { + reset_new_counters(1); + stop_source source; + auto token = source.get_token(); + optional> cbs[5]; + cbs[0].emplace(token, cbs[idx]); + cbs[1].emplace(token, cbs[1]); + cbs[2].emplace(token, cbs[2]); + cbs[3].emplace(token, cbs[3]); + cbs[4].emplace(token, cbs[4]); + cbs[2].reset(); + source.request_stop(); + } + + // exception safety cases + reset_new_counters(0); + try { + stop_source source; + (void) source; + assert(false); + } catch (const bad_alloc&) { + // expected + } + + reset_new_counters(1); + try { + stop_source source; + stop_callback cb{source.get_token(), 42}; + } catch (int i) { + assert(i == 42); + } + + reset_new_counters(1); + try { + stop_source source; + auto token_lvalue = source.get_token(); + stop_callback cb{token_lvalue, 43}; + } catch (int i) { + assert(i == 43); + } + + reset_new_counters(0); + + puts("pass"); +} diff --git a/tests/std/tests/P0660R10_stop_token_death/env.lst b/tests/std/tests/P0660R10_stop_token_death/env.lst new file mode 100644 index 00000000000..e5b00aee0d4 --- /dev/null +++ b/tests/std/tests/P0660R10_stop_token_death/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_latest_winsdk_matrix.lst diff --git a/tests/std/tests/P0660R10_stop_token_death/test.cpp b/tests/std/tests/P0660R10_stop_token_death/test.cpp new file mode 100644 index 00000000000..e9075218b7a --- /dev/null +++ b/tests/std/tests/P0660R10_stop_token_death/test.cpp @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include + +#include + +using namespace std; + +struct throwing_functor { + void operator()() { + throw 42; + } +}; + +void test_case_throw_during_callback_ctor() { + stop_source source; + source.request_stop(); + stop_callback cb{source.get_token(), throwing_functor{}}; + (void) cb; +} + +void test_case_throw_during_callback_lvalue_ctor() { + stop_source source; + source.request_stop(); + auto lvalue_token = source.get_token(); + stop_callback cb{lvalue_token, throwing_functor{}}; + (void) cb; +} + +void test_case_throw_during_request_stop() { + stop_source source; + stop_callback cb{source.get_token(), throwing_functor{}}; + (void) cb; + source.request_stop(); +} + +int main(int argc, char* argv[]) { + std_testing::death_test_executive exec([] {}); + + exec.add_death_tests({ + test_case_throw_during_request_stop, + test_case_throw_during_callback_lvalue_ctor, + test_case_throw_during_request_stop, + }); + + return exec.run(argc, argv); +} diff --git a/tests/std/tests/VSO_0157762_feature_test_macros/test.cpp b/tests/std/tests/VSO_0157762_feature_test_macros/test.cpp index c3eaa1b5777..b5534b29293 100644 --- a/tests/std/tests/VSO_0157762_feature_test_macros/test.cpp +++ b/tests/std/tests/VSO_0157762_feature_test_macros/test.cpp @@ -826,6 +826,20 @@ STATIC_ASSERT(__cpp_lib_is_nothrow_convertible == 201806L); #endif #endif +#if _HAS_CXX20 +#ifndef __cpp_lib_jthread +#error __cpp_lib_jthread is not defined +#elif __cpp_lib_jthread != 201911L +#error __cpp_lib_jthread is not 201911L +#else +STATIC_ASSERT(__cpp_lib_jthread == 201911L); +#endif +#else +#ifdef __cpp_lib_jthread +#error __cpp_lib_jthread is defined +#endif +#endif + #ifndef __cpp_lib_is_null_pointer #error __cpp_lib_is_null_pointer is not defined #elif __cpp_lib_is_null_pointer != 201309L From c8b2db35a6276f6c2037e23aa4ccafd1018cc2b9 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 14 Aug 2020 04:37:07 -0700 Subject: [PATCH 02/20] Bill didn't name wait correctly. --- stl/inc/condition_variable | 2 +- tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index 82dba8563ae..cebb4e7b8ee 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -148,7 +148,7 @@ private: public: template - bool wait_until(_Lock& _Lck, stop_token _Stoken, _Predicate _Pred) { + bool wait(_Lock& _Lck, stop_token _Stoken, _Predicate _Pred) { stop_callback<_Cv_any_notify_all> _Cb{_STD move(_Stoken), this}; while (!_Stoken.stop_requested()) { if (_Pred()) { diff --git a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp index 923c6f41224..baa12b2ffb7 100644 --- a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp +++ b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp @@ -146,7 +146,7 @@ int main() { mutex m; condition_variable_any cv; unique_lock lck{m}; - assert(cv.wait_until(lck, move(token), [] { return false; }) == false); + assert(cv.wait(lck, move(token), [] { return false; }) == false); }); } @@ -176,8 +176,8 @@ int main() { bool b = false; jthread worker([&](stop_token token) { unique_lock lck{m}; - assert(cv.wait_until(lck, move(token), [] { return true; }) == true); - assert(cv.wait_until(lck, move(token), [&] { return b; }) == true); + assert(cv.wait(lck, move(token), [] { return true; }) == true); + assert(cv.wait(lck, move(token), [&] { return b; }) == true); }); { From f0554d2826078fab68a13a41a5f09474f1ecabd1 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 14 Aug 2020 05:02:47 -0700 Subject: [PATCH 03/20] Fix deadlock found by tests that came with the reference implementation. --- stl/inc/condition_variable | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index cebb4e7b8ee..29d5d92fab5 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -149,7 +149,7 @@ private: public: template bool wait(_Lock& _Lck, stop_token _Stoken, _Predicate _Pred) { - stop_callback<_Cv_any_notify_all> _Cb{_STD move(_Stoken), this}; + stop_callback<_Cv_any_notify_all> _Cb{_Stoken, this}; while (!_Stoken.stop_requested()) { if (_Pred()) { return true; @@ -164,7 +164,7 @@ public: template bool wait_until( _Lock& _Lck, stop_token _Stoken, const chrono::time_point<_Clock, _Duration>& _Abs_time, _Predicate _Pred) { - stop_callback<_Cv_any_notify_all> _Cb{_STD move(_Stoken), this}; + stop_callback<_Cv_any_notify_all> _Cb{_Stoken, this}; while (!_Stoken.stop_requested()) { if (_Pred()) { return true; From a1886a0393af81853ed0457fabe09bbd47587f27 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 14 Aug 2020 05:08:56 -0700 Subject: [PATCH 04/20] Fix a nullptr dereference found by tests from the reference implementation. --- stl/inc/stop_token | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/stl/inc/stop_token b/stl/inc/stop_token index ca3afff39ee..8d8088e77bb 100644 --- a/stl/inc/stop_token +++ b/stl/inc/stop_token @@ -332,8 +332,11 @@ inline void _Stop_callback_base::_Detach() noexcept { // we are still in the list, so the callback is not being request_stop'd, and there is at least one other // callback still registered const auto _Local_next = _Next; - _Next->_Prev = _Local_prev; - _Prev->_Next = _Local_next; + if (_Local_next != nullptr) { + _Next->_Prev = _Local_prev; + } + + _Prev->_Next = _Local_next; _Token._State->_Callbacks._Store_and_unlock(_Head); return; } From 8b26e2a32743c3343cbacb1e4a27340150137739 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 17 Aug 2020 15:03:21 -0700 Subject: [PATCH 05/20] Fix small CR nitpicks and back off on memory_order aggressiveness. --- stl/inc/atomic | 11 ++++++-- stl/inc/condition_variable | 19 ++++++------- stl/inc/memory | 3 +-- stl/inc/stop_token | 55 +++++++++++++++++--------------------- stl/inc/thread | 12 ++++----- 5 files changed, 51 insertions(+), 49 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index a34ce217570..d14abf902b7 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -2973,6 +2973,7 @@ inline void atomic_flag_notify_all(atomic_flag* const _Flag) noexcept { template class _Locked_pointer { +public: static_assert(alignof(_Ty) >= (1 << 2), "2 low order bits are needed by _Locked_pointer"); static constexpr uintptr_t _Lock_mask = 3; static constexpr uintptr_t _Not_locked = 0; @@ -2981,7 +2982,13 @@ class _Locked_pointer { static constexpr uintptr_t _Ptr_value_mask = ~_Lock_mask; public: - [[nodiscard]] _Ty* _Lock_and_load() noexcept { + constexpr _Locked_pointer() noexcept : _Storage{} {} + constexpr _Locked_pointer(_Ty* _Ptr) noexcept : _Storage{reinterpret_cast(_Ptr)} {} + + _Locked_pointer(const _Locked_pointer&) = delete; + _Locked_pointer& operator=(const _Locked_pointer&) = delete; + + _NODISCARD _Ty* _Lock_and_load() noexcept { uintptr_t _Rep = _Storage.load(memory_order_relaxed); for (;;) { switch (_Rep & _Lock_mask) { @@ -3013,7 +3020,7 @@ public: } void _Store_and_unlock(_Ty* const _Value) noexcept { - uintptr_t _Rep = _Storage.exchange(reinterpret_cast(_Value)); + const auto _Rep = _Storage.exchange(reinterpret_cast(_Value)); if ((_Rep & _Lock_mask) == _Locked_notify_needed) { // As we don't count waiters, every waiter is notified, and then some may re-request notification _Storage.notify_all(); diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index 29d5d92fab5..f959340727b 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -150,31 +150,32 @@ public: template bool wait(_Lock& _Lck, stop_token _Stoken, _Predicate _Pred) { stop_callback<_Cv_any_notify_all> _Cb{_Stoken, this}; - while (!_Stoken.stop_requested()) { - if (_Pred()) { - return true; + for (;;) { + const bool _Stop = _Stoken.stop_requested(); + const bool _Result = _Pred(); + if (_Stop || _Result) { + return _Result; } wait(_Lck); } - - return _Pred(); } template bool wait_until( _Lock& _Lck, stop_token _Stoken, const chrono::time_point<_Clock, _Duration>& _Abs_time, _Predicate _Pred) { stop_callback<_Cv_any_notify_all> _Cb{_Stoken, this}; - while (!_Stoken.stop_requested()) { - if (_Pred()) { - return true; + for (;;) { + const bool _Stop = _Stoken.stop_requested(); + const bool _Result = _Pred(); + if (_Stop || _Result) { + return _Result; } if (wait_until(_Lck, _Abs_time) == cv_status::timeout) { return _Pred(); } } - return _Pred(); } template diff --git a/stl/inc/memory b/stl/inc/memory index b2d7569efc7..6c1df05d14a 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -3202,8 +3202,7 @@ class alignas(2 * sizeof(void*)) _Atomic_ptr_base { protected: constexpr _Atomic_ptr_base() noexcept = default; - _Atomic_ptr_base(_Ty* const _Px, _Ref_count_base* const _Ref) noexcept - : _Ptr(_Px), _Repptr(reinterpret_cast(_Ref)) {} + _Atomic_ptr_base(_Ty* const _Px, _Ref_count_base* const _Ref) noexcept : _Ptr(_Px), _Repptr(_Ref) {} void _Wait(_Ty* _Old, memory_order) const noexcept { for (;;) { diff --git a/stl/inc/stop_token b/stl/inc/stop_token index 8d8088e77bb..3b5e16be539 100644 --- a/stl/inc/stop_token +++ b/stl/inc/stop_token @@ -41,10 +41,9 @@ public: using _Callback_fn = void(__cdecl*)(_Stop_callback_base*) _NOEXCEPT_FNPTR; explicit _Stop_callback_base(const _Callback_fn _Fn_) noexcept : _Fn{_Fn_} {} + _Stop_callback_base(const _Stop_callback_base&) = delete; - _Stop_callback_base(_Stop_callback_base&&) = delete; _Stop_callback_base& operator=(const _Stop_callback_base&) = delete; - _Stop_callback_base& operator=(_Stop_callback_base&&) = delete; // if _Token is _Stop_requested, calls the callback; // otherwise, inserts *this into the callback list if stop is possible @@ -74,15 +73,15 @@ struct _Stop_state { atomic _Current_callback = nullptr; _Thrd_id_t _Stopping_thread = 0; - [[nodiscard]] bool _Stop_requested() const noexcept { + _NODISCARD bool _Stop_requested() const noexcept { return (_Stop_sources.load() & uint32_t{1}) != 0; } - [[nodiscard]] bool _Stop_possible() const noexcept { + _NODISCARD bool _Stop_possible() const noexcept { return _Stop_sources.load() != 0; } - [[nodiscard]] bool _Request_stop() noexcept { + _NODISCARD bool _Request_stop() noexcept { // Attempts to request stop and call callbacks, returns whether request was successful if ((_Stop_sources.fetch_or(uint32_t{1}) & uint32_t{1}) != 0) { // another thread already requested @@ -93,7 +92,7 @@ struct _Stop_state { for (;;) { auto _Head = _Callbacks._Lock_and_load(); _Current_callback.store(_Head, memory_order_relaxed); - _Current_callback.notify_one(); + _Current_callback.notify_all(); if (_Head == nullptr) { _Callbacks._Store_and_unlock(nullptr); return true; @@ -152,19 +151,17 @@ public: _STD swap(_State, _Other._State); } - [[nodiscard]] bool stop_requested() const noexcept { + _NODISCARD bool stop_requested() const noexcept { const auto _Local = _State; return _Local != nullptr && _Local->_Stop_requested(); } - [[nodiscard]] bool stop_possible() const noexcept { + _NODISCARD bool stop_possible() const noexcept { const auto _Local = _State; return _Local != nullptr && _Local->_Stop_possible(); } - [[nodiscard]] friend bool operator==(const stop_token& _Lhs, const stop_token& _Rhs) noexcept { - return _Lhs._State == _Rhs._State; - } + _NODISCARD friend bool operator==(const stop_token& _Lhs, const stop_token& _Rhs) noexcept = default; friend void swap(stop_token& _Lhs, stop_token& _Rhs) noexcept { _STD swap(_Lhs._State, _Rhs._State); @@ -201,7 +198,7 @@ public: ~stop_source() { const auto _Local = _State; if (_Local != nullptr) { - if ((_Local->_Stop_sources.fetch_sub(2, memory_order_relaxed) >> 1) == 1) { + if ((_Local->_Stop_sources.fetch_sub(2, memory_order_acq_rel) >> 1) == 1) { if (_Local->_Stop_tokens.fetch_sub(1, memory_order_acq_rel) == 1) { delete _Local; } @@ -213,7 +210,7 @@ public: _STD swap(_State, _Other._State); } - [[nodiscard]] stop_token get_token() const noexcept { + _NODISCARD stop_token get_token() const noexcept { const auto _Local = _State; if (_Local != nullptr) { _Local->_Stop_tokens.fetch_add(1, memory_order_relaxed); @@ -222,12 +219,12 @@ public: return stop_token{_Local}; } - [[nodiscard]] bool stop_requested() const noexcept { + _NODISCARD bool stop_requested() const noexcept { const auto _Local = _State; return _Local != nullptr && _Local->_Stop_requested(); } - [[nodiscard]] bool stop_possible() const noexcept { + _NODISCARD bool stop_possible() const noexcept { return _State != nullptr; } @@ -236,9 +233,7 @@ public: return _Local && _Local->_Request_stop(); } - [[nodiscard]] friend bool operator==(const stop_source& _Lhs, const stop_source& _Rhs) noexcept { - return _Lhs._State == _Rhs._State; - } + _NODISCARD friend bool operator==(const stop_source& _Lhs, const stop_source& _Rhs) noexcept = default; friend void swap(stop_source& _Lhs, stop_source& _Rhs) noexcept { _STD swap(_Lhs._State, _Rhs._State); @@ -251,7 +246,7 @@ private: template void _Stop_callback_base::_Do_attach( conditional_t<_Transfer_ownership, _Stop_state*&, _Stop_state* const> _State_raw) noexcept { - auto _State = _State_raw; // avoid an indirection in all of the below + const auto _State = _State_raw; // avoid an indirection in all of the below if (_State == nullptr) { return; } @@ -344,7 +339,7 @@ inline void _Stop_callback_base::_Detach() noexcept { // we aren't in the callback list even though we were added to it, so the stop requesting thread is attempting to // call the callback _STL_INTERNAL_CHECK((_Token._State->_Stop_sources.load() & uint32_t{1}) != 0); - if (_Token._State->_Current_callback.load(memory_order_relaxed) != this + if (_Token._State->_Current_callback.load(memory_order_acquire) != this || _Token._State->_Stopping_thread == _Thrd_id()) { // the callback is done or the dtor is being recursively reentered, do not block _Token._State->_Callbacks._Store_and_unlock(_Head); @@ -353,7 +348,7 @@ inline void _Stop_callback_base::_Detach() noexcept { // the callback is being executed by another thread, block until it is complete _Token._State->_Callbacks._Store_and_unlock(_Head); - _Token._State->_Current_callback.wait(this, memory_order_relaxed); + _Token._State->_Current_callback.wait(this, memory_order_acquire); } template @@ -361,17 +356,17 @@ class stop_callback : public _Stop_callback_base { public: using callback_type = _Callback; - template , int> = 0> - explicit stop_callback(const stop_token& _Token, _Cb_init_ty&& _Cb_) noexcept( - is_nothrow_constructible_v<_Callback, _Cb_init_ty>) - : _Stop_callback_base{_Invoke_by_stop}, _Cb(_STD forward<_Cb_init_ty>(_Cb_)) { + template , int> = 0> + explicit stop_callback(const stop_token& _Token, _CbInitTy&& _Cb_) noexcept( + is_nothrow_constructible_v<_Callback, _CbInitTy>) + : _Stop_callback_base{_Invoke_by_stop}, _Cb(_STD forward<_CbInitTy>(_Cb_)) { _Attach(_Token); } - template , int> = 0> - explicit stop_callback(stop_token&& _Token, _Cb_init_ty&& _Cb_) noexcept( - is_nothrow_constructible_v<_Callback, _Cb_init_ty>) - : _Stop_callback_base{_Invoke_by_stop}, _Cb(_STD forward<_Cb_init_ty>(_Cb_)) { + template , int> = 0> + explicit stop_callback(stop_token&& _Token, _CbInitTy&& _Cb_) noexcept( + is_nothrow_constructible_v<_Callback, _CbInitTy>) + : _Stop_callback_base{_Invoke_by_stop}, _Cb(_STD forward<_CbInitTy>(_Cb_)) { _Attach(_STD move(_Token)); } @@ -380,7 +375,7 @@ public: } private: - static void __cdecl _Invoke_by_stop(_Stop_callback_base* const _This) noexcept // enforces termination + static void __cdecl _Invoke_by_stop(_Stop_callback_base* const _This) noexcept // terminates { _STD forward<_Callback>(static_cast(_This)->_Cb)(); } diff --git a/stl/inc/thread b/stl/inc/thread index 8d64c186f8d..61cd3cf683e 100644 --- a/stl/inc/thread +++ b/stl/inc/thread @@ -263,7 +263,7 @@ public: jthread() noexcept : _Impl{}, _Ssource{nostopstate} {} - template , jthread>, int> = 0> + template , jthread>, int> = 0> explicit jthread(_Fn&& _Fx, _Args&&... _Ax) { if constexpr (is_invocable_v, stop_token, decay_t<_Args>...>) { _Impl._Start(_STD forward<_Fn>(_Fx), _Ssource.get_token(), _STD forward<_Args>(_Ax)...); @@ -293,7 +293,7 @@ public: _Ssource.swap(_Other._Ssource); } - [[nodiscard]] bool joinable() const noexcept { + _NODISCARD bool joinable() const noexcept { return _Impl.joinable(); } @@ -305,15 +305,15 @@ public: _Impl.detach(); } - id get_id() const noexcept { + _NODISCARD id get_id() const noexcept { return _Impl.get_id(); } - [[nodiscard]] stop_source get_stop_source() noexcept { + _NODISCARD stop_source get_stop_source() noexcept { return _Ssource; } - [[nodiscard]] stop_token get_stop_token() const noexcept { + _NODISCARD stop_token get_stop_token() const noexcept { return _Ssource.get_token(); } @@ -325,7 +325,7 @@ public: _Lhs.swap(_Rhs); } - [[nodiscard]] static unsigned int hardware_concurrency() noexcept { + _NODISCARD static unsigned int hardware_concurrency() noexcept { return thread::hardware_concurrency(); } From 01fc42c6f1d8f1d599bae41fca488d1404b6a355 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 24 Aug 2020 21:19:25 -0700 Subject: [PATCH 06/20] Tests pass tests pass woo woo nice nice --- stl/inc/atomic | 4 ++++ stl/inc/memory | 4 ++-- stl/inc/thread | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index d14abf902b7..cc44961f907 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -3019,6 +3019,10 @@ public: } } + _NODISCARD _Ty* _Load_relaxed() noexcept { + return reinterpret_cast<_Ty*>(_Storage.load(memory_order_relaxed)); + } + void _Store_and_unlock(_Ty* const _Value) noexcept { const auto _Rep = _Storage.exchange(reinterpret_cast(_Value)); if ((_Rep & _Lock_mask) == _Locked_notify_needed) { diff --git a/stl/inc/memory b/stl/inc/memory index 6c1df05d14a..62f8b9c909a 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -3338,7 +3338,7 @@ public: } ~atomic() { - const auto _Rep = reinterpret_cast<_Ref_count_base*>(this->_Repptr.load(memory_order_relaxed)); + const auto _Rep = this->_Repptr._Load_relaxed(); if (_Rep) { _Rep->_Decref(); } @@ -3455,7 +3455,7 @@ public: } ~atomic() { - const auto _Rep = reinterpret_cast<_Ref_count_base*>(this->_Repptr.load(memory_order_relaxed)); + const auto _Rep = this->_Repptr._Load_relaxed(); if (_Rep) { _Rep->_Decwref(); } diff --git a/stl/inc/thread b/stl/inc/thread index 61cd3cf683e..9b4db7c91b0 100644 --- a/stl/inc/thread +++ b/stl/inc/thread @@ -42,7 +42,9 @@ public: thread() noexcept : _Thr{} {} private: +#if _HAS_CXX20 friend jthread; +#endif // _HAS_CXX20 template static unsigned int __stdcall _Invoke(void* _RawVals) noexcept /* terminates */ { From c28e072019597d4c5b2e17cc01755202604695d6 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 24 Aug 2020 22:15:36 -0700 Subject: [PATCH 07/20] Add all the missing [replacement.functions]. --- tests/std/include/new_counter.hpp | 57 ++++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/tests/std/include/new_counter.hpp b/tests/std/include/new_counter.hpp index c7d79861f30..c9162f558ef 100644 --- a/tests/std/include/new_counter.hpp +++ b/tests/std/include/new_counter.hpp @@ -1,8 +1,5 @@ -#ifdef _STD_TEST_NEW_COUNTER -#error new_counter.hpp defines non-inline functions and thus must be used only once. -#else // ^^^ _STD_TEST_NEW_COUNTER // !_STD_TEST_NEW_COUNTER vvv -#define _STD_TEST_NEW_COUNTER -#endif // ^^^ !_STD_TEST_NEW_COUNTER +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception #include #include @@ -34,21 +31,43 @@ void* operator new(size_t size) { return p; } +void* operator new(std::size_t, std::align_val_t) { + abort(); +} + void* operator new(size_t size, const std::nothrow_t&) noexcept { if (std_testing::g_total_news == std_testing::g_maximum_news) { return nullptr; } + if (size == 0) { ++size; } + ++std_testing::g_total_news; return malloc(size); } +void* operator new(size_t size, std::align_val_t, const std::nothrow_t&) noexcept { + abort(); +} + void operator delete(void* ptr) noexcept { ::operator delete(ptr, std::nothrow); } +void operator delete(void* ptr, std::size_t) noexcept { + ::operator delete(ptr, std::nothrow); +} + +void operator delete(void* ptr, std::align_val_t) noexcept { + abort(); +} + +void operator delete(void*, const std::nothrow_t&) noexcept { + abort(); +} + void operator delete(void* ptr, const std::nothrow_t&) noexcept { if (ptr) { ++std_testing::g_total_deletes; @@ -57,20 +76,48 @@ void operator delete(void* ptr, const std::nothrow_t&) noexcept { } } +void operator delete(void*, std::align_val_t, const std::nothrow_t&) { + abort(); +} + void* operator new[](size_t size) { return ::operator new(size); } +void* operator new[](std::size_t, std::align_val_t) { + abort(); +} + void* operator new[](size_t size, const std::nothrow_t&) noexcept { return ::operator new(size, std::nothrow); } +void* operator new[](std::size_t, std::align_val_t, const std::nothrow_t&) noexcept { + abort(); +} + void operator delete[](void* ptr) noexcept { ::operator delete(ptr); } +void operator delete[](void* ptr, std::size_t size) noexcept { + ::operator delete(ptr, size); +} + +void operator delete[](void* ptr, std::align_val_t) noexcept { + abort(); +} + +void operator delete[](void* ptr, std::size_t, std::align_val_t) noexcept { + abort(); +} + void operator delete[](void* ptr, const std::nothrow_t&) noexcept { ::operator delete(ptr, std::nothrow); } +void operator delete[](void*, std::align_val_t, const std::nothrow_t&) noexcept { + abort(); +} + #pragma warning(pop) From f2bf64bd117d5dbdf5cec296552b9dd84d1c4ac8 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 24 Aug 2020 22:19:22 -0700 Subject: [PATCH 08/20] Test and comment nitpicks. --- stl/inc/yvals_core.h | 2 +- .../P0660R10_jthread_and_cv_any/test.cpp | 3 +- tests/std/tests/P0660R10_stop_token/test.cpp | 4 +-- .../VSO_0157762_feature_test_macros/test.cpp | 28 +++++++++---------- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/stl/inc/yvals_core.h b/stl/inc/yvals_core.h index cb2f0e72ed8..47c9429d115 100644 --- a/stl/inc/yvals_core.h +++ b/stl/inc/yvals_core.h @@ -159,7 +159,7 @@ // P0646R1 list/forward_list remove()/remove_if()/unique() Return size_type // P0653R2 to_address() // P0655R1 visit() -// P0661R10 and jthread +// P0661R10 And jthread // P0674R1 make_shared() For Arrays // P0718R2 atomic>, atomic> // P0758R1 is_nothrow_convertible diff --git a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp index baa12b2ffb7..8b1be36f311 100644 --- a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp +++ b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #define STATIC_ASSERT(...) static_assert(__VA_ARGS__, #__VA_ARGS__) @@ -134,7 +135,7 @@ int main() { assert(source.stop_possible()); } - // TRANSITION, MSFT-11107628 "_Exit allows cleanup in other DLLs" + // TRANSITION, OS-11107628 "_Exit allows cleanup in other DLLs" // detach() is intentionally not tested // get_id, get_stop_source, get_stop_token tested above diff --git a/tests/std/tests/P0660R10_stop_token/test.cpp b/tests/std/tests/P0660R10_stop_token/test.cpp index b27d8e6751d..eaf5cdc24e0 100644 --- a/tests/std/tests/P0660R10_stop_token/test.cpp +++ b/tests/std/tests/P0660R10_stop_token/test.cpp @@ -56,7 +56,7 @@ struct cb_destroying_functor { int main() noexcept { reset_new_counters(0); - { // all the following must not allocate and work with a nostopstate source; in rough synopsis order + { // all the following must not allocate, and must work with a nostopstate source; in rough synopsis order stop_token token; stop_token token_copy{token}; stop_token token_moved{move(token)}; @@ -333,7 +333,7 @@ int main() noexcept { // block_destroy makes it more likely that the timing assumption above is correct because the timer doesn't // start until we know the worker thread is actively running trying to request_stop atomic block_destroy{false}; - std::thread worker{[&] { + thread worker{[&] { // run the callbacks in the worker thread block_request_stop.wait(false); block_destroy.store(true); diff --git a/tests/std/tests/VSO_0157762_feature_test_macros/test.cpp b/tests/std/tests/VSO_0157762_feature_test_macros/test.cpp index b5534b29293..bb84f83e36b 100644 --- a/tests/std/tests/VSO_0157762_feature_test_macros/test.cpp +++ b/tests/std/tests/VSO_0157762_feature_test_macros/test.cpp @@ -826,20 +826,6 @@ STATIC_ASSERT(__cpp_lib_is_nothrow_convertible == 201806L); #endif #endif -#if _HAS_CXX20 -#ifndef __cpp_lib_jthread -#error __cpp_lib_jthread is not defined -#elif __cpp_lib_jthread != 201911L -#error __cpp_lib_jthread is not 201911L -#else -STATIC_ASSERT(__cpp_lib_jthread == 201911L); -#endif -#else -#ifdef __cpp_lib_jthread -#error __cpp_lib_jthread is defined -#endif -#endif - #ifndef __cpp_lib_is_null_pointer #error __cpp_lib_is_null_pointer is not defined #elif __cpp_lib_is_null_pointer != 201309L @@ -862,6 +848,20 @@ STATIC_ASSERT(__cpp_lib_is_swappable == 201603L); #endif #endif +#if _HAS_CXX20 +#ifndef __cpp_lib_jthread +#error __cpp_lib_jthread is not defined +#elif __cpp_lib_jthread != 201911L +#error __cpp_lib_jthread is not 201911L +#else +STATIC_ASSERT(__cpp_lib_jthread == 201911L); +#endif +#else +#ifdef __cpp_lib_jthread +#error __cpp_lib_jthread is defined +#endif +#endif + #if _HAS_CXX17 #ifndef __cpp_lib_launder #error __cpp_lib_launder is not defined From 9cf088f9958d339c9f13775e3acfdd78589d2b29 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Tue, 18 Aug 2020 15:14:57 -0700 Subject: [PATCH 09/20] Fix CV race wherein missed wakes could result in deadlock, see example https://github.com/microsoft/STL/pull/1196#discussion_r471773407 This is broken in the spec. I asked SG1 to comment: ``` { condition_variable_any cv; mutex m; jthread j{[&](stop_token token) { unique_lock lck{m}; cv.wait(lck, token, [] { return false; }); }}; } // destroy j ``` This program can deadlock under the current spec; there is nothing forbidding the following execution: T1: launches T2 T2: takes `m` T2: [thread.condvarany.intwait]/2 "Registers for the duration of this call `*this` to get notified on a stop request on `stoken` during this call", stop is not requested so execution continues T2: `while (!stoken.stop_requested()) {` T2: `if (pred())` (`pred` returns `false`) T1: `j.~jthread` T1: `request_stop()`, calls `cv.notify_all()` T2: `wait(lck)` T1: `join()` Deadlock. --- stl/inc/condition_variable | 121 ++++++++++++++++++++++++------------- 1 file changed, 79 insertions(+), 42 deletions(-) diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index f959340727b..89b25a1c49f 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -28,6 +28,27 @@ _STL_DISABLE_CLANG_WARNINGS #endif // _M_CEE _STD_BEGIN +template +struct _Unlock_guard { + _Unlock_guard(_Lock& _Mtx_) : _Mtx(_Mtx_) { + _Mtx.unlock(); + } + + ~_Unlock_guard() noexcept /* terminates */ { + // relock mutex or terminate() + // condition_variable_any wait functions are required to terminate if + // the mutex cannot be relocked; + // we slam into noexcept here for easier user debugging. + _Mtx.lock(); + } + + _Unlock_guard(const _Unlock_guard&) = delete; + _Unlock_guard& operator=(const _Unlock_guard&) = delete; + +private: + _Lock& _Mtx; +}; + class condition_variable_any { // class for waiting for conditions with any kind of mutex public: condition_variable_any() : _Myptr{_STD make_shared()} { @@ -53,15 +74,12 @@ public: template void wait(_Lock& _Lck) noexcept /* terminates */ { // wait for signal - { - const shared_ptr _Ptr = _Myptr; // for immunity to *this destruction - lock_guard _Guard{*_Ptr}; - _Lck.unlock(); - _Cnd_wait(_Mycnd(), _Ptr->_Mymtx()); - } // unlock - - _Lck.lock(); - } + const shared_ptr _Ptr = _Myptr; // for immunity to *this destruction + unique_lock _Guard{*_Ptr}; + _Unlock_guard<_Lock> _Unlock_outer{_Lck}; + _Cnd_wait(_Mycnd(), _Ptr->_Mymtx()); + _Guard.unlock(); + } // relock _Lck template void wait(_Lock& _Lck, _Predicate _Pred) noexcept(noexcept(!_Pred())) /* strengthened */ { @@ -92,8 +110,8 @@ public: template cv_status wait_for(_Lock& _Lck, const chrono::duration<_Rep, _Period>& _Rel_time) { // wait for duration if (_Rel_time <= chrono::duration<_Rep, _Period>::zero()) { - _Lck.unlock(); - _Relock(_Lck); + _Unlock_guard<_Lock> _Unlock_outer{_Lck}; + (void) _Unlock_outer; return cv_status::timeout; } @@ -148,17 +166,27 @@ private: public: template - bool wait(_Lock& _Lck, stop_token _Stoken, _Predicate _Pred) { + bool wait(_Lock& _Lck, stop_token _Stoken, _Predicate _Pred) noexcept(noexcept(!_Pred())) /* strengthened */ { + // TRANSITION, ABI: Due to the unsynchronized delivery of notify_all by _Stoken, + // this implementation cannot tolerate *this destruction while an interruptible wait + // is outstanding. A future ABI should store both the internal CV and internal mutex + // in the reference counted block to allow this. stop_callback<_Cv_any_notify_all> _Cb{_Stoken, this}; for (;;) { - const bool _Stop = _Stoken.stop_requested(); - const bool _Result = _Pred(); - if (_Stop || _Result) { - return _Result; + if (_Pred()) { + return true; } - wait(_Lck); - } + unique_lock _Guard{*_Myptr}; + if (_Stoken.stop_requested()) { + _Guard.unlock(); + return _Pred(); + } + + _Unlock_guard<_Lock> _Unlock_outer{_Lck}; + _Cnd_wait(_Mycnd(), _Myptr->_Mymtx()); + _Guard.unlock(); + } // relock } template @@ -166,16 +194,37 @@ public: _Lock& _Lck, stop_token _Stoken, const chrono::time_point<_Clock, _Duration>& _Abs_time, _Predicate _Pred) { stop_callback<_Cv_any_notify_all> _Cb{_Stoken, this}; for (;;) { - const bool _Stop = _Stoken.stop_requested(); - const bool _Result = _Pred(); - if (_Stop || _Result) { - return _Result; + if (_Pred()) { + return true; } - if (wait_until(_Lck, _Abs_time) == cv_status::timeout) { - return _Pred(); + unique_lock _Guard{*_Myptr}; + if (_Stoken.stop_requested()) { + break; } - } + + _Unlock_guard<_Lock> _Unlock_outer{_Lck}; + const auto _Now = _Clock::now(); + if (_Now >= _Abs_time) { + break; + } + + const auto _Rel_time = _Abs_time - _Now; + // TRANSITION, ABI: The standard says that we should use a steady clock, + // but unfortunately our ABI speaks struct xtime, which is relative to the system clock. + _CSTD xtime _Tgt; + const bool _Clamped = _To_xtime_10_day_clamped(_Tgt, _Rel_time); + const int _Res = _Cnd_timedwait(_Mycnd(), _Myptr->_Mymtx(), &_Tgt); + _Guard.unlock(); + + if (_Res == _Thrd_timedout) { + break; + } else if (_Res != _Thrd_success) { + _Throw_C_error(_Res); + } + } // relock + + return _Pred(); } template @@ -195,16 +244,11 @@ private: template cv_status _Wait_until(_Lock& _Lck, const xtime* const _Abs_time) { // wait for signal with timeout - int _Res; - - { - const shared_ptr _Ptr = _Myptr; // for immunity to *this destruction - lock_guard _Guard{*_Ptr}; - _Lck.unlock(); - _Res = _Cnd_timedwait(_Mycnd(), _Ptr->_Mymtx(), _Abs_time); - } // unlock - - _Relock(_Lck); + const shared_ptr _Ptr = _Myptr; // for immunity to *this destruction + unique_lock _Guard{*_Ptr}; + _Unlock_guard<_Lock> _Unlock_outer{_Lck}; + const int _Res = _Cnd_timedwait(_Mycnd(), _Ptr->_Mymtx(), _Abs_time); + _Guard.unlock(); switch (_Res) { case _Thrd_success: @@ -215,13 +259,6 @@ private: _Throw_C_error(_Res); } } - - template - static void _Relock(_Lock& _Lck) noexcept /* terminates */ { // relock external mutex or terminate() - // Wait functions are required to terminate if the mutex cannot be locked; - // we slam into noexcept here for easier user debugging. - _Lck.lock(); - } }; inline void notify_all_at_thread_exit(condition_variable& _Cnd, unique_lock _Lck) { From 8854f4f7e9186d1c38357453f3f26aa6ec54f132 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Tue, 18 Aug 2020 15:44:55 -0700 Subject: [PATCH 10/20] Compiler errors :/ --- stl/inc/atomic | 4 ++++ stl/inc/memory | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index cc44961f907..ff6327522ce 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -3031,6 +3031,10 @@ public: } } + _Ty* _Unsafe_load_relaxed() const noexcept { + return reinterpret_cast<_Ty*>(_Storage.load(memory_order_relaxed)); + } + private: atomic _Storage; }; diff --git a/stl/inc/memory b/stl/inc/memory index 62f8b9c909a..fd713bd150e 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -3338,7 +3338,7 @@ public: } ~atomic() { - const auto _Rep = this->_Repptr._Load_relaxed(); + const auto _Rep = this->_Repptr._Unsafe_load_relaxed(); if (_Rep) { _Rep->_Decref(); } @@ -3455,7 +3455,7 @@ public: } ~atomic() { - const auto _Rep = this->_Repptr._Load_relaxed(); + const auto _Rep = reinterpret_cast<_Ref_count_base*>(this->_Repptr.load(memory_order_relaxed)); if (_Rep) { _Rep->_Decwref(); } From 338249d32c72df75192a3aa956e0d433ad244689 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Tue, 18 Aug 2020 16:07:53 -0700 Subject: [PATCH 11/20] More compiler errors. --- stl/inc/condition_variable | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index 89b25a1c49f..1030e53af11 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -218,7 +218,9 @@ public: _Guard.unlock(); if (_Res == _Thrd_timedout) { - break; + if (!_Clamped) { + break; + } } else if (_Res != _Thrd_success) { _Throw_C_error(_Res); } From 7aa09ca56f1e826d74a46ba7b572e3ba91a43f74 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 24 Aug 2020 22:52:17 -0700 Subject: [PATCH 12/20] More test fixes. --- tests/std/include/new_counter.hpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/std/include/new_counter.hpp b/tests/std/include/new_counter.hpp index c9162f558ef..5dfd6fbc1ea 100644 --- a/tests/std/include/new_counter.hpp +++ b/tests/std/include/new_counter.hpp @@ -48,7 +48,7 @@ void* operator new(size_t size, const std::nothrow_t&) noexcept { return malloc(size); } -void* operator new(size_t size, std::align_val_t, const std::nothrow_t&) noexcept { +void* operator new(size_t, std::align_val_t, const std::nothrow_t&) noexcept { abort(); } @@ -60,11 +60,7 @@ void operator delete(void* ptr, std::size_t) noexcept { ::operator delete(ptr, std::nothrow); } -void operator delete(void* ptr, std::align_val_t) noexcept { - abort(); -} - -void operator delete(void*, const std::nothrow_t&) noexcept { +void operator delete(void*, std::align_val_t) noexcept { abort(); } @@ -76,7 +72,7 @@ void operator delete(void* ptr, const std::nothrow_t&) noexcept { } } -void operator delete(void*, std::align_val_t, const std::nothrow_t&) { +void operator delete(void*, std::align_val_t, const std::nothrow_t&) noexcept { abort(); } @@ -104,11 +100,11 @@ void operator delete[](void* ptr, std::size_t size) noexcept { ::operator delete(ptr, size); } -void operator delete[](void* ptr, std::align_val_t) noexcept { +void operator delete[](void*, std::align_val_t) noexcept { abort(); } -void operator delete[](void* ptr, std::size_t, std::align_val_t) noexcept { +void operator delete[](void*, std::size_t, std::align_val_t) noexcept { abort(); } From 7334bae596dc936216526ab221520d1c88021690 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal Date: Tue, 25 Aug 2020 13:38:11 -0700 Subject: [PATCH 13/20] Make test more resilient to differences between system_clock and steady_clock. --- tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp index 8b1be36f311..bab3a4da77c 100644 --- a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp +++ b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp @@ -234,9 +234,10 @@ int main() { condition_variable_any cv; unique_lock lck{m}; auto started_at = chrono::steady_clock::now(); - assert(cv.wait_for(lck, never_stopped.get_token(), 100ms, [] { return false; }) == false); + auto until = started_at + 100ms; + assert(cv.wait_until(lck, never_stopped.get_token(), until, [] { return false; }) == false); // not a timing assumption: the wait_for must wait at least that long - assert(started_at + 100ms <= chrono::steady_clock::now()); + assert(until <= chrono::steady_clock::now()); }); } From 1a4b5cbfc7094354e90801d2f4995cff81c69422 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal Date: Wed, 26 Aug 2020 02:47:50 -0700 Subject: [PATCH 14/20] Ignore timeout status from the underlying CV in cv_any because the result is not useful. We will just recheck with the clock. --- stl/inc/condition_variable | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index 1030e53af11..98b9242a5f4 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -217,11 +217,11 @@ public: const int _Res = _Cnd_timedwait(_Mycnd(), _Myptr->_Mymtx(), &_Tgt); _Guard.unlock(); - if (_Res == _Thrd_timedout) { - if (!_Clamped) { - break; - } - } else if (_Res != _Thrd_success) { + switch (_Res) { + case _Thrd_timedout: + case _Thrd_success: + break; + default: _Throw_C_error(_Res); } } // relock From 66a14c2710d5030a8ba67b29a7ec422a75a9e5fe Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 26 Aug 2020 10:17:28 -0700 Subject: [PATCH 15/20] Repair rebase damage. --- stl/inc/memory | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/memory b/stl/inc/memory index fd713bd150e..19bb0c15f05 100644 --- a/stl/inc/memory +++ b/stl/inc/memory @@ -3455,7 +3455,7 @@ public: } ~atomic() { - const auto _Rep = reinterpret_cast<_Ref_count_base*>(this->_Repptr.load(memory_order_relaxed)); + const auto _Rep = this->_Repptr._Unsafe_load_relaxed(); if (_Rep) { _Rep->_Decwref(); } From e416bbd240aee95f9ad16f90e61d23f81a31300a Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 26 Aug 2020 10:21:01 -0700 Subject: [PATCH 16/20] Fix ill-formed constexpr, more rebase damage. --- stl/inc/atomic | 6 +----- stl/inc/condition_variable | 4 ++-- tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp | 5 ++--- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/stl/inc/atomic b/stl/inc/atomic index ff6327522ce..1ab6faad6ea 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -2983,7 +2983,7 @@ public: public: constexpr _Locked_pointer() noexcept : _Storage{} {} - constexpr _Locked_pointer(_Ty* _Ptr) noexcept : _Storage{reinterpret_cast(_Ptr)} {} + _Locked_pointer(_Ty* _Ptr) noexcept : _Storage{reinterpret_cast(_Ptr)} {} _Locked_pointer(const _Locked_pointer&) = delete; _Locked_pointer& operator=(const _Locked_pointer&) = delete; @@ -3019,10 +3019,6 @@ public: } } - _NODISCARD _Ty* _Load_relaxed() noexcept { - return reinterpret_cast<_Ty*>(_Storage.load(memory_order_relaxed)); - } - void _Store_and_unlock(_Ty* const _Value) noexcept { const auto _Rep = _Storage.exchange(reinterpret_cast(_Value)); if ((_Rep & _Lock_mask) == _Locked_notify_needed) { diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index 98b9242a5f4..df3262ec7bd 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -213,8 +213,8 @@ public: // TRANSITION, ABI: The standard says that we should use a steady clock, // but unfortunately our ABI speaks struct xtime, which is relative to the system clock. _CSTD xtime _Tgt; - const bool _Clamped = _To_xtime_10_day_clamped(_Tgt, _Rel_time); - const int _Res = _Cnd_timedwait(_Mycnd(), _Myptr->_Mymtx(), &_Tgt); + _To_xtime_10_day_clamped(_Tgt, _Rel_time); + const int _Res = _Cnd_timedwait(_Mycnd(), _Myptr->_Mymtx(), &_Tgt); _Guard.unlock(); switch (_Res) { diff --git a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp index bab3a4da77c..8b1be36f311 100644 --- a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp +++ b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp @@ -234,10 +234,9 @@ int main() { condition_variable_any cv; unique_lock lck{m}; auto started_at = chrono::steady_clock::now(); - auto until = started_at + 100ms; - assert(cv.wait_until(lck, never_stopped.get_token(), until, [] { return false; }) == false); + assert(cv.wait_for(lck, never_stopped.get_token(), 100ms, [] { return false; }) == false); // not a timing assumption: the wait_for must wait at least that long - assert(until <= chrono::steady_clock::now()); + assert(started_at + 100ms <= chrono::steady_clock::now()); }); } From c38d004a76f8b0f177866a5ad39d15f620094af8 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Wed, 26 Aug 2020 10:44:37 -0700 Subject: [PATCH 17/20] No nodiscard. --- stl/inc/condition_variable | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index df3262ec7bd..d5f1d549123 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -213,7 +213,7 @@ public: // TRANSITION, ABI: The standard says that we should use a steady clock, // but unfortunately our ABI speaks struct xtime, which is relative to the system clock. _CSTD xtime _Tgt; - _To_xtime_10_day_clamped(_Tgt, _Rel_time); + (void) _To_xtime_10_day_clamped(_Tgt, _Rel_time); const int _Res = _Cnd_timedwait(_Mycnd(), _Myptr->_Mymtx(), &_Tgt); _Guard.unlock(); From 02d992275e1cfbf1b5be481677af924c12a3c6ee Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 18 Sep 2020 16:12:46 -0700 Subject: [PATCH 18/20] Resolve more CR comments. --- stl/inc/__msvc_all_public_headers.hpp | 1 + stl/inc/atomic | 7 +++---- stl/inc/condition_variable | 11 ++++++----- stl/inc/stop_token | 8 +++++--- stl/inc/thread | 7 +++++-- stl/inc/yvals_core.h | 2 +- tests/std/include/new_counter.hpp | 15 ++++++++------- .../tests/P0660R10_jthread_and_cv_any/test.cpp | 17 ++++++++--------- .../tests/include_each_header_alone_matrix.lst | 1 + 9 files changed, 38 insertions(+), 31 deletions(-) diff --git a/stl/inc/__msvc_all_public_headers.hpp b/stl/inc/__msvc_all_public_headers.hpp index 3ba146c97bc..912896ed2d9 100644 --- a/stl/inc/__msvc_all_public_headers.hpp +++ b/stl/inc/__msvc_all_public_headers.hpp @@ -108,6 +108,7 @@ #include #include #include +#include #endif // _M_CEE_PURE #ifndef _M_CEE diff --git a/stl/inc/atomic b/stl/inc/atomic index 5a9ce9407c1..bf107a58f89 100644 --- a/stl/inc/atomic +++ b/stl/inc/atomic @@ -2981,9 +2981,8 @@ public: static constexpr uintptr_t _Locked_notify_needed = 2; static constexpr uintptr_t _Ptr_value_mask = ~_Lock_mask; -public: constexpr _Locked_pointer() noexcept : _Storage{} {} - _Locked_pointer(_Ty* _Ptr) noexcept : _Storage{reinterpret_cast(_Ptr)} {} + explicit _Locked_pointer(_Ty* const _Ptr) noexcept : _Storage{reinterpret_cast(_Ptr)} {} _Locked_pointer(const _Locked_pointer&) = delete; _Locked_pointer& operator=(const _Locked_pointer&) = delete; @@ -3001,7 +3000,7 @@ public: case _Locked_notify_not_needed: // Try to set "notify needed" and wait if (!_Storage.compare_exchange_weak(_Rep, (_Rep & _Ptr_value_mask) | _Locked_notify_needed)) { - // Failed to put notify needed flag on, try again + // Failed to set notify needed flag, try again _YIELD_PROCESSOR(); break; } @@ -3027,7 +3026,7 @@ public: } } - _Ty* _Unsafe_load_relaxed() const noexcept { + _NODISCARD _Ty* _Unsafe_load_relaxed() const noexcept { return reinterpret_cast<_Ty*>(_Storage.load(memory_order_relaxed)); } diff --git a/stl/inc/condition_variable b/stl/inc/condition_variable index d5f1d549123..1289183537c 100644 --- a/stl/inc/condition_variable +++ b/stl/inc/condition_variable @@ -30,7 +30,7 @@ _STL_DISABLE_CLANG_WARNINGS _STD_BEGIN template struct _Unlock_guard { - _Unlock_guard(_Lock& _Mtx_) : _Mtx(_Mtx_) { + explicit _Unlock_guard(_Lock& _Mtx_) : _Mtx(_Mtx_) { _Mtx.unlock(); } @@ -82,9 +82,9 @@ public: } // relock _Lck template - void wait(_Lock& _Lck, _Predicate _Pred) noexcept(noexcept(!_Pred())) /* strengthened */ { + void wait(_Lock& _Lck, _Predicate _Pred) noexcept(noexcept(static_cast(_Pred()))) /* strengthened */ { // wait for signal and check predicate - while (!_Pred()) { + while (!static_cast(_Pred())) { wait(_Lck); } } @@ -154,7 +154,7 @@ private: struct _Cv_any_notify_all { condition_variable_any* _This; - _Cv_any_notify_all(condition_variable_any* _This_) : _This{_This_} {} + explicit _Cv_any_notify_all(condition_variable_any* _This_) : _This{_This_} {} _Cv_any_notify_all(const _Cv_any_notify_all&) = delete; _Cv_any_notify_all& operator=(const _Cv_any_notify_all&) = delete; @@ -166,7 +166,8 @@ private: public: template - bool wait(_Lock& _Lck, stop_token _Stoken, _Predicate _Pred) noexcept(noexcept(!_Pred())) /* strengthened */ { + bool wait(_Lock& _Lck, stop_token _Stoken, _Predicate _Pred) noexcept( + noexcept(static_cast(_Pred()))) /* strengthened */ { // TRANSITION, ABI: Due to the unsynchronized delivery of notify_all by _Stoken, // this implementation cannot tolerate *this destruction while an interruptible wait // is outstanding. A future ABI should store both the internal CV and internal mutex diff --git a/stl/inc/stop_token b/stl/inc/stop_token index 3b5e16be539..ac4bc775af4 100644 --- a/stl/inc/stop_token +++ b/stl/inc/stop_token @@ -37,14 +37,16 @@ class stop_token; class _Stop_callback_base { friend _Stop_state; -public: +private: using _Callback_fn = void(__cdecl*)(_Stop_callback_base*) _NOEXCEPT_FNPTR; +public: explicit _Stop_callback_base(const _Callback_fn _Fn_) noexcept : _Fn{_Fn_} {} _Stop_callback_base(const _Stop_callback_base&) = delete; _Stop_callback_base& operator=(const _Stop_callback_base&) = delete; +protected: // if _Token is _Stop_requested, calls the callback; // otherwise, inserts *this into the callback list if stop is possible inline void _Attach(const stop_token& _Token) noexcept; @@ -383,8 +385,8 @@ private: _Callback _Cb; }; -template -stop_callback(stop_token, Callback) -> stop_callback; +template +stop_callback(stop_token, _Callback) -> stop_callback<_Callback>; _STD_END #pragma pop_macro("new") diff --git a/stl/inc/thread b/stl/inc/thread index 9b4db7c91b0..20d6ee84272 100644 --- a/stl/inc/thread +++ b/stl/inc/thread @@ -278,12 +278,15 @@ public: _Try_cancel_and_join(); } - jthread(const jthread&) = delete; - jthread(jthread&& _Other) noexcept = default; + jthread(const jthread&) = delete; + jthread(jthread&&) noexcept = default; jthread& operator=(const jthread&) = delete; jthread& operator=(jthread&& _Other) noexcept { // note: the standard specifically disallows making self-move-assignment a no-op here + // N4861 [thread.jthread.cons]/13 + // Effects: If joinable() is true, calls request_stop() and then join(). Assigns the state + // of x to *this and sets x to a default constructed state. _Try_cancel_and_join(); _Impl = _STD move(_Other._Impl); _Ssource = _STD move(_Other._Ssource); diff --git a/stl/inc/yvals_core.h b/stl/inc/yvals_core.h index 5219a098628..6113e82987e 100644 --- a/stl/inc/yvals_core.h +++ b/stl/inc/yvals_core.h @@ -159,7 +159,7 @@ // P0646R1 list/forward_list remove()/remove_if()/unique() Return size_type // P0653R2 to_address() // P0655R1 visit() -// P0661R10 And jthread +// P0660R10 And jthread // P0674R1 make_shared() For Arrays // P0718R2 atomic>, atomic> // P0758R1 is_nothrow_convertible diff --git a/tests/std/include/new_counter.hpp b/tests/std/include/new_counter.hpp index 5dfd6fbc1ea..9ecb27607a5 100644 --- a/tests/std/include/new_counter.hpp +++ b/tests/std/include/new_counter.hpp @@ -3,6 +3,7 @@ #include #include +#include #include #pragma once @@ -25,13 +26,13 @@ namespace std_testing { void* operator new(size_t size) { void* const p = ::operator new(size, std::nothrow); if (p == nullptr) { - throw std::bad_alloc(); + throw std::bad_alloc{}; } return p; } -void* operator new(std::size_t, std::align_val_t) { +void* operator new(size_t, std::align_val_t) { abort(); } @@ -56,7 +57,7 @@ void operator delete(void* ptr) noexcept { ::operator delete(ptr, std::nothrow); } -void operator delete(void* ptr, std::size_t) noexcept { +void operator delete(void* ptr, size_t) noexcept { ::operator delete(ptr, std::nothrow); } @@ -80,7 +81,7 @@ void* operator new[](size_t size) { return ::operator new(size); } -void* operator new[](std::size_t, std::align_val_t) { +void* operator new[](size_t, std::align_val_t) { abort(); } @@ -88,7 +89,7 @@ void* operator new[](size_t size, const std::nothrow_t&) noexcept { return ::operator new(size, std::nothrow); } -void* operator new[](std::size_t, std::align_val_t, const std::nothrow_t&) noexcept { +void* operator new[](size_t, std::align_val_t, const std::nothrow_t&) noexcept { abort(); } @@ -96,7 +97,7 @@ void operator delete[](void* ptr) noexcept { ::operator delete(ptr); } -void operator delete[](void* ptr, std::size_t size) noexcept { +void operator delete[](void* ptr, size_t size) noexcept { ::operator delete(ptr, size); } @@ -104,7 +105,7 @@ void operator delete[](void*, std::align_val_t) noexcept { abort(); } -void operator delete[](void*, std::size_t, std::align_val_t) noexcept { +void operator delete[](void*, size_t, std::align_val_t) noexcept { abort(); } diff --git a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp index 8b1be36f311..7aefb925967 100644 --- a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp +++ b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp @@ -91,7 +91,7 @@ int main() { assert(!worker_b.joinable()); } - { // self move assign, currently specified to try to cancel and join + { // self move assign, as of N4861 specified to try to cancel and join [thread.jthread.cons]/13 jthread worker{[] {}}; auto source = worker.get_stop_source(); worker = move(worker); @@ -151,13 +151,13 @@ int main() { }); } + constexpr auto infinity = chrono::steady_clock::time_point::max(); { // ditto without the cancellation this would deadlock jthread worker([](stop_token token) { mutex m; condition_variable_any cv; unique_lock lck{m}; - assert(cv.wait_until(lck, move(token), chrono::steady_clock::time_point::max(), [] { return false; }) - == false); + assert(cv.wait_until(lck, move(token), infinity, [] { return false; }) == false); }); } @@ -166,7 +166,7 @@ int main() { mutex m; condition_variable_any cv; unique_lock lck{m}; - assert(cv.wait_for(lck, move(token), chrono::steady_clock::duration::max(), [] { return false; }) == false); + assert(cv.wait_for(lck, move(token), infinity, [] { return false; }) == false); }); } @@ -195,9 +195,8 @@ int main() { bool b = false; jthread worker([&](stop_token token) { unique_lock lck{m}; - assert( - cv.wait_until(lck, move(token), chrono::steady_clock::time_point::max(), [] { return true; }) == true); - assert(cv.wait_until(lck, move(token), chrono::steady_clock::time_point::max(), [&] { return b; }) == true); + assert(cv.wait_until(lck, move(token), infinity, [] { return true; }) == true); + assert(cv.wait_until(lck, move(token), infinity, [&] { return b; }) == true); }); { @@ -214,8 +213,8 @@ int main() { bool b = false; jthread worker([&](stop_token token) { unique_lock lck{m}; - assert(cv.wait_for(lck, move(token), chrono::steady_clock::duration::max(), [] { return true; }) == true); - assert(cv.wait_for(lck, move(token), chrono::steady_clock::duration::max(), [&] { return b; }) == true); + assert(cv.wait_for(lck, move(token), infinity, [] { return true; }) == true); + assert(cv.wait_for(lck, move(token), infinity, [&] { return b; }) == true); }); { diff --git a/tests/std/tests/include_each_header_alone_matrix.lst b/tests/std/tests/include_each_header_alone_matrix.lst index 71cb23f8d3d..6728e9f7124 100644 --- a/tests/std/tests/include_each_header_alone_matrix.lst +++ b/tests/std/tests/include_each_header_alone_matrix.lst @@ -62,6 +62,7 @@ PM_CL="/DMEOW_HEADER=span" PM_CL="/DMEOW_HEADER=sstream" PM_CL="/DMEOW_HEADER=stack" PM_CL="/DMEOW_HEADER=stdexcept" +PM_CL="/DMEOW_HEADER=stop_token" PM_CL="/DMEOW_HEADER=streambuf" PM_CL="/DMEOW_HEADER=string" PM_CL="/DMEOW_HEADER=string_view" From d4778eda852eb985d0cfa7aa81f82c3596695857 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 18 Sep 2020 19:17:13 -0700 Subject: [PATCH 19/20] Sort stl/CMakeLists.txt. --- stl/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/CMakeLists.txt b/stl/CMakeLists.txt index c06a9bbbb3a..d3ee49f175f 100644 --- a/stl/CMakeLists.txt +++ b/stl/CMakeLists.txt @@ -179,8 +179,8 @@ set(HEADERS ${CMAKE_CURRENT_LIST_DIR}/inc/span ${CMAKE_CURRENT_LIST_DIR}/inc/sstream ${CMAKE_CURRENT_LIST_DIR}/inc/stack - ${CMAKE_CURRENT_LIST_DIR}/inc/stop_token ${CMAKE_CURRENT_LIST_DIR}/inc/stdexcept + ${CMAKE_CURRENT_LIST_DIR}/inc/stop_token ${CMAKE_CURRENT_LIST_DIR}/inc/streambuf ${CMAKE_CURRENT_LIST_DIR}/inc/string ${CMAKE_CURRENT_LIST_DIR}/inc/string_view From 0cbc0677a69f2ef22b2a07794d7a62574d1ecb0c Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 18 Sep 2020 19:17:28 -0700 Subject: [PATCH 20/20] Fix compiler errors in test. --- tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp index 7aefb925967..1495227aacd 100644 --- a/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp +++ b/tests/std/tests/P0660R10_jthread_and_cv_any/test.cpp @@ -151,7 +151,9 @@ int main() { }); } - constexpr auto infinity = chrono::steady_clock::time_point::max(); + static constexpr auto forever = chrono::steady_clock::duration::max(); + static constexpr auto infinity = chrono::steady_clock::time_point::max(); + { // ditto without the cancellation this would deadlock jthread worker([](stop_token token) { mutex m; @@ -166,7 +168,7 @@ int main() { mutex m; condition_variable_any cv; unique_lock lck{m}; - assert(cv.wait_for(lck, move(token), infinity, [] { return false; }) == false); + assert(cv.wait_for(lck, move(token), forever, [] { return false; }) == false); }); } @@ -213,8 +215,8 @@ int main() { bool b = false; jthread worker([&](stop_token token) { unique_lock lck{m}; - assert(cv.wait_for(lck, move(token), infinity, [] { return true; }) == true); - assert(cv.wait_for(lck, move(token), infinity, [&] { return b; }) == true); + assert(cv.wait_for(lck, move(token), forever, [] { return true; }) == true); + assert(cv.wait_for(lck, move(token), forever, [&] { return b; }) == true); }); {