Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhaul condition_variable #4720

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion stl/inc/__msvc_threads_core.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ struct _Mtx_internal_imp_t {

using _Mtx_t = _Mtx_internal_imp_t*;

struct _Stl_condition_variable {
void* _Unused = nullptr; // TRANSITION, ABI: was the vptr
void* _Win_cv = nullptr;
};

#pragma warning(push)
#pragma warning(disable : 26495) // Variable 'meow' is uninitialized. Always initialize a member variable (type.6).
struct _Cnd_internal_imp_t {
#if defined(_CRT_WINDOWS) // for Windows-internal code
static constexpr size_t _Cnd_internal_imp_size = 2 * sizeof(void*);
Expand All @@ -61,8 +68,12 @@ struct _Cnd_internal_imp_t {
static constexpr size_t _Cnd_internal_imp_size = 40;
#endif // ^^^ ordinary 32-bit code ^^^

_STD _Aligned_storage_t<_Cnd_internal_imp_size, alignof(void*)> _Cv_storage;
union {
_Stl_condition_variable _Stl_cv{};
_STD _Aligned_storage_t<_Cnd_internal_imp_size, alignof(void*)> _Cv_storage;
};
};
#pragma warning(pop)

using _Cnd_t = _Cnd_internal_imp_t*;
} // extern "C"
Expand Down
8 changes: 4 additions & 4 deletions stl/inc/condition_variable
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ private:
_EXPORT_STD class condition_variable_any { // class for waiting for conditions with any kind of mutex
public:
condition_variable_any() : _Myptr{_STD make_shared<mutex>()} {
#ifdef _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
_Cnd_init_in_situ(_Mycnd());
#endif // ^^^ defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) ^^^
}

~condition_variable_any() noexcept {
_Cnd_destroy_in_situ(_Mycnd());
}
~condition_variable_any() noexcept = default;

condition_variable_any(const condition_variable_any&) = delete;
condition_variable_any& operator=(const condition_variable_any&) = delete;
Expand Down Expand Up @@ -218,7 +218,7 @@ public:
private:
shared_ptr<mutex> _Myptr;

_Cnd_internal_imp_t _Cnd_storage;
_Cnd_internal_imp_t _Cnd_storage{};

_NODISCARD _Cnd_t _Mycnd() noexcept {
return &_Cnd_storage;
Expand Down
10 changes: 6 additions & 4 deletions stl/inc/mutex
Original file line number Diff line number Diff line change
Expand Up @@ -531,13 +531,15 @@ _EXPORT_STD enum class cv_status { // names for wait returns

_EXPORT_STD class condition_variable { // class for waiting for conditions
public:
#ifdef _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
condition_variable() noexcept /* strengthened */ {
_Cnd_init_in_situ(_Mycnd());
}
#else // ^^^ defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) / !defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) vvv
condition_variable() noexcept /* strengthened */ = default;
#endif // ^^^ !defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) ^^^

~condition_variable() noexcept {
_Cnd_destroy_in_situ(_Mycnd());
}
~condition_variable() noexcept = default;

condition_variable(const condition_variable&) = delete;
condition_variable& operator=(const condition_variable&) = delete;
Expand Down Expand Up @@ -629,7 +631,7 @@ public:
}

private:
_Cnd_internal_imp_t _Cnd_storage;
_Cnd_internal_imp_t _Cnd_storage{};

_Cnd_t _Mycnd() noexcept {
return &_Cnd_storage;
Expand Down
13 changes: 4 additions & 9 deletions stl/inc/xthreads.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,9 @@ enum { // mutex types
_Mtx_recursive = 0x100
};

#ifdef _CRTBLD
_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_init(_Mtx_t*, int) noexcept;
_CRTIMP2_PURE void __cdecl _Mtx_destroy(_Mtx_t) noexcept;
#endif // _CRTBLD
#if defined(_CRTBLD) || defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR)
_CRTIMP2_PURE void __cdecl _Mtx_init_in_situ(_Mtx_t, int) noexcept;
#endif // ^^^ defined(_CRTBLD) || defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) ^^^
_CRTIMP2_PURE int __cdecl _Mtx_current_owns(_Mtx_t) noexcept;
_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_lock(_Mtx_t) noexcept;
_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_trylock(_Mtx_t) noexcept;
Expand All @@ -55,12 +53,9 @@ void __cdecl _Smtx_unlock_exclusive(_Smtx_t*) noexcept;
void __cdecl _Smtx_unlock_shared(_Smtx_t*) noexcept;

// condition variables
#ifdef _CRTBLD
_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_init(_Cnd_t*) noexcept;
_CRTIMP2_PURE void __cdecl _Cnd_destroy(_Cnd_t) noexcept;
#endif // _CRTBLD
#ifdef _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR
_CRTIMP2_PURE void __cdecl _Cnd_init_in_situ(_Cnd_t) noexcept;
_CRTIMP2_PURE void __cdecl _Cnd_destroy_in_situ(_Cnd_t) noexcept;
#endif // ^^^ defined(_DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR) ^^^
_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(_Cnd_t, _Mtx_t) noexcept; // TRANSITION, ABI: Always succeeds
_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_broadcast(_Cnd_t) noexcept; // TRANSITION, ABI: Always succeeds
_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_signal(_Cnd_t) noexcept; // TRANSITION, ABI: Always succeeds
Expand Down
21 changes: 10 additions & 11 deletions stl/src/cond.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@

extern "C" {


// TRANSITION, ABI: preserved for binary compatibility (and _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR)
_CRTIMP2_PURE void __cdecl _Cnd_init_in_situ(const _Cnd_t cond) noexcept { // initialize condition variable in situ
new (Concurrency::details::_Get_cond_var(cond)) Concurrency::details::stl_condition_variable_win7;
new (cond) _Cnd_internal_imp_t;
}

// TRANSITION, ABI: preserved for binary compatibility
_CRTIMP2_PURE void __cdecl _Cnd_destroy_in_situ(_Cnd_t) noexcept {} // destroy condition variable in situ

// TRANSITION, ABI: preserved for binary compatibility
_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_init(_Cnd_t* const pcond) noexcept { // initialize
*pcond = nullptr;

Expand All @@ -32,9 +34,9 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_init(_Cnd_t* const pcond) noexcept { //
return _Thrd_result::_Success;
}

// TRANSITION, ABI: preserved for binary compatibility
_CRTIMP2_PURE void __cdecl _Cnd_destroy(const _Cnd_t cond) noexcept { // clean up
if (cond) { // something to do, do it
_Cnd_destroy_in_situ(cond);
_free_crt(cond);
}
}
Expand All @@ -52,9 +54,8 @@ _CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t mtx) noexcept { // set owner
}

_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx) noexcept { // wait until signaled
const auto cs = &mtx->_Critical_section;
_Mtx_clear_owner(mtx);
Concurrency::details::_Get_cond_var(cond)->wait(cs);
_Primitive_wait(cond, mtx);
_Mtx_reset_owner(mtx);
return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds
}
Expand All @@ -63,17 +64,15 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx
_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_timedwait(
const _Cnd_t cond, const _Mtx_t mtx, const _timespec64* const target) noexcept {
_Thrd_result res = _Thrd_result::_Success;
const auto cs = &mtx->_Critical_section;
if (target == nullptr) { // no target time specified, wait on mutex
_Mtx_clear_owner(mtx);
Concurrency::details::_Get_cond_var(cond)->wait(cs);
_Primitive_wait(cond, mtx);
_Mtx_reset_owner(mtx);
} else { // target time specified, wait for it
_timespec64 now;
_Timespec64_get_sys(&now);
_Mtx_clear_owner(mtx);
if (!Concurrency::details::_Get_cond_var(cond)->wait_for(
cs, _Xtime_diff_to_millis2(target, &now))) { // report timeout
if (!_Primitive_wait_for(cond, mtx, _Xtime_diff_to_millis2(target, &now))) { // report timeout
_Timespec64_get_sys(&now);
if (_Xtime_diff_to_millis2(target, &now) == 0) {
res = _Thrd_result::_Timedout;
Expand All @@ -85,12 +84,12 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_timedwait(
}

_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_signal(const _Cnd_t cond) noexcept { // release one waiting thread
Concurrency::details::_Get_cond_var(cond)->notify_one();
_Primitive_notify_one(cond);
return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds
}

_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_broadcast(const _Cnd_t cond) noexcept { // release all waiting threads
Concurrency::details::_Get_cond_var(cond)->notify_all();
_Primitive_notify_all(cond);
return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds
}

Expand Down
11 changes: 5 additions & 6 deletions stl/src/cthread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,11 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Thrd_create(_Thrd_t* thr, _Thrd_start_t func
_Thrd_result res;
_Thrd_binder b;
int started = 0;
_Cnd_t cond;
_Mtx_t mtx;
_Cnd_init(&cond);
_Mtx_init(&mtx, _Mtx_plain);
_Cnd_internal_imp_t cond_var{};
_Cnd_t cond = &cond_var;
_Mtx_internal_imp_t mtx_var{};
_Mtx_t mtx = &mtx_var;
_Mtx_init_in_situ(mtx, _Mtx_plain);
b.func = func;
b.data = d;
b.cond = &cond;
Expand All @@ -129,8 +130,6 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Thrd_create(_Thrd_t* thr, _Thrd_start_t func
}
}
_Mtx_unlock(mtx);
_Cnd_destroy(cond);
_Mtx_destroy(mtx);
return res;
}

Expand Down
5 changes: 2 additions & 3 deletions stl/src/mutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
#include <xthreads.h>
#include <xtimec.h>

#include "primitives.hpp"

extern "C" {

// TRANSITION, ABI: exported only for ABI compat
Expand All @@ -37,7 +35,7 @@ _CRTIMP2 void __cdecl __set_stl_sync_api_mode(__stl_sync_api_modes_enum) noexcep
return reinterpret_cast<PSRWLOCK>(&mtx->_Critical_section._M_srw_lock);
}

// TRANSITION, only used when constexpr mutex constructor is not enabled
// TRANSITION, ABI: preserved for binary compatibility (and _DISABLE_CONSTEXPR_MUTEX_CONSTRUCTOR)
_CRTIMP2_PURE void __cdecl _Mtx_init_in_situ(_Mtx_t mtx, int type) noexcept { // initialize mutex in situ
new (&mtx->_Critical_section) _Stl_critical_section;
mtx->_Thread_id = -1;
Expand All @@ -51,6 +49,7 @@ _CRTIMP2_PURE void __cdecl _Mtx_destroy_in_situ(_Mtx_t mtx) noexcept { // destro
(void) mtx;
}

// TRANSITION, ABI: preserved for binary compatibility
_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_init(_Mtx_t* mtx, int type) noexcept { // initialize mutex
*mtx = nullptr;

Expand Down
62 changes: 21 additions & 41 deletions stl/src/primitives.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,47 +5,27 @@

#include <__msvc_threads_core.hpp>
#include <cstdlib>
#include <type_traits>

#include <Windows.h>

namespace Concurrency {
namespace details {
class stl_condition_variable_win7 {
public:
stl_condition_variable_win7() = default;

~stl_condition_variable_win7() = delete;
stl_condition_variable_win7(const stl_condition_variable_win7&) = delete;
stl_condition_variable_win7& operator=(const stl_condition_variable_win7&) = delete;

void wait(_Stl_critical_section* lock) {
if (!wait_for(lock, INFINITE)) {
_CSTD abort();
}
}

bool wait_for(_Stl_critical_section* lock, unsigned int timeout) {
return SleepConditionVariableSRW(
&m_condition_variable, reinterpret_cast<PSRWLOCK>(&lock->_M_srw_lock), timeout, 0)
!= 0;
}

void notify_one() {
WakeConditionVariable(&m_condition_variable);
}

void notify_all() {
WakeAllConditionVariable(&m_condition_variable);
}

private:
void* unused = nullptr; // TRANSITION, ABI: was the vptr
CONDITION_VARIABLE m_condition_variable = CONDITION_VARIABLE_INIT;
};

[[nodiscard]] inline stl_condition_variable_win7* _Get_cond_var(::_Cnd_internal_imp_t* _Cond) noexcept {
return reinterpret_cast<stl_condition_variable_win7*>(&_Cond->_Cv_storage);
}
} // namespace details
} // namespace Concurrency
inline bool _Primitive_wait_for(const _Cnd_t cond, const _Mtx_t mtx, unsigned int timeout) noexcept {
const auto pcv = reinterpret_cast<PCONDITION_VARIABLE>(&cond->_Stl_cv._Win_cv);
const auto psrw = reinterpret_cast<PSRWLOCK>(&mtx->_Critical_section._M_srw_lock);
return SleepConditionVariableSRW(pcv, psrw, timeout, 0) != 0;
}

inline void _Primitive_wait(const _Cnd_t cond, const _Mtx_t mtx) noexcept {
if (!_Primitive_wait_for(cond, mtx, INFINITE)) {
_CSTD abort();
}
}

inline void _Primitive_notify_one(const _Cnd_t cond) noexcept {
const auto pcv = reinterpret_cast<PCONDITION_VARIABLE>(&cond->_Stl_cv._Win_cv);
WakeConditionVariable(pcv);
}

inline void _Primitive_notify_all(const _Cnd_t cond) noexcept {
const auto pcv = reinterpret_cast<PCONDITION_VARIABLE>(&cond->_Stl_cv._Win_cv);
WakeAllConditionVariable(pcv);
}
3 changes: 1 addition & 2 deletions stl/src/sharedmutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,13 @@ void __stdcall _Thrd_sleep_for(const unsigned long ms) noexcept { // suspend cur

_Thrd_result __stdcall _Cnd_timedwait_for(const _Cnd_t cond, const _Mtx_t mtx, const unsigned int target_ms) noexcept {
_Thrd_result res = _Thrd_result::_Success;
const auto cs = &mtx->_Critical_section;
const auto start_ms = GetTickCount64();

// TRANSITION: replace with _Mtx_clear_owner(mtx);
mtx->_Thread_id = -1;
--mtx->_Count;

if (!Concurrency::details::_Get_cond_var(cond)->wait_for(cs, target_ms)) { // report timeout
if (!_Primitive_wait_for(cond, mtx, target_ms)) { // report timeout
const auto end_ms = GetTickCount64();
if (end_ms - start_ms >= target_ms) {
res = _Thrd_result::_Timedout;
Expand Down
5 changes: 5 additions & 0 deletions tests/libcxx/expected_results.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ std/time/time.syn/formatter.year_month_weekday.pass.cpp:1 FAIL
std/utilities/memory/specialized.algorithms/uninitialized.copy/uninitialized_copy.pass.cpp FAIL
std/utilities/memory/specialized.algorithms/uninitialized.move/uninitialized_move.pass.cpp FAIL

# LLVM-94907: [libc++][test] Avoid -Wunused-variable warnings in mutex tests
std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/default.pass.cpp:2 FAIL
std/thread/thread.mutex/thread.mutex.requirements/thread.timedmutex.requirements/thread.timedmutex.class/default.pass.cpp:2 FAIL
std/thread/thread.mutex/thread.mutex.requirements/thread.timedmutex.requirements/thread.timedmutex.recursive/default.pass.cpp:2 FAIL

# Non-Standard regex behavior.
# "It seems likely that the test is still non-conforming due to how libc++ handles the 'w' character class."
std/re/re.traits/lookup_classname.pass.cpp FAIL
Expand Down