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

Wait condition_variable_any for steady_clock #4755

Merged
merged 5 commits into from
Aug 8, 2024
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
42 changes: 20 additions & 22 deletions stl/inc/condition_variable
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,26 @@ public:
}

const auto _Rel_time_ms = _Clamped_rel_time_ms_count(_Rel_time);
const cv_status _Result = _Wait_for_ms_count(_Lck, _Rel_time_ms._Count);
if (_Rel_time_ms._Clamped) {
return cv_status::no_timeout;
}

return _Result;
// wait for signal with timeout
const shared_ptr<mutex> _Ptr = _Myptr; // for immunity to *this destruction
const auto _Start = _CHRONO steady_clock::now();
const auto _Target = _Start + _Rel_time;

unique_lock<mutex> _Guard{*_Ptr};
_Unlock_guard<_Lock> _Unlock_outer{_Lck};
const _Thrd_result _Res = _Cnd_timedwait_for_unchecked(_Mycnd(), _Ptr->_Mymtx(), _Rel_time_ms._Count);
_Guard.unlock();

if (_Res == _Thrd_result::_Success) {
return cv_status::no_timeout; // Real or WinAPI-internal spurious wake
} else if (_Rel_time_ms._Clamped) {
return cv_status::no_timeout; // Spuriously wake due to clamped timeout
} else if (_CHRONO steady_clock::now() < _Target) {
return cv_status::no_timeout; // Spuriously wake due to premature timeout
} else {
return cv_status::timeout;
}
}

template <class _Lock, class _Rep, class _Period, class _Predicate>
Expand Down Expand Up @@ -202,7 +216,7 @@ public:
}

const unsigned long _Rel_ms_count = _Clamped_rel_time_ms_count(_Abs_time - _Now)._Count;
(void) _Cnd_timedwait_for(_Mycnd(), _Myptr->_Mymtx(), _Rel_ms_count);
(void) _Cnd_timedwait_for_unchecked(_Mycnd(), _Myptr->_Mymtx(), _Rel_ms_count);
_Guard_unlocks_before_locking_outer.unlock();
} // relock

Expand All @@ -223,22 +237,6 @@ private:
_NODISCARD _Cnd_t _Mycnd() noexcept {
return &_Cnd_storage;
}

template <class _Lock>
cv_status _Wait_for_ms_count(_Lock& _Lck, const unsigned int _Rel_ms_count) {
// wait for signal with timeout
const shared_ptr<mutex> _Ptr = _Myptr; // for immunity to *this destruction
unique_lock<mutex> _Guard{*_Ptr};
_Unlock_guard<_Lock> _Unlock_outer{_Lck};
const _Thrd_result _Res = _Cnd_timedwait_for(_Mycnd(), _Ptr->_Mymtx(), _Rel_ms_count);
_Guard.unlock();

if (_Res == _Thrd_result::_Success) {
return cv_status::no_timeout;
} else {
return cv_status::timeout;
}
}
};

_EXPORT_STD inline void notify_all_at_thread_exit(condition_variable& _Cnd, unique_lock<mutex> _Lck) noexcept
Expand Down
2 changes: 1 addition & 1 deletion stl/inc/mutex
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ public:

const unsigned long _Rel_ms_count = _Clamped_rel_time_ms_count(_Abs_time - _Now)._Count;

