8248657: Windows: strengthening in ThreadCritical regarding memory model

Reviewed-by: dholmes, kbarrett, aph, stuefe
This commit is contained in:
Ludovic Henry 2020-07-29 10:38:28 +02:00 committed by Thomas Stuefe
parent 3349e10b7f
commit 6af643e5a1

View File

@ -35,10 +35,10 @@
// See threadCritical.hpp for details of this class.
//
static bool initialized = false;
static volatile int lock_count = -1;
static INIT_ONCE initialized = INIT_ONCE_STATIC_INIT;
static int lock_count = 0;
static HANDLE lock_event;
static DWORD lock_owner = -1;
static DWORD lock_owner = 0;
//
// Note that Microsoft's critical region code contains a race
@ -51,48 +51,36 @@ static DWORD lock_owner = -1;
// and found them ~30 times slower than the critical region code.
//
ThreadCritical::ThreadCritical() {
DWORD current_thread = GetCurrentThreadId();
static BOOL WINAPI initialize(PINIT_ONCE InitOnce, PVOID Parameter, PVOID *Context) {
lock_event = CreateEvent(NULL, false, true, NULL);
assert(lock_event != NULL, "unexpected return value from CreateEvent");
return true;
}
ThreadCritical::ThreadCritical() {
InitOnceExecuteOnce(&initialized, &initialize, NULL, NULL);
DWORD current_thread = GetCurrentThreadId();
if (lock_owner != current_thread) {
// Grab the lock before doing anything.
while (Atomic::cmpxchg(&lock_count, -1, 0) != -1) {
if (initialized) {
DWORD ret = WaitForSingleObject(lock_event, INFINITE);
assert(ret == WAIT_OBJECT_0, "unexpected return value from WaitForSingleObject");
}
}
// Make sure the event object is allocated.
if (!initialized) {
// Locking will not work correctly unless this is autoreset.
lock_event = CreateEvent(NULL, false, false, NULL);
initialized = true;
}
assert(lock_owner == -1, "Lock acquired illegally.");
DWORD ret = WaitForSingleObject(lock_event, INFINITE);
assert(ret == WAIT_OBJECT_0, "unexpected return value from WaitForSingleObject");
lock_owner = current_thread;
} else {
// Atomicity isn't required. Bump the recursion count.
lock_count++;
}
assert(lock_owner == GetCurrentThreadId(), "Lock acquired illegally.");
// Atomicity isn't required. Bump the recursion count.
lock_count++;
}
ThreadCritical::~ThreadCritical() {
assert(lock_owner == GetCurrentThreadId(), "unlock attempt by wrong thread");
assert(lock_count >= 0, "Attempt to unlock when already unlocked");
lock_count--;
if (lock_count == 0) {
// We're going to unlock
lock_owner = -1;
lock_count = -1;
lock_owner = 0;
// No lost wakeups, lock_event stays signaled until reset.
DWORD ret = SetEvent(lock_event);
assert(ret != 0, "unexpected return value from SetEvent");
} else {
// Just unwinding a recursive lock;
lock_count--;
}
}