From 82ed6e5617ca429e4f02d0a45c8af881493a03b3 Mon Sep 17 00:00:00 2001 From: J Reece Wilson Date: Wed, 9 Oct 2024 01:58:18 +0100 Subject: [PATCH] [*] AuAsync build regression [*] Fix potential for null deref under net adapters api [*] Improved generic IO WaitFor * AND * (still suxs) --- Include/Aurora/Async/AuFutures.hpp | 4 + Source/IO/Loop/Loop.cpp | 165 +++++++++++++++--------- Source/IO/Loop/WaitSingleBase.cpp | 8 +- Source/IO/Net/AuNetAdapter.NT.cpp | 11 +- Source/IO/Net/AuNetSrvWorkers.cpp | 4 + Source/Threading/AuWaitFor.cpp | 26 +++- Source/Threading/Primitives/AuEvent.cpp | 2 +- 7 files changed, 152 insertions(+), 68 deletions(-) diff --git a/Include/Aurora/Async/AuFutures.hpp b/Include/Aurora/Async/AuFutures.hpp index 024327fc..a70b749f 100644 --- a/Include/Aurora/Async/AuFutures.hpp +++ b/Include/Aurora/Async/AuFutures.hpp @@ -703,6 +703,10 @@ namespace __detail #endif #endif +#if defined(AU_LANG_CPP_20) && !defined(AU_NO_COROUTINES) + #define AU_HasCoRoutineTraitsAvailable +#endif + #if defined(AU_NO_COROUTINES) || ((defined(AURORA_COMPILER_MSVC) && defined(AU_LANG_CPP_14)) || (defined(AURORA_COMPILER_CLANG) && defined(AU_LANG_CPP_14)) || (defined(AURORA_COMPILER_CLANG) && defined(AU_LANG_CPP_17)) || (defined(AURORA_COMPILER_MSVC) && !defined(_RESUMABLE_FUNCTIONS_SUPPORTED))) #elif defined(AU_HasCoRoutinedIncluded) diff --git a/Source/IO/Loop/Loop.cpp b/Source/IO/Loop/Loop.cpp index ad1fca27..022555b9 100644 --- a/Source/IO/Loop/Loop.cpp +++ b/Source/IO/Loop/Loop.cpp @@ -134,6 +134,7 @@ namespace Aurora::IO::Loop if (lsList.size() == 1) { + AuUInt8 uFlags {}; auto pSource = lsList[0]; if (!pSource) { @@ -141,18 +142,23 @@ namespace Aurora::IO::Loop return true; } + if (!bSpin) + { + uFlags = kFlagLSTryNoSpin; + } + bool bStatus {}; if (bSleepForever) { - bStatus = pSource->WaitOn(0); + bStatus = pSource->WaitOnExt(uFlags, 0); } else if (bHasTimeOut) { - bStatus = pSource->WaitOn(optTimeoutMS.value()); + bStatus = pSource->WaitOnExt(uFlags, optTimeoutMS.value()); } else { - bStatus = pSource->IsSignaled(); + bStatus = pSource->IsSignaledExt(uFlags); } if (bStatus) @@ -173,6 +179,8 @@ namespace Aurora::IO::Loop if (!bAny) { + AuUInt32 uStartingOffset { 1 }; + auto &entryZero = lsList[0]; if (!entryZero) @@ -183,25 +191,32 @@ namespace Aurora::IO::Loop if (entryZero) { bool bStatus {}; + auto eType = entryZero->GetType(); + AuUInt8 uFlags {}; - if (bSleepForever) + if (!bSpin) { - bStatus = entryZero->WaitOn(0); + uFlags = kFlagLSTryNoSpin; + } + + if (eType == ELoopSource::eSourceMutex || + eType == ELoopSource::eSourceFastMutex) + { + bStatus = false; + uStartingOffset = 0; + goto mainAllSleep; + } + else if (bSleepForever) + { + bStatus = entryZero->WaitOnExt(uFlags, 0); } else if (bHasTimeOut) { - bStatus = entryZero->WaitOn(optTimeoutMS.value()); - } - else if (bSpin) - { - bStatus = entryZero->IsSignaled(); + bStatus = entryZero->WaitOnExt(uFlags, optTimeoutMS.value()); } else { - auto pSourceEx = AuDynamicCast(entryZero); - bStatus = pSourceEx ? - pSourceEx->IsSignaledNoSpinIfUserland() : - entryZero->IsSignaled(); + bStatus = entryZero->IsSignaledExt(uFlags); } if (!bStatus) @@ -215,66 +230,99 @@ namespace Aurora::IO::Loop } } - if (lsList.size() > 1 && + mainAllSleep: + + if (lsList.size() > uStartingOffset && (!bAvoidKrn || signaled.empty())) { - for (AU_ITERATE_N_TO_X(i, 1, lsList.size())) + for (AU_ITERATE_N(a, 2)) { - AuUInt32 uTimeoutMS {}; + bool dBreak {}; - if (uTimeoutEnd) + for (AU_ITERATE_N_TO_X(i, uStartingOffset, lsList.size())) { - auto uStartTime = Time::SteadyClockNS(); - if (uStartTime >= uTimeoutEnd) - { - #if 0 - break; - #else - bZeroTick = true; - #endif - } + AuUInt32 uTimeoutMS {}; + bool bIsMutex {}; + ELoopSource eType; - uTimeoutMS = AuNSToMS(uTimeoutEnd - uStartTime); - if (!uTimeoutMS) - { - #if 0 - break; - #else - bZeroTick = true; - #endif - } - } + auto &pCurrent = lsList[i]; - if (bZeroTick) - { - if (bSpin) + eType = pCurrent->GetType(); + bIsMutex = eType == ELoopSource::eSourceMutex || + eType == ELoopSource::eSourceFastMutex; + + if (bIsMutex ^ bool(a)) { - if (!lsList[i]->IsSignaled()) + continue; + } + + if (uTimeoutEnd && !bZeroTick) + { + auto uStartTime = Time::SteadyClockNS(); + if (uStartTime >= uTimeoutEnd) { - break; + bZeroTick = true; + } + + uTimeoutMS = AuNSToMS(uTimeoutEnd - uStartTime); + if (!uTimeoutMS) + { + bZeroTick = true; + } + } + + if (bZeroTick) + { + if (bSpin) + { + if (!pCurrent->IsSignaled()) + { + dBreak = true; + break; + } + + bSpin = false; + } + else + { + if (!pCurrent->IsSignaledExt(kFlagLSTryNoSpin)) + { + dBreak = true; + break; + } } } else { - auto pSourceEx = AuDynamicCast(lsList[i]); - if (!(pSourceEx ? - pSourceEx->IsSignaledNoSpinIfUserland() : - entryZero->IsSignaled())) + if (bSpin) { - break; + if (!pCurrent->WaitOnAbs(uTimeoutEnd)) + { + dBreak = true; + break; + } + + // TBD + bSpin = false; + } + else + { + if (!pCurrent->WaitOnAbsExt(kFlagLSTryNoSpin, uTimeoutEnd)) + { + dBreak = true; + break; + } } } - } - else - { - if (!lsList[i]->WaitOn(uTimeoutMS)) - { - break; - } + + reverseList.push_back(i); + signaled.push_back(pCurrent); } - reverseList.push_back(i); - signaled.push_back(lsList[i]); + if (dBreak || signaled.size() != lsList.size()) + { + break; + } } } @@ -328,12 +376,9 @@ namespace Aurora::IO::Loop eType == ELoopSource::eSourceFastSemaphore || eType == ELoopSource::eSourceFastEvent) { - auto pSourceEx = AuDynamicCast(pSource); - bAnyFound = true; - if ((pSourceEx && pSourceEx->IsSignaledNoSpinIfUserland()) || - (!pSourceEx && pSource->IsSignaled())) + if (pSource->IsSignaledExt(kFlagLSTryNoSpin)) { signalTemp.push_back(pSource); itr = lsList2.erase(itr); diff --git a/Source/IO/Loop/WaitSingleBase.cpp b/Source/IO/Loop/WaitSingleBase.cpp index 43e37b55..1436c066 100644 --- a/Source/IO/Loop/WaitSingleBase.cpp +++ b/Source/IO/Loop/WaitSingleBase.cpp @@ -17,7 +17,7 @@ namespace Aurora::IO::Loop bool WaitSingleBase::IsSignaled() { - return this->IsSignaledExt(0); + return WaitSingleBase::IsSignaledExt(0); } bool WaitSingleBase::IsSignaledExt(AuUInt8 uFlags) @@ -75,7 +75,7 @@ namespace Aurora::IO::Loop bool WaitSingleBase::WaitOn(AuUInt32 timeout) { - return this->WaitOnExt(0, timeout); + return WaitSingleBase::WaitOnExt(0, timeout); } bool WaitSingleBase::WaitOnExt(AuUInt8 uFlags, AuUInt32 timeout) @@ -96,12 +96,12 @@ namespace Aurora::IO::Loop AuTime::SteadyClockNS() + AuMSToNS(timeout) : 0; - return this->WaitOnAbsExt(uFlags, uEndTime); + return WaitSingleBase::WaitOnAbsExt(uFlags, uEndTime); } bool WaitSingleBase::WaitOnAbs(AuUInt64 uTimeoutAbs) { - return this->WaitOnAbsExt(0, uTimeoutAbs); + return WaitSingleBase::WaitOnAbsExt(0, uTimeoutAbs); } bool WaitSingleBase::WaitOnAbsExt(AuUInt8 uFlags, AuUInt64 uEndTime) diff --git a/Source/IO/Net/AuNetAdapter.NT.cpp b/Source/IO/Net/AuNetAdapter.NT.cpp index cb8f6ad6..05d1bb41 100644 --- a/Source/IO/Net/AuNetAdapter.NT.cpp +++ b/Source/IO/Net/AuNetAdapter.NT.cpp @@ -239,9 +239,16 @@ namespace Aurora::IO::Net adapter.uReceiveBytesPerSec = pCurrAddresses->ReceiveLinkSpeed / 8; } - if (adapter.address.Value().ip == EIPProtocol::eIPProtocolV4) + if (adapter.address) { - adapter.index = pCurrAddresses->IfIndex; + if (adapter.address.Value().ip == EIPProtocol::eIPProtocolV4) + { + adapter.index = pCurrAddresses->IfIndex; + } + else + { + adapter.index = adaptersOut.size(); + } } else { diff --git a/Source/IO/Net/AuNetSrvWorkers.cpp b/Source/IO/Net/AuNetSrvWorkers.cpp index ae044fca..5bfd9607 100644 --- a/Source/IO/Net/AuNetSrvWorkers.cpp +++ b/Source/IO/Net/AuNetSrvWorkers.cpp @@ -63,6 +63,10 @@ namespace Aurora::IO::Net AuSPtr NetSrvWorkers::GetWorkerByIndex(AuUInt index) { AU_LOCK_GUARD(this->mutex_); + if (this->workerPool_.empty()) + { + return {}; + } return this->workerPool_[index % this->workerPool_.size()]; } diff --git a/Source/Threading/AuWaitFor.cpp b/Source/Threading/AuWaitFor.cpp index 1283f322..f6269e12 100644 --- a/Source/Threading/AuWaitFor.cpp +++ b/Source/Threading/AuWaitFor.cpp @@ -180,7 +180,31 @@ namespace Aurora::Threading { if (releasedObjects[i]) { - waitables[i]->Unlock(); + AuUInt uHandle(0); + + if (!waitables[i]->HasOSHandle(uHandle) && + uHandle == 0xFF69421) + { + // Autoreset events + // In 2020/2021, I didn't want semaphore and mutex behaviour in event iwaitable::unlock() + // The logic: + // + // AU_LOCK_GUARD(gMutex) makes sense + // + // Semaphore gSemaphore(1); // a mutex + // AU_LOCK_GUARD(gSemaphore) can make sense in academic theory only + // + // AU_LOCK_GUARD(gResetEvent) does not sense. + // in the condvar pattern, the signaling thread does the unlock. + // + // ...therefore, the ::Unlock() should not be event setters. + AuStaticCast(waitables[i])->Set(); + } + else + { + // Semaphores and Mutexes + waitables[i]->Unlock(); + } } } } diff --git a/Source/Threading/Primitives/AuEvent.cpp b/Source/Threading/Primitives/AuEvent.cpp index c55706f4..a8ddd571 100644 --- a/Source/Threading/Primitives/AuEvent.cpp +++ b/Source/Threading/Primitives/AuEvent.cpp @@ -297,7 +297,7 @@ namespace Aurora::Threading::Primitives bool EventImpl::HasOSHandle(AuMach &mach) { - mach = 0; + mach = this->bAtomicRelease_ ? 0xFF69421 : 0; return false; }