const _Thrd_result _Res = _Cnd_timedwait_for(_Mycnd(), _Lck.mutex()->_Mymtx(), _Rel_ms_count);
const _Thrd_result _Res = _Cnd_timedwait_for_unchecked(_Mycnd(), _Lck.mutex()->_Mymtx(), _Rel_ms_count);
if (_Res == _Thrd_result::_Success) {
return cv_status::no_timeout;
}
Expand Down
3 changes: 2 additions & 1 deletion stl/inc/xthreads.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_signal(_Cnd_t) noexcept; // TRANSITION,
_CRTIMP2_PURE void __cdecl _Cnd_register_at_thread_exit(_Cnd_t, _Mtx_t, int*) noexcept;
_CRTIMP2_PURE void __cdecl _Cnd_unregister_at_thread_exit(_Mtx_t) noexcept;
_CRTIMP2_PURE void __cdecl _Cnd_do_broadcast_at_thread_exit() noexcept;
_Thrd_result __stdcall _Cnd_timedwait_for(_Cnd_t, _Mtx_t, unsigned int) noexcept;
// '_unchecked' means it is not checked against the 'steady_clock', so may report timeout prematurely
_Thrd_result __stdcall _Cnd_timedwait_for_unchecked(_Cnd_t, _Mtx_t, unsigned int) noexcept;
} // extern "C"

_STD_BEGIN
Expand Down
45 changes: 31 additions & 14 deletions stl/src/sharedmutex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,25 +41,42 @@ void __stdcall _Thrd_sleep_for(const unsigned long ms) noexcept { // suspend cur
Sleep(ms);
}

_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 start_ms = GetTickCount64();
namespace {
_Thrd_result __stdcall _Cnd_timedwait_for_impl(
const _Cnd_t cond, const _Mtx_t mtx, const unsigned int target_ms, const bool checked) noexcept {
_Thrd_result res = _Thrd_result::_Success;
unsigned long long start_ms = 0;

if (checked) {
start_ms = GetTickCount64();
}

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

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;
if (!_Primitive_wait_for(cond, mtx, target_ms)) { // report timeout
if (!checked || GetTickCount64() - start_ms >= target_ms) {
res = _Thrd_result::_Timedout;
}
}
// TRANSITION: replace with _Mtx_reset_owner(mtx);
mtx->_Thread_id = static_cast<long>(GetCurrentThreadId());
++mtx->_Count;

return res;
}
// TRANSITION: replace with _Mtx_reset_owner(mtx);
mtx->_Thread_id = static_cast<long>(GetCurrentThreadId());
++mtx->_Count;
} // namespace

return res;
// TRANSITION, ABI: preserved for compatibility
_Thrd_result __stdcall _Cnd_timedwait_for(const _Cnd_t cond, const _Mtx_t mtx, const unsigned int target_ms) noexcept {
return _Cnd_timedwait_for_impl(cond, mtx, target_ms, true);
}

_Thrd_result __stdcall _Cnd_timedwait_for_unchecked(
const _Cnd_t cond, const _Mtx_t mtx, const unsigned int target_ms) noexcept {
return _Cnd_timedwait_for_impl(cond, mtx, target_ms, false);
}


} // extern "C"
8 changes: 0 additions & 8 deletions tests/libcxx/expected_results.txt
Original file line number Diff line number Diff line change
Expand Up @@ -467,14 +467,6 @@ std/input.output/string.streams/stringbuf/stringbuf.members/view.pass.cpp FAIL
std/input.output/syncstream/syncbuf/syncstream.syncbuf.cons/dtor.pass.cpp FAIL
std/input.output/syncstream/syncbuf/syncstream.syncbuf.members/emit.pass.cpp FAIL

# GH-4723: <condition_variable>: condition_variable_any::wait_for returns cv_status::timeout when the elapsed time is shorter than requested
std/thread/thread.condition/thread.condition.condvarany/notify_all.pass.cpp SKIPPED
std/thread/thread.condition/thread.condition.condvarany/notify_one.pass.cpp SKIPPED
std/thread/thread.condition/thread.condition.condvarany/wait_for_pred.pass.cpp SKIPPED
std/thread/thread.condition/thread.condition.condvarany/wait_for.pass.cpp SKIPPED
std/thread/thread.condition/thread.condition.condvarany/wait_until_pred.pass.cpp SKIPPED
std/thread/thread.condition/thread.condition.condvarany/wait_until.pass.cpp SKIPPED


# *** VCRUNTIME BUGS ***
# DevCom-10373274 VSO-1824997 "vcruntime nothrow array operator new falls back on the wrong function"
Expand Down