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.

Sign up for GitHub

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

xnotify.cpp: SRWLOCK to protect the at-thread-exit data instead of an indexed CRITICAL_SECTION #4408

Merged
merged 4 commits into from
Feb 27, 2024
Merged

xnotify.cpp: SRWLOCK to protect the at-thread-exit data instead of an indexed CRITICAL_SECTION #4408

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3c77f1b
`SRWLOCK` for `_Cnd_register_at_thread_exit`
AlexGuteniev Feb 18, 2024
6675cdb
format
AlexGuteniev Feb 18, 2024
1a63a45
revert size of table reduction
AlexGuteniev Feb 19, 2024
e823d4b
Merge branch 'main' into slim_atexit
StephanTLavavej Feb 27, 2024
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
9 changes: 4 additions & 5 deletions stl/inc/yvals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,11 +309,10 @@ _EMIT_STL_WARNING(STL4001, "/clr:pure is deprecated and will be REMOVED.");
#endif
#endif // !defined(_CRTDATA2_IMPORT)

#define _LOCK_LOCALE 0
#define _LOCK_MALLOC 1
#define _LOCK_STREAM 2
#define _LOCK_DEBUG 3
#define _LOCK_AT_THREAD_EXIT 4
#define _LOCK_LOCALE 0
#define _LOCK_MALLOC 1
#define _LOCK_STREAM 2
#define _LOCK_DEBUG 3

#ifndef _STD_ATOMIC_ALWAYS_USE_CMPXCHG16B
#if _STL_WIN32_WINNT >= _STL_WIN32_WINNT_WINBLUE && defined(_WIN64)
Expand Down
12 changes: 1 addition & 11 deletions stl/src/xlock.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

_STD_BEGIN

constexpr int _Max_lock = 8; // must be power of two
constexpr int _Max_lock = 8; // must be power of two; TRANSITION, ABI: may be less now
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No change requested: It would actually be ABI-safe to reduce _Max_lock to 4, assuming that nobody external was ever hijacking our internal APIs for their own nefarious purposes. I might do this in a followup PR. I noticed it too late to update this comment and it's not worth resetting testing.

(The remaining _LOCK_LOCALE, _LOCK_STREAM, and _LOCK_DEBUG constants must remain unchanged so that old-compiled code and new-compiled code can agree on what they're locking. We're just fortunate that they all happened to be less than 4.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know. As vNext is clearly not happening soon, we might want to keep some mutexes around for C++26 features or whatever.


#pragma warning(disable : 4074)
#pragma init_seg(compiler)
Expand Down Expand Up @@ -121,14 +121,4 @@ void __cdecl _Lockit::_Lockit_dtor(int kind) noexcept { // unlock the mutex
_Mtxunlock(&mtx[kind & (_Max_lock - 1)]);
}
}

extern "C" {
void _Lock_at_thread_exit_mutex() noexcept { // lock the at-thread-exit mutex
_Mtxlock(&mtx[_LOCK_AT_THREAD_EXIT]);
}
void _Unlock_at_thread_exit_mutex() noexcept { // unlock the at-thread-exit mutex
_Mtxunlock(&mtx[_LOCK_AT_THREAD_EXIT]);
}
} // extern "C"

_STD_END
15 changes: 6 additions & 9 deletions stl/src/xnotify.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

#include <cstdlib>
#include <mutex>
#include <xthreads.h>

#include <Windows.h>
Expand All @@ -23,20 +24,19 @@ namespace {
};

_At_thread_exit_block _Thread_exit_data;

constinit std::mutex _Thread_exit_data_mutex;
} // unnamed namespace

extern "C" {

void _Lock_at_thread_exit_mutex() noexcept;
void _Unlock_at_thread_exit_mutex() noexcept;

_CRTIMP2_PURE void __cdecl _Cnd_register_at_thread_exit(_Cnd_t cnd, _Mtx_t mtx, int* p) noexcept {
// register condition variable and mutex for cleanup at thread exit

// find block with available space
_At_thread_exit_block* block = &_Thread_exit_data;

_Lock_at_thread_exit_mutex();
std::lock_guard _Lock{_Thread_exit_data_mutex};
while (block != nullptr) { // loop through list of blocks
if (block->num_used == _Nitems) { // block is full; move to next block and allocate
if (block->next == nullptr) {
Expand All @@ -58,7 +58,6 @@ _CRTIMP2_PURE void __cdecl _Cnd_register_at_thread_exit(_Cnd_t cnd, _Mtx_t mtx,
block = nullptr;
}
}
_Unlock_at_thread_exit_mutex();
}

_CRTIMP2_PURE void __cdecl _Cnd_unregister_at_thread_exit(_Mtx_t mtx) noexcept {
Expand All @@ -67,7 +66,7 @@ _CRTIMP2_PURE void __cdecl _Cnd_unregister_at_thread_exit(_Mtx_t mtx) noexcept {
// find condition variables waiting for this thread to exit
_At_thread_exit_block* block = &_Thread_exit_data;

_Lock_at_thread_exit_mutex();
std::lock_guard _Lock{_Thread_exit_data_mutex};
while (block != nullptr) { // loop through list of blocks
for (int i = 0; block->num_used != 0 && i < _Nitems; ++i) {
if (block->data[i].mtx == mtx) { // release slot
Expand All @@ -78,7 +77,6 @@ _CRTIMP2_PURE void __cdecl _Cnd_unregister_at_thread_exit(_Mtx_t mtx) noexcept {

block = block->next;
}
_Unlock_at_thread_exit_mutex();
}

_CRTIMP2_PURE void __cdecl _Cnd_do_broadcast_at_thread_exit() noexcept {
Expand All @@ -88,7 +86,7 @@ _CRTIMP2_PURE void __cdecl _Cnd_do_broadcast_at_thread_exit() noexcept {
_At_thread_exit_block* block = &_Thread_exit_data;
const unsigned int currentThreadId = _Thrd_id();

_Lock_at_thread_exit_mutex();
std::lock_guard _Lock{_Thread_exit_data_mutex};
while (block != nullptr) { // loop through list of blocks
for (int i = 0; block->num_used != 0 && i < _Nitems; ++i) {
if (block->data[i].mtx != nullptr && block->data[i].id._Id == currentThreadId) { // notify and release slot
Expand All @@ -104,7 +102,6 @@ _CRTIMP2_PURE void __cdecl _Cnd_do_broadcast_at_thread_exit() noexcept {

block = block->next;
}
_Unlock_at_thread_exit_mutex();
}

} // extern "C"
Expand Down