[*] Harden against WaitNode bAlive race condition

This commit is contained in:
Reece Wilson 2024-11-23 20:01:22 +00:00
parent 2ed05ce8dd
commit 6ac47bccf9
2 changed files with 22 additions and 11 deletions

View File

@ -669,23 +669,32 @@ namespace Aurora::Threading
{
decltype(pCurrentHead) pBefore {};
#if !defined(WOA_SEMAPHORE_MODE) && !defined(WOA_STRICTER_FIFO)
// Condvar wait-list insertion barrier
#if !defined(WOA_SEMAPHORE_MODE) && defined(WOA_STRICTER_FIFO)
// do { ... | bAlive consumer | } while (...) lock guard
AU_LOCK_GUARD(pCurrentHead->mutex);
#elif !defined(WOA_SEMAPHORE_MODE) && !defined(WOA_STRICTER_FIFO)
// Condvar wait-list insertion barrier required for binary-semaphore-like conditions.
// We only need to lock against the { lock() if (should Sleep) { *** waitList++; *** unlock(); yield(); lock() } else { ... } unlocks() } condvar pattern.
// I often only care about the order of ->Signal() after the CondVar::waitList++ to ensure one state change is paired with one waitList signal.
// Don't block during the wakeup check, we only care about observing the condvar or semaphore waitlist, no codeguarding mutex required.
{
AU_LOCK_GUARD(pCurrentHead->mutex);
}
#elif !defined(WOA_SEMAPHORE_MODE)
// do { ... | bAlive consumer | } while (...) lock guard
AU_LOCK_GUARD(pCurrentHead->mutex);
#endif
AuAtomicStore<AuUInt8>(&pCurrentHead->bAlive, 0);
auto [bCont, bRemove] = callback(*pCurrentHead);
pBefore = pCurrentHead->pBefore;
if (bRemove)
{
this->RemoveEntry(pCurrentHead, true);
this->RemoveEntry<true>(pCurrentHead);
}
else
{
AuAtomicStore<AuUInt8>(&pCurrentHead->bAlive, 1);
}
if (!bCont)
@ -707,8 +716,8 @@ namespace Aurora::Threading
return bRetStatus;
}
void ProcessWaitNodeContainer::RemoveEntry(WaitEntry *pEntry,
bool bAllUnderLock)
template <bool bAllUnderLock>
void ProcessWaitNodeContainer::RemoveEntry(WaitEntry *pEntry)
{
if (this->waitList.pHead == pEntry)
{
@ -734,7 +743,7 @@ namespace Aurora::Threading
{
pEntry->pBefore = nullptr;
pEntry->pNext = nullptr;
pEntry->bAlive = false;
//pEntry->bAlive = false; - redundant
}
}
@ -742,7 +751,7 @@ namespace Aurora::Threading
{
{
this->Lock();
this->RemoveEntry(pSelf, false);
this->RemoveEntry<false>(pSelf);
this->Unlock();
}

View File

@ -125,7 +125,9 @@ namespace Aurora::Threading
bool IterateWake(T callback);
void RemoveSelf(WaitEntry *pSelf);
void RemoveEntry(WaitEntry *pSelf, bool bAllUnderLock);
template <bool bAllUnderLock>
void RemoveEntry(WaitEntry *pSelf);
void Lock();