AuroraSupport/AuroraRuntime
2
1
Fork 0
Code Issues 27 Pull Requests Releases Wiki Activity

[*] Harden Windows XP - 7 condvar; where signalers can lock up pending scheduling n threads whilst a spurious wake is occurring and trying to preemptively require the lock under the signal

Browse Source
This commit is contained in:
Reece Wilson 2024-01-27 11:46:18 +00:00
parent d1c668b2c1
commit 2943ffdbc2
2 changed files with 49 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

102
Source/Threading/Primitives/AuConditionVariable.NT.cpp
View File

@ -26,6 +26,21 @@ namespace Aurora::Threading::Primitives
#endif #endif
} }
void ConditionVariableNT::AddWaiter()
{
while (true)
{
auto uNow = this->wlist;
auto waiting = uNow >> kShiftCountByBits;
auto uNext = ((waiting + 1) << kShiftCountByBits) | (!bool(waiting)) | (uNow & 1);
if (AuAtomicCompareExchange(&this->wlist, uNext, uNow) == uNow)
{
break;
}
}
}
bool ConditionVariableNT::WaitForSignalNsEx(Win32ConditionMutex *pMutex, AuUInt64 qwTimeout, bool bSpin) bool ConditionVariableNT::WaitForSignalNsEx(Win32ConditionMutex *pMutex, AuUInt64 qwTimeout, bool bSpin)
{ {
#if !defined(AURORA_FORCE_SRW_LOCKS) #if !defined(AURORA_FORCE_SRW_LOCKS)
@ -39,6 +54,10 @@ namespace Aurora::Threading::Primitives
auto uTargetTimeNt = AuTime::ConvertTimestampNs(uEndTimeWall); auto uTargetTimeNt = AuTime::ConvertTimestampNs(uEndTimeWall);
bool bIOU {}; bool bIOU {};
this->AddWaiter();
pMutex->Unlock();
while (true) while (true)
{ {
LARGE_INTEGER word; LARGE_INTEGER word;
@ -46,20 +65,6 @@ namespace Aurora::Threading::Primitives
if (bRet) if (bRet)
{ {
while (true)
{
auto uNow = this->wlist;
auto waiting = uNow >> kShiftCountByBits;
auto uNext = ((waiting + 1) << kShiftCountByBits) | (!bool(waiting)) | (uNow & 1);
if (AuAtomicCompareExchange(&this->wlist, uNext, uNow) == uNow)
{
break;
}
}
pMutex->Unlock();
if (gUseNativeWaitCondvar) if (gUseNativeWaitCondvar)
{ {
// Reverted: 5b495f7fd9495aa55395666e166ac499955215dc // Reverted: 5b495f7fd9495aa55395666e166ac499955215dc
@ -93,10 +98,8 @@ namespace Aurora::Threading::Primitives
} }
} }
bRet = pNtWaitForKeyedEvent(gKeyedEventHandle, (void *)&this->wlist, 0, &word) != NTSTATUS_TIMEOUT; bRet = pNtWaitForKeyedEvent(gKeyedEventHandle, (void *)&this->wlist, 0, &word) == 0;
} }
pMutex->Lock();
} }
else /* unblock NtReleaseKeyedEvent after an atomic this->wlist change <-> NtReleaseKeyedEvent race condition. */ else /* unblock NtReleaseKeyedEvent after an atomic this->wlist change <-> NtReleaseKeyedEvent race condition. */
{ /* this->wlist waiters should still be accounting for us, leading to a NtReleaseKeyedEvent block condition*/ { /* this->wlist waiters should still be accounting for us, leading to a NtReleaseKeyedEvent block condition*/
@ -105,8 +108,6 @@ namespace Aurora::Threading::Primitives
if (gUseNativeWaitCondvar) if (gUseNativeWaitCondvar)
{ {
pMutex->Unlock();
// do not do a syscall (worst case) under ::unlock(...) barrier/pMutex's lock // do not do a syscall (worst case) under ::unlock(...) barrier/pMutex's lock
// best case: rdtsc extrapolation, if nts hal trusts our system // best case: rdtsc extrapolation, if nts hal trusts our system
if (uEndTimeSteady <= AuTime::SteadyClockNS()) if (uEndTimeSteady <= AuTime::SteadyClockNS())
@ -121,35 +122,31 @@ namespace Aurora::Threading::Primitives
} }
else else
{ {
pMutex->Unlock();
// Obligatory Windows XP+ resched // Obligatory Windows XP+ resched
bRet = pNtWaitForKeyedEvent(gKeyedEventHandle, (void *)&this->wlist, 0, nullptr) != NTSTATUS_TIMEOUT; bRet = pNtWaitForKeyedEvent(gKeyedEventHandle, (void *)&this->wlist, 0, nullptr) == 0;
} }
pMutex->Lock();
} }
if (!bRet) if (!bRet)
{ {
auto uNow = this->wlist; while (true)
auto uOld = (uNow >> kShiftCountByBits);
if (uOld == 0)
{ {
continue; auto uNow = this->wlist;
} auto uOld = (uNow >> kShiftCountByBits);
auto waiting = uOld - 1u; if (uOld == 0)
auto uNext = waiting << kShiftCountByBits; {
break;
}
if (AuAtomicCompareExchange(&this->wlist, uNext, uNow) == uNow) auto waiting = uOld - 1u;
{ auto uNext = waiting << kShiftCountByBits;
return bIOU;
} if (AuAtomicCompareExchange(&this->wlist, uNext, uNow) == uNow)
else {
{ pMutex->Lock();
continue; return bIOU;
}
} }
} }
else else
@ -157,30 +154,24 @@ namespace Aurora::Threading::Primitives
if (bIOU || if (bIOU ||
(bSpin ? this->CheckOut() : this->CheckOutNoSpin())) (bSpin ? this->CheckOut() : this->CheckOutNoSpin()))
{ {
pMutex->Lock();
return true; return true;
} }
this->AddWaiter();
} }
} }
} }
else else
{ {
bool bIOU {}; bool bIOU {};
this->AddWaiter();
pMutex->Unlock();
while (true) while (true)
{ {
while (true)
{
auto uNow = this->wlist;
auto waiting = uNow >> kShiftCountByBits;
auto uNext = ((waiting + 1) << kShiftCountByBits) | (!bool(waiting)) | (uNow & 1);
if (AuAtomicCompareExchange(&this->wlist, uNext, uNow) == uNow)
{
break;
}
}
pMutex->Unlock();
if (gUseNativeWaitCondvar) if (gUseNativeWaitCondvar)
{ {
if (bSpin && if (bSpin &&
@ -215,13 +206,14 @@ namespace Aurora::Threading::Primitives
pNtWaitForKeyedEvent(gKeyedEventHandle, (void *)&this->wlist, 0, nullptr); pNtWaitForKeyedEvent(gKeyedEventHandle, (void *)&this->wlist, 0, nullptr);
} }
pMutex->Lock();
if (bIOU || if (bIOU ||
(bSpin ? this->CheckOut() : this->CheckOutNoSpin())) (bSpin ? this->CheckOut() : this->CheckOutNoSpin()))
{ {
pMutex->Lock();
return bRet; return bRet;
} }
this->AddWaiter();
} }
} }
#else #else

3
Source/Threading/Primitives/AuConditionVariable.NT.hpp
View File

@ -21,7 +21,8 @@ namespace Aurora::Threading::Primitives
void Signal(); void Signal();
void Broadcast(); void Broadcast();
void BroadcastN(AuUInt32 nBroadcast); void BroadcastN(AuUInt32 nBroadcast);
auline void AddWaiter();
auline bool CheckOut(); auline bool CheckOut();
auline bool CheckOutNoSpin(); auline bool CheckOutNoSpin();