From bb5dec0dab620e54841afd232b4c2bc7eaa2355a Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Sun, 30 Jul 2023 12:37:38 +0800 Subject: [PATCH 01/10] cache the result of `GetCurrentThreadId` --- stl/src/mutex.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/stl/src/mutex.cpp b/stl/src/mutex.cpp index a63d7576bf..8527d241f4 100644 --- a/stl/src/mutex.cpp +++ b/stl/src/mutex.cpp @@ -77,10 +77,11 @@ void _Mtx_destroy(_Mtx_t mtx) { // destroy mutex } static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock mutex + const long current_thread_id = static_cast(GetCurrentThreadId()); if ((mtx->_Type & ~_Mtx_recursive) == _Mtx_plain) { // set the lock - if (mtx->_Thread_id != static_cast(GetCurrentThreadId())) { // not current thread, do lock + if (mtx->_Thread_id != current_thread_id) { // not current thread, do lock AcquireSRWLockExclusive(get_srw_lock(mtx)); - mtx->_Thread_id = static_cast(GetCurrentThreadId()); + mtx->_Thread_id = current_thread_id; } ++mtx->_Count; @@ -88,7 +89,7 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock } else { // handle timed or recursive mutex int res = WAIT_TIMEOUT; if (target == nullptr) { // no target --> plain wait (i.e. infinite timeout) - if (mtx->_Thread_id != static_cast(GetCurrentThreadId())) { + if (mtx->_Thread_id != current_thread_id) { AcquireSRWLockExclusive(get_srw_lock(mtx)); } @@ -96,7 +97,7 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock } else if (target->tv_sec < 0 || target->tv_sec == 0 && target->tv_nsec <= 0) { // target time <= 0 --> plain trylock or timed wait for time that has passed; try to lock with 0 timeout - if (mtx->_Thread_id != static_cast(GetCurrentThreadId())) { // not this thread, lock it + if (mtx->_Thread_id != current_thread_id) { // not this thread, lock it if (TryAcquireSRWLockExclusive(get_srw_lock(mtx)) != 0) { res = WAIT_OBJECT_0; } else { @@ -111,7 +112,7 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock _Timespec64_get_sys(&now); while (now.tv_sec < target->tv_sec || now.tv_sec == target->tv_sec && now.tv_nsec < target->tv_nsec) { // time has not expired - if (mtx->_Thread_id == static_cast(GetCurrentThreadId()) + if (mtx->_Thread_id == current_thread_id || TryAcquireSRWLockExclusive(get_srw_lock(mtx)) != 0) { // stop waiting res = WAIT_OBJECT_0; break; @@ -130,7 +131,7 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock res = WAIT_TIMEOUT; } } else { - mtx->_Thread_id = static_cast(GetCurrentThreadId()); + mtx->_Thread_id = current_thread_id; } } From 31adc563f65e7e205c65074e8e1e0baebb43855c Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Sun, 30 Jul 2023 12:40:49 +0800 Subject: [PATCH 02/10] logic cleanups --- stl/src/mutex.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/stl/src/mutex.cpp b/stl/src/mutex.cpp index 8527d241f4..fc18d08511 100644 --- a/stl/src/mutex.cpp +++ b/stl/src/mutex.cpp @@ -124,7 +124,7 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock } } - if (res == WAIT_OBJECT_0 || res == WAIT_ABANDONED) { + if (res == WAIT_OBJECT_0) { if (1 < ++mtx->_Count) { // check count if ((mtx->_Type & _Mtx_recursive) != _Mtx_recursive) { // not recursive, fixup count --mtx->_Count; @@ -135,20 +135,14 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock } } - switch (res) { - case WAIT_OBJECT_0: - case WAIT_ABANDONED: + if (res == WAIT_OBJECT_0) { return _Thrd_result::_Success; - - case WAIT_TIMEOUT: + } else { // WAIT_TIMEOUT if (target == nullptr || (target->tv_sec == 0 && target->tv_nsec == 0)) { return _Thrd_result::_Busy; } else { return _Thrd_result::_Timedout; } - - default: - return _Thrd_result::_Error; } } } From a11a3d1c3cdaf5de2612cda2e0d84ab3bf04871d Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Sun, 30 Jul 2023 14:11:36 +0800 Subject: [PATCH 03/10] hide `_Mtx_timedlock` and `_Mtx_getconcrtcs` --- stl/inc/xthreads.h | 2 -- stl/src/mutex.cpp | 4 ++++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/stl/inc/xthreads.h b/stl/inc/xthreads.h index e2bdf732c0..7b59f85adf 100644 --- a/stl/inc/xthreads.h +++ b/stl/inc/xthreads.h @@ -111,10 +111,8 @@ _CRTIMP2_PURE void __cdecl _Mtx_destroy_in_situ(_Mtx_t); _CRTIMP2_PURE int __cdecl _Mtx_current_owns(_Mtx_t); _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_lock(_Mtx_t); _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_trylock(_Mtx_t); -_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_timedlock(_Mtx_t, const _timespec64*); _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_unlock(_Mtx_t); // TRANSITION, ABI: Always succeeds -_CRTIMP2_PURE void* __cdecl _Mtx_getconcrtcs(_Mtx_t); _CRTIMP2_PURE void __cdecl _Mtx_clear_owner(_Mtx_t); _CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t); diff --git a/stl/src/mutex.cpp b/stl/src/mutex.cpp index fc18d08511..de95bf12c9 100644 --- a/stl/src/mutex.cpp +++ b/stl/src/mutex.cpp @@ -77,6 +77,7 @@ void _Mtx_destroy(_Mtx_t mtx) { // destroy mutex } static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock mutex + // TRANSITION, ABI: the use of `const _timespec64*` is preserved for `_Mtx_timedlock` const long current_thread_id = static_cast(GetCurrentThreadId()); if ((mtx->_Type & ~_Mtx_recursive) == _Mtx_plain) { // set the lock if (mtx->_Thread_id != current_thread_id) { // not current thread, do lock @@ -108,6 +109,7 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock } } else { // check timeout + // TRANSITION, ABI: this branch is preserved for `_Mtx_timedlock` _timespec64 now; _Timespec64_get_sys(&now); while (now.tv_sec < target->tv_sec || now.tv_sec == target->tv_sec && now.tv_nsec < target->tv_nsec) { @@ -173,6 +175,7 @@ _Thrd_result _Mtx_trylock(_Mtx_t mtx) { // attempt to lock try_mutex return mtx_do_lock(mtx, &xt); } +// TRANSITION, ABI: preserved for binary compatibility _Thrd_result _Mtx_timedlock(_Mtx_t mtx, const _timespec64* xt) { // attempt to lock timed mutex _Thrd_result res; @@ -185,6 +188,7 @@ int _Mtx_current_owns(_Mtx_t mtx) { // test if current thread owns mutex return mtx->_Count != 0 && mtx->_Thread_id == static_cast(GetCurrentThreadId()); } +// TRANSITION, ABI: preserved for binary compatibility void* _Mtx_getconcrtcs(_Mtx_t mtx) { // get internal cs impl return &mtx->_Critical_section; } From 7c03c3dc4ad32b41ef335f72cdb41e18d4fe5f8a Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 1 Aug 2023 17:51:07 -0700 Subject: [PATCH 04/10] Enhancement: Guard 6 declarations with `#ifdef _CRTBLD`, comment why. --- stl/inc/xthreads.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/stl/inc/xthreads.h b/stl/inc/xthreads.h index 7b59f85adf..bc903da70e 100644 --- a/stl/inc/xthreads.h +++ b/stl/inc/xthreads.h @@ -104,8 +104,10 @@ enum { // mutex types _Mtx_recursive = 0x100 }; -_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_init(_Mtx_t*, int); -_CRTIMP2_PURE void __cdecl _Mtx_destroy(_Mtx_t); +#ifdef _CRTBLD +_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_init(_Mtx_t*, int); // used only by _Thrd_create +_CRTIMP2_PURE void __cdecl _Mtx_destroy(_Mtx_t); // used only by _Thrd_create +#endif // _CRTBLD _CRTIMP2_PURE void __cdecl _Mtx_init_in_situ(_Mtx_t, int); _CRTIMP2_PURE void __cdecl _Mtx_destroy_in_situ(_Mtx_t); _CRTIMP2_PURE int __cdecl _Mtx_current_owns(_Mtx_t); @@ -113,8 +115,10 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_lock(_Mtx_t); _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_trylock(_Mtx_t); _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_unlock(_Mtx_t); // TRANSITION, ABI: Always succeeds -_CRTIMP2_PURE void __cdecl _Mtx_clear_owner(_Mtx_t); -_CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t); +#ifdef _CRTBLD +_CRTIMP2_PURE void __cdecl _Mtx_clear_owner(_Mtx_t); // used only by _Cnd_wait and _Cnd_timedwait +_CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t); // used only by _Cnd_wait and _Cnd_timedwait +#endif // _CRTBLD // shared mutex // these declarations must be in sync with those in sharedmutex.cpp @@ -126,8 +130,10 @@ void __cdecl _Smtx_unlock_exclusive(_Smtx_t*); void __cdecl _Smtx_unlock_shared(_Smtx_t*); // condition variables -_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_init(_Cnd_t*); -_CRTIMP2_PURE void __cdecl _Cnd_destroy(_Cnd_t); +#ifdef _CRTBLD +_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_init(_Cnd_t*); // used only by _Thrd_create +_CRTIMP2_PURE void __cdecl _Cnd_destroy(_Cnd_t); // used only by _Thrd_create +#endif // _CRTBLD _CRTIMP2_PURE void __cdecl _Cnd_init_in_situ(_Cnd_t); _CRTIMP2_PURE void __cdecl _Cnd_destroy_in_situ(_Cnd_t); _CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(_Cnd_t, _Mtx_t); // TRANSITION, ABI: Always succeeds From 0b7a6e60fad7667c2a2c8656b21ee1f5428b4340 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 1 Aug 2023 18:06:49 -0700 Subject: [PATCH 05/10] Fix bincompat: Copy `_CRTIMP2_PURE` and `__cdecl` from `xthreads.h`. Also mark `_Thrd_abort`, `_Thrd_create`, `_Thrd_current`, `_Thrd_equal`, `_Thrd_exit`, `_Thrd_start` as `__cdecl`; see MSVC-PR-135576. Wrap `cond.cpp` and `mutex.cpp` in `_EXTERN_C`, copied from `xthreads.h`. This also affects the following: * `struct _Cnd_internal_imp_t` + Good, it was already declared within `_EXTERN_C` in `xthreads.h`. * `enum class __stl_sync_api_modes_enum` + Good, it's used only as a parameter type for `__set_stl_sync_api_mode()` which is `extern "C"`. * `get_srw_lock()` + Good, it's `static`. * `mtx_do_lock()` + Good, it's `static`. --- stl/src/cond.cpp | 21 +++++++++++++-------- stl/src/cthread.cpp | 23 ++++++++++++----------- stl/src/mutex.cpp | 32 ++++++++++++++++++-------------- stl/src/xnotify.cpp | 13 +++++++++---- 4 files changed, 52 insertions(+), 37 deletions(-) diff --git a/stl/src/cond.cpp b/stl/src/cond.cpp index 157d97577e..d6e6e6c854 100644 --- a/stl/src/cond.cpp +++ b/stl/src/cond.cpp @@ -11,6 +11,8 @@ #include "primitives.hpp" +_EXTERN_C + struct _Cnd_internal_imp_t { // condition variable implementation for ConcRT typename std::_Aligned_storage::type cv; @@ -24,13 +26,13 @@ struct _Cnd_internal_imp_t { // condition variable implementation for ConcRT static_assert(sizeof(_Cnd_internal_imp_t) == _Cnd_internal_imp_size, "incorrect _Cnd_internal_imp_size"); static_assert(alignof(_Cnd_internal_imp_t) == _Cnd_internal_imp_alignment, "incorrect _Cnd_internal_imp_alignment"); -void _Cnd_init_in_situ(const _Cnd_t cond) { // initialize condition variable in situ +_CRTIMP2_PURE void __cdecl _Cnd_init_in_situ(const _Cnd_t cond) { // initialize condition variable in situ Concurrency::details::create_stl_condition_variable(cond->_get_cv()); } -void _Cnd_destroy_in_situ(_Cnd_t) {} // destroy condition variable in situ +_CRTIMP2_PURE void __cdecl _Cnd_destroy_in_situ(_Cnd_t) {} // destroy condition variable in situ -_Thrd_result _Cnd_init(_Cnd_t* const pcond) { // initialize +_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_init(_Cnd_t* const pcond) { // initialize *pcond = nullptr; const auto cond = static_cast<_Cnd_t>(_calloc_crt(1, sizeof(_Cnd_internal_imp_t))); @@ -43,14 +45,14 @@ _Thrd_result _Cnd_init(_Cnd_t* const pcond) { // initialize return _Thrd_result::_Success; } -void _Cnd_destroy(const _Cnd_t cond) { // clean up +_CRTIMP2_PURE void __cdecl _Cnd_destroy(const _Cnd_t cond) { // clean up if (cond) { // something to do, do it _Cnd_destroy_in_situ(cond); _free_crt(cond); } } -_Thrd_result _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx) { // wait until signaled +_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx) { // wait until signaled const auto cs = &mtx->_Critical_section; _Mtx_clear_owner(mtx); cond->_get_cv()->wait(cs); @@ -59,7 +61,8 @@ _Thrd_result _Cnd_wait(const _Cnd_t cond, const _Mtx_t mtx) { // wait until sign } // wait until signaled or timeout -_Thrd_result _Cnd_timedwait(const _Cnd_t cond, const _Mtx_t mtx, const _timespec64* const target) { +_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_timedwait( + const _Cnd_t cond, const _Mtx_t mtx, const _timespec64* const target) { _Thrd_result res = _Thrd_result::_Success; const auto cs = &mtx->_Critical_section; if (target == nullptr) { // no target time specified, wait on mutex @@ -81,16 +84,18 @@ _Thrd_result _Cnd_timedwait(const _Cnd_t cond, const _Mtx_t mtx, const _timespec return res; } -_Thrd_result _Cnd_signal(const _Cnd_t cond) { // release one waiting thread +_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_signal(const _Cnd_t cond) { // release one waiting thread cond->_get_cv()->notify_one(); return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds } -_Thrd_result _Cnd_broadcast(const _Cnd_t cond) { // release all waiting threads +_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_broadcast(const _Cnd_t cond) { // release all waiting threads cond->_get_cv()->notify_all(); return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds } +_END_EXTERN_C + /* * This file is derived from software bearing the following * restrictions: diff --git a/stl/src/cthread.cpp b/stl/src/cthread.cpp index 389b8ee9cd..618037bbfd 100644 --- a/stl/src/cthread.cpp +++ b/stl/src/cthread.cpp @@ -42,17 +42,17 @@ namespace { _EXTERN_C // TRANSITION, ABI: _Thrd_exit() is preserved for binary compatibility -[[noreturn]] _CRTIMP2_PURE void _Thrd_exit(int res) { // terminate execution of calling thread +[[noreturn]] _CRTIMP2_PURE void __cdecl _Thrd_exit(int res) { // terminate execution of calling thread _endthreadex(res); } // TRANSITION, ABI: _Thrd_start() is preserved for binary compatibility -_CRTIMP2_PURE _Thrd_result _Thrd_start(_Thrd_t* thr, _Thrd_callback_t func, void* b) { // start a thread +_CRTIMP2_PURE _Thrd_result __cdecl _Thrd_start(_Thrd_t* thr, _Thrd_callback_t func, void* b) { // start a thread thr->_Hnd = reinterpret_cast(_beginthreadex(nullptr, 0, func, b, 0, &thr->_Id)); return thr->_Hnd == nullptr ? _Thrd_result::_Error : _Thrd_result::_Success; } -_Thrd_result _Thrd_join(_Thrd_t thr, int* code) { // returns when thread terminates +_CRTIMP2_PURE _Thrd_result __cdecl _Thrd_join(_Thrd_t thr, int* code) { // returns when thread terminates if (WaitForSingleObjectEx(thr._Hnd, INFINITE, FALSE) == WAIT_FAILED) { return _Thrd_result::_Error; } @@ -68,11 +68,12 @@ _Thrd_result _Thrd_join(_Thrd_t thr, int* code) { // returns when thread termina return CloseHandle(thr._Hnd) ? _Thrd_result::_Success : _Thrd_result::_Error; } -_Thrd_result _Thrd_detach(_Thrd_t thr) { // tell OS to release thread's resources when it terminates +_CRTIMP2_PURE _Thrd_result __cdecl _Thrd_detach(_Thrd_t thr) { + // tell OS to release thread's resources when it terminates return CloseHandle(thr._Hnd) ? _Thrd_result::_Success : _Thrd_result::_Error; } -void _Thrd_sleep(const _timespec64* xt) { // suspend thread until time xt +_CRTIMP2_PURE void __cdecl _Thrd_sleep(const _timespec64* xt) { // suspend thread until time xt _timespec64 now; _Timespec64_get_sys(&now); do { // sleep and check time @@ -81,35 +82,35 @@ void _Thrd_sleep(const _timespec64* xt) { // suspend thread until time xt } while (now.tv_sec < xt->tv_sec || now.tv_sec == xt->tv_sec && now.tv_nsec < xt->tv_nsec); } -void _Thrd_yield() { // surrender remainder of timeslice +_CRTIMP2_PURE void __cdecl _Thrd_yield() { // surrender remainder of timeslice SwitchToThread(); } // TRANSITION, ABI: _Thrd_equal() is preserved for binary compatibility -_CRTIMP2_PURE int _Thrd_equal(_Thrd_t thr0, _Thrd_t thr1) { // return 1 if thr0 and thr1 identify same thread +_CRTIMP2_PURE int __cdecl _Thrd_equal(_Thrd_t thr0, _Thrd_t thr1) { // return 1 if thr0 and thr1 identify same thread return thr0._Id == thr1._Id; } // TRANSITION, ABI: _Thrd_current() is preserved for binary compatibility -_CRTIMP2_PURE _Thrd_t _Thrd_current() { // return _Thrd_t identifying current thread +_CRTIMP2_PURE _Thrd_t __cdecl _Thrd_current() { // return _Thrd_t identifying current thread _Thrd_t result; result._Hnd = nullptr; result._Id = GetCurrentThreadId(); return result; } -_Thrd_id_t _Thrd_id() { // return unique id for current thread +_CRTIMP2_PURE _Thrd_id_t __cdecl _Thrd_id() { // return unique id for current thread return GetCurrentThreadId(); } -unsigned int _Thrd_hardware_concurrency() { // return number of processors +_CRTIMP2_PURE unsigned int __cdecl _Thrd_hardware_concurrency() { // return number of processors SYSTEM_INFO info; GetNativeSystemInfo(&info); return info.dwNumberOfProcessors; } // TRANSITION, ABI: _Thrd_create() is preserved for binary compatibility -_CRTIMP2_PURE _Thrd_result _Thrd_create(_Thrd_t* thr, _Thrd_start_t func, void* d) { // create thread +_CRTIMP2_PURE _Thrd_result __cdecl _Thrd_create(_Thrd_t* thr, _Thrd_start_t func, void* d) { // create thread _Thrd_result res; _Thrd_binder b; int started = 0; diff --git a/stl/src/mutex.cpp b/stl/src/mutex.cpp index de95bf12c9..890a0bce71 100644 --- a/stl/src/mutex.cpp +++ b/stl/src/mutex.cpp @@ -13,7 +13,9 @@ #include "primitives.hpp" -extern "C" [[noreturn]] _CRTIMP2_PURE void _Thrd_abort(const char* msg) { // abort on precondition failure +_EXTERN_C + +[[noreturn]] _CRTIMP2_PURE void __cdecl _Thrd_abort(const char* msg) { // abort on precondition failure fputs(msg, stderr); fputc('\n', stderr); abort(); @@ -35,26 +37,26 @@ extern "C" [[noreturn]] _CRTIMP2_PURE void _Thrd_abort(const char* msg) { // abo // TRANSITION, ABI: preserved for binary compatibility enum class __stl_sync_api_modes_enum { normal, win7, vista, concrt }; -extern "C" _CRTIMP2 void __cdecl __set_stl_sync_api_mode(__stl_sync_api_modes_enum) {} +_CRTIMP2 void __cdecl __set_stl_sync_api_mode(__stl_sync_api_modes_enum) {} [[nodiscard]] static PSRWLOCK get_srw_lock(_Mtx_t mtx) { return reinterpret_cast(&mtx->_Critical_section._M_srw_lock); } // TRANSITION, only used when constexpr mutex constructor is not enabled -void _Mtx_init_in_situ(_Mtx_t mtx, int type) { // initialize mutex in situ +_CRTIMP2_PURE void __cdecl _Mtx_init_in_situ(_Mtx_t mtx, int type) { // initialize mutex in situ Concurrency::details::create_stl_critical_section(&mtx->_Critical_section); mtx->_Thread_id = -1; mtx->_Type = type; mtx->_Count = 0; } -void _Mtx_destroy_in_situ(_Mtx_t mtx) { // destroy mutex in situ +_CRTIMP2_PURE void __cdecl _Mtx_destroy_in_situ(_Mtx_t mtx) { // destroy mutex in situ _THREAD_ASSERT(mtx->_Count == 0, "mutex destroyed while busy"); (void) mtx; } -_Thrd_result _Mtx_init(_Mtx_t* mtx, int type) { // initialize mutex +_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_init(_Mtx_t* mtx, int type) { // initialize mutex *mtx = nullptr; _Mtx_t mutex = static_cast<_Mtx_t>(_calloc_crt(1, sizeof(_Mtx_internal_imp_t))); @@ -69,7 +71,7 @@ _Thrd_result _Mtx_init(_Mtx_t* mtx, int type) { // initialize mutex return _Thrd_result::_Success; } -void _Mtx_destroy(_Mtx_t mtx) { // destroy mutex +_CRTIMP2_PURE void __cdecl _Mtx_destroy(_Mtx_t mtx) { // destroy mutex if (mtx) { // something to do, do it _Mtx_destroy_in_situ(mtx); _free_crt(mtx); @@ -149,7 +151,7 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock } } -_Thrd_result _Mtx_unlock(_Mtx_t mtx) { // unlock mutex +_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_unlock(_Mtx_t mtx) { // unlock mutex _THREAD_ASSERT( 1 <= mtx->_Count && mtx->_Thread_id == static_cast(GetCurrentThreadId()), "unlock of unowned mutex"); @@ -163,11 +165,11 @@ _Thrd_result _Mtx_unlock(_Mtx_t mtx) { // unlock mutex return _Thrd_result::_Success; // TRANSITION, ABI: Always succeeds } -_Thrd_result _Mtx_lock(_Mtx_t mtx) { // lock mutex +_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_lock(_Mtx_t mtx) { // lock mutex return mtx_do_lock(mtx, nullptr); } -_Thrd_result _Mtx_trylock(_Mtx_t mtx) { // attempt to lock try_mutex +_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_trylock(_Mtx_t mtx) { // attempt to lock try_mutex _timespec64 xt; _THREAD_ASSERT((mtx->_Type & (_Mtx_try | _Mtx_timed)) != 0, "trylock not supported by mutex"); xt.tv_sec = 0; @@ -176,7 +178,7 @@ _Thrd_result _Mtx_trylock(_Mtx_t mtx) { // attempt to lock try_mutex } // TRANSITION, ABI: preserved for binary compatibility -_Thrd_result _Mtx_timedlock(_Mtx_t mtx, const _timespec64* xt) { // attempt to lock timed mutex +_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_timedlock(_Mtx_t mtx, const _timespec64* xt) { // attempt to lock timed mutex _Thrd_result res; _THREAD_ASSERT((mtx->_Type & _Mtx_timed) != 0, "timedlock not supported by mutex"); @@ -184,25 +186,27 @@ _Thrd_result _Mtx_timedlock(_Mtx_t mtx, const _timespec64* xt) { // attempt to l return res == _Thrd_result::_Busy ? _Thrd_result::_Timedout : res; } -int _Mtx_current_owns(_Mtx_t mtx) { // test if current thread owns mutex +_CRTIMP2_PURE int __cdecl _Mtx_current_owns(_Mtx_t mtx) { // test if current thread owns mutex return mtx->_Count != 0 && mtx->_Thread_id == static_cast(GetCurrentThreadId()); } // TRANSITION, ABI: preserved for binary compatibility -void* _Mtx_getconcrtcs(_Mtx_t mtx) { // get internal cs impl +_CRTIMP2_PURE void* __cdecl _Mtx_getconcrtcs(_Mtx_t mtx) { // get internal cs impl return &mtx->_Critical_section; } -void _Mtx_clear_owner(_Mtx_t mtx) { // set owner to nobody +_CRTIMP2_PURE void __cdecl _Mtx_clear_owner(_Mtx_t mtx) { // set owner to nobody mtx->_Thread_id = -1; --mtx->_Count; } -void _Mtx_reset_owner(_Mtx_t mtx) { // set owner to current thread +_CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t mtx) { // set owner to current thread mtx->_Thread_id = static_cast(GetCurrentThreadId()); ++mtx->_Count; } +_END_EXTERN_C + /* * This file is derived from software bearing the following * restrictions: diff --git a/stl/src/xnotify.cpp b/stl/src/xnotify.cpp index 64a513855d..9d44df6875 100644 --- a/stl/src/xnotify.cpp +++ b/stl/src/xnotify.cpp @@ -32,8 +32,9 @@ _EXTERN_C void _Lock_at_thread_exit_mutex(); void _Unlock_at_thread_exit_mutex(); -void _Cnd_register_at_thread_exit( - _Cnd_t cnd, _Mtx_t mtx, int* p) { // register condition variable and mutex for cleanup at thread exit +_CRTIMP2_PURE void __cdecl _Cnd_register_at_thread_exit(_Cnd_t cnd, _Mtx_t mtx, int* p) { + // register condition variable and mutex for cleanup at thread exit + // find block with available space _At_thread_exit_block* block = &_Thread_exit_data; @@ -62,7 +63,9 @@ void _Cnd_register_at_thread_exit( _Unlock_at_thread_exit_mutex(); } -void _Cnd_unregister_at_thread_exit(_Mtx_t mtx) { // unregister condition variable/mutex for cleanup at thread exit +_CRTIMP2_PURE void __cdecl _Cnd_unregister_at_thread_exit(_Mtx_t mtx) { + // unregister condition variable/mutex for cleanup at thread exit + // find condition variables waiting for this thread to exit _At_thread_exit_block* block = &_Thread_exit_data; @@ -80,7 +83,9 @@ void _Cnd_unregister_at_thread_exit(_Mtx_t mtx) { // unregister condition variab _Unlock_at_thread_exit_mutex(); } -void _Cnd_do_broadcast_at_thread_exit() { // notify condition variables waiting for this thread to exit +_CRTIMP2_PURE void __cdecl _Cnd_do_broadcast_at_thread_exit() { + // notify condition variables waiting for this thread to exit + // find condition variables waiting for this thread to exit _At_thread_exit_block* block = &_Thread_exit_data; const unsigned int currentThreadId = _Thrd_id(); From 19a88acb08791a79ec9739a6a00bd370fb9eb5f6 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 1 Aug 2023 19:03:03 -0700 Subject: [PATCH 06/10] Style: Use `auto` after `static_cast`. --- stl/src/mutex.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/src/mutex.cpp b/stl/src/mutex.cpp index 890a0bce71..fddb6c728b 100644 --- a/stl/src/mutex.cpp +++ b/stl/src/mutex.cpp @@ -80,7 +80,7 @@ _CRTIMP2_PURE void __cdecl _Mtx_destroy(_Mtx_t mtx) { // destroy mutex static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock mutex // TRANSITION, ABI: the use of `const _timespec64*` is preserved for `_Mtx_timedlock` - const long current_thread_id = static_cast(GetCurrentThreadId()); + const auto current_thread_id = static_cast(GetCurrentThreadId()); if ((mtx->_Type & ~_Mtx_recursive) == _Mtx_plain) { // set the lock if (mtx->_Thread_id != current_thread_id) { // not current thread, do lock AcquireSRWLockExclusive(get_srw_lock(mtx)); From a3783df22add133681b0562aac0b286200890d29 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 1 Aug 2023 19:09:23 -0700 Subject: [PATCH 07/10] Style: After `if`-`return`, avoid `else`. --- stl/src/mutex.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/stl/src/mutex.cpp b/stl/src/mutex.cpp index fddb6c728b..b5e195ea03 100644 --- a/stl/src/mutex.cpp +++ b/stl/src/mutex.cpp @@ -141,13 +141,15 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock if (res == WAIT_OBJECT_0) { return _Thrd_result::_Success; - } else { // WAIT_TIMEOUT - if (target == nullptr || (target->tv_sec == 0 && target->tv_nsec == 0)) { - return _Thrd_result::_Busy; - } else { - return _Thrd_result::_Timedout; - } } + + // res is WAIT_TIMEOUT here + + if (target == nullptr || (target->tv_sec == 0 && target->tv_nsec == 0)) { + return _Thrd_result::_Busy; + } + + return _Thrd_result::_Timedout; } } From 9b55a457928a86f4017945a39509a547291eeca8 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 1 Aug 2023 19:34:28 -0700 Subject: [PATCH 08/10] Comment that `_Mtx_plain` is for `_Thrd_create`. --- stl/src/mutex.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/stl/src/mutex.cpp b/stl/src/mutex.cpp index b5e195ea03..4a0ac57dca 100644 --- a/stl/src/mutex.cpp +++ b/stl/src/mutex.cpp @@ -82,6 +82,8 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock // TRANSITION, ABI: the use of `const _timespec64*` is preserved for `_Mtx_timedlock` const auto current_thread_id = static_cast(GetCurrentThreadId()); if ((mtx->_Type & ~_Mtx_recursive) == _Mtx_plain) { // set the lock + // TRANSITION, ABI: this branch is preserved for `_Thrd_create` + if (mtx->_Thread_id != current_thread_id) { // not current thread, do lock AcquireSRWLockExclusive(get_srw_lock(mtx)); mtx->_Thread_id = current_thread_id; From 9fb5e722f6977a70e0cf6a7bbc2cd2e86cf090f3 Mon Sep 17 00:00:00 2001 From: achabense <60953653+achabense@users.noreply.github.com> Date: Wed, 9 Aug 2023 14:26:34 +0800 Subject: [PATCH 09/10] remove redundant assignments --- stl/src/mutex.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/stl/src/mutex.cpp b/stl/src/mutex.cpp index 4a0ac57dca..9b1f14bf1e 100644 --- a/stl/src/mutex.cpp +++ b/stl/src/mutex.cpp @@ -105,8 +105,6 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock if (mtx->_Thread_id != current_thread_id) { // not this thread, lock it if (TryAcquireSRWLockExclusive(get_srw_lock(mtx)) != 0) { res = WAIT_OBJECT_0; - } else { - res = WAIT_TIMEOUT; } } else { res = WAIT_OBJECT_0; @@ -122,8 +120,6 @@ static _Thrd_result mtx_do_lock(_Mtx_t mtx, const _timespec64* target) { // lock || TryAcquireSRWLockExclusive(get_srw_lock(mtx)) != 0) { // stop waiting res = WAIT_OBJECT_0; break; - } else { - res = WAIT_TIMEOUT; } _Timespec64_get_sys(&now); From 61353c9056c0f3ddcaef8a8820c044c7fdfde46e Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Wed, 9 Aug 2023 18:00:18 -0700 Subject: [PATCH 10/10] Drop "// used only by MEOW" comments. --- stl/inc/xthreads.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/stl/inc/xthreads.h b/stl/inc/xthreads.h index bc903da70e..f7c1492caf 100644 --- a/stl/inc/xthreads.h +++ b/stl/inc/xthreads.h @@ -105,8 +105,8 @@ enum { // mutex types }; #ifdef _CRTBLD -_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_init(_Mtx_t*, int); // used only by _Thrd_create -_CRTIMP2_PURE void __cdecl _Mtx_destroy(_Mtx_t); // used only by _Thrd_create +_CRTIMP2_PURE _Thrd_result __cdecl _Mtx_init(_Mtx_t*, int); +_CRTIMP2_PURE void __cdecl _Mtx_destroy(_Mtx_t); #endif // _CRTBLD _CRTIMP2_PURE void __cdecl _Mtx_init_in_situ(_Mtx_t, int); _CRTIMP2_PURE void __cdecl _Mtx_destroy_in_situ(_Mtx_t); @@ -116,8 +116,8 @@ _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_trylock(_Mtx_t); _CRTIMP2_PURE _Thrd_result __cdecl _Mtx_unlock(_Mtx_t); // TRANSITION, ABI: Always succeeds #ifdef _CRTBLD -_CRTIMP2_PURE void __cdecl _Mtx_clear_owner(_Mtx_t); // used only by _Cnd_wait and _Cnd_timedwait -_CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t); // used only by _Cnd_wait and _Cnd_timedwait +_CRTIMP2_PURE void __cdecl _Mtx_clear_owner(_Mtx_t); +_CRTIMP2_PURE void __cdecl _Mtx_reset_owner(_Mtx_t); #endif // _CRTBLD // shared mutex @@ -131,8 +131,8 @@ void __cdecl _Smtx_unlock_shared(_Smtx_t*); // condition variables #ifdef _CRTBLD -_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_init(_Cnd_t*); // used only by _Thrd_create -_CRTIMP2_PURE void __cdecl _Cnd_destroy(_Cnd_t); // used only by _Thrd_create +_CRTIMP2_PURE _Thrd_result __cdecl _Cnd_init(_Cnd_t*); +_CRTIMP2_PURE void __cdecl _Cnd_destroy(_Cnd_t); #endif // _CRTBLD _CRTIMP2_PURE void __cdecl _Cnd_init_in_situ(_Cnd_t); _CRTIMP2_PURE void __cdecl _Cnd_destroy_in_situ(_Cnd_t);