From 253c198421aef8cf00fd009e2fe46683a20ee7ca Mon Sep 17 00:00:00 2001 From: ergo720 <45463469+ergo720@users.noreply.github.com> Date: Sat, 18 Mar 2023 11:38:39 +0100 Subject: [PATCH] Always create a wait object even when we satisfy the wait on the host side + fixed a bug in KiWaitTestNoYield This fixes an occasionl freeze in Steel Battalion + the slowness in JSRF --- src/core/hle/XAPI/Xapi.cpp | 38 ++++++++++++++++++++++++--- src/core/kernel/exports/EmuKrnl.cpp | 2 ++ src/core/kernel/exports/EmuKrnl.h | 22 +++++++++------- src/core/kernel/exports/EmuKrnlKe.cpp | 15 +++++++++++ src/core/kernel/exports/EmuKrnlKi.cpp | 2 +- src/core/kernel/exports/EmuKrnlNt.cpp | 15 +++++++++++ 6 files changed, 81 insertions(+), 13 deletions(-) diff --git a/src/core/hle/XAPI/Xapi.cpp b/src/core/hle/XAPI/Xapi.cpp index a66ffd73d..a0c1368c8 100644 --- a/src/core/hle/XAPI/Xapi.cpp +++ b/src/core/hle/XAPI/Xapi.cpp @@ -960,15 +960,47 @@ xbox::dword_xt WINAPI xbox::EMUPATCH(SignalObjectAndWait) NewTime.QuadPart += (static_cast(dwMilliseconds) * CLOCK_TIME_INCREMENT); } - xbox::dword_xt ret = WaitApc([hObjectToSignal, hObjectToWaitOn, bAlertable]() -> std::optional { + if (!Timeout || (Timeout->QuadPart == 0)) { + // Use the built-in ktimer as a dummy wait object, so that KiUnwaitThreadAndLock can still work + PKTHREAD kThread = KeGetCurrentThread(); + xbox::PKWAIT_BLOCK WaitBlock = &kThread->TimerWaitBlock; + kThread->WaitBlockList = WaitBlock; + xbox::PKTIMER Timer = &kThread->Timer; + WaitBlock->NextWaitBlock = WaitBlock; + Timer->Header.WaitListHead.Flink = &WaitBlock->WaitListEntry; + Timer->Header.WaitListHead.Blink = &WaitBlock->WaitListEntry; + InsertTailList(&Timer->Header.WaitListHead, &WaitBlock->WaitListEntry); + } + + xbox::ntstatus_xt status = WaitApc([hObjectToSignal, hObjectToWaitOn, bAlertable]() -> std::optional { DWORD dwRet = SignalObjectAndWait(hObjectToSignal, hObjectToWaitOn, 0, bAlertable); if (dwRet == WAIT_TIMEOUT) { return std::nullopt; } - return std::make_optional(dwRet); + // If the wait was satisfied with the host, then also unwait the thread on the guest side, to be sure to remove WaitBlocks that might have been added + // to the thread + xbox::ntstatus_xt Status; + switch (dwRet) + { + case WAIT_ABANDONED: Status = X_STATUS_ABANDONED; break; + case WAIT_IO_COMPLETION: Status = X_STATUS_USER_APC; break; + case WAIT_OBJECT_0: Status = X_STATUS_SUCCESS; break; + default: Status = X_STATUS_INVALID_HANDLE; + } + xbox::KiUnwaitThreadAndLock(xbox::KeGetCurrentThread(), Status, 0); + return std::make_optional(Status); }, Timeout, bAlertable, UserMode); - RETURN((ret == X_STATUS_USER_APC) ? WAIT_IO_COMPLETION : (ret == X_STATUS_TIMEOUT) ? WAIT_TIMEOUT : ret); + xbox::dword_xt ret; + switch (status) + { + case X_STATUS_ABANDONED: ret = WAIT_ABANDONED; break; + case X_STATUS_USER_APC: ret = WAIT_IO_COMPLETION; break; + case X_STATUS_SUCCESS: ret = WAIT_OBJECT_0; break; + case X_STATUS_TIMEOUT: ret = WAIT_TIMEOUT; break; + default: ret = WAIT_FAILED; + } + RETURN(ret); } // ****************************************************************** diff --git a/src/core/kernel/exports/EmuKrnl.cpp b/src/core/kernel/exports/EmuKrnl.cpp index 6934cd1b5..b8424ae5d 100644 --- a/src/core/kernel/exports/EmuKrnl.cpp +++ b/src/core/kernel/exports/EmuKrnl.cpp @@ -85,6 +85,8 @@ void InsertTailList(xbox::PLIST_ENTRY pListHead, xbox::PLIST_ENTRY pEntry) //#define RemoveEntryList(e) do { PLIST_ENTRY f = (e)->Flink, b = (e)->Blink; f->Blink = b; b->Flink = f; (e)->Flink = (e)->Blink = NULL; } while (0) // Returns TRUE if the list has become empty after removing the element, FALSE otherwise. +// NOTE: this function is a mess. _EX_Flink and _EX_Flink should never be nullptr, and it should never be called on a detached element either. Try to fix +// the bugs in the caller instead of trying to handle it here with these hacks xbox::boolean_xt RemoveEntryList(xbox::PLIST_ENTRY pEntry) { xbox::PLIST_ENTRY _EX_Flink = pEntry->Flink; diff --git a/src/core/kernel/exports/EmuKrnl.h b/src/core/kernel/exports/EmuKrnl.h index 3aa2f8d8b..c735b834a 100644 --- a/src/core/kernel/exports/EmuKrnl.h +++ b/src/core/kernel/exports/EmuKrnl.h @@ -141,15 +141,16 @@ xbox::ntstatus_xt WaitApc(T &&Lambda, xbox::PLARGE_INTEGER Timeout, xbox::boolea // NOTE: kThread->Alerted is currently never set. When the alerted mechanism is implemented, the alerts should // also interrupt the wait - xbox::PKTHREAD kThread = EmuKeGetPcr()->Prcb->CurrentThread; + xbox::ntstatus_xt status; + xbox::PKTHREAD kThread = xbox::KeGetCurrentThread(); if (Timeout == nullptr) { // No timout specified, so this is an infinite wait until an alert, a user apc or the object(s) become(s) signalled kThread->State = xbox::Waiting; while (true) { if (const auto ret = SatisfyWait(Lambda, kThread, Alertable, WaitMode)) { - kThread->State = xbox::Running; - return *ret; + status = *ret; + break; } std::this_thread::yield(); @@ -158,10 +159,10 @@ xbox::ntstatus_xt WaitApc(T &&Lambda, xbox::PLARGE_INTEGER Timeout, xbox::boolea else if (Timeout->QuadPart == 0) { // A zero timeout means that we only have to check the conditions once and then return immediately if they are not satisfied if (const auto ret = SatisfyWait(Lambda, kThread, Alertable, WaitMode)) { - return *ret; + status = *ret; } else { - return X_STATUS_TIMEOUT; + status = X_STATUS_TIMEOUT; } } else { @@ -186,18 +187,21 @@ xbox::ntstatus_xt WaitApc(T &&Lambda, xbox::PLARGE_INTEGER Timeout, xbox::boolea kThread->State = xbox::Waiting; while (true) { if (const auto ret = SatisfyWait(Lambda, kThread, Alertable, WaitMode)) { - kThread->State = xbox::Running; - return *ret; + status = *ret; + break; } if (setup_ktimer && (kThread->State == xbox::Ready)) { - kThread->State = xbox::Running; - return kThread->WaitStatus; + status = kThread->WaitStatus; + break; } std::this_thread::yield(); } } + + kThread->State = xbox::Running; + return status; } #endif diff --git a/src/core/kernel/exports/EmuKrnlKe.cpp b/src/core/kernel/exports/EmuKrnlKe.cpp index b39bc60ff..4e7bf7a0b 100644 --- a/src/core/kernel/exports/EmuKrnlKe.cpp +++ b/src/core/kernel/exports/EmuKrnlKe.cpp @@ -733,6 +733,18 @@ XBSYSAPI EXPORTNUM(99) xbox::ntstatus_xt NTAPI xbox::KeDelayExecutionThread // We can't remove NtDll::NtDelayExecution until all APCs queued by Io are implemented by our kernel as well // Test case: Metal Slug 3 + if (!Interval || (Interval->QuadPart == 0)) { + // Use the built-in ktimer as a dummy wait object, so that KiUnwaitThreadAndLock can still work + PKTHREAD kThread = KeGetCurrentThread(); + xbox::PKWAIT_BLOCK WaitBlock = &kThread->TimerWaitBlock; + kThread->WaitBlockList = WaitBlock; + xbox::PKTIMER Timer = &kThread->Timer; + WaitBlock->NextWaitBlock = WaitBlock; + Timer->Header.WaitListHead.Flink = &WaitBlock->WaitListEntry; + Timer->Header.WaitListHead.Blink = &WaitBlock->WaitListEntry; + InsertTailList(&Timer->Header.WaitListHead, &WaitBlock->WaitListEntry); + } + xbox::ntstatus_xt ret = WaitApc([Alertable]() -> std::optional { NtDll::LARGE_INTEGER ExpireTime; ExpireTime.QuadPart = 0; @@ -741,6 +753,9 @@ XBSYSAPI EXPORTNUM(99) xbox::ntstatus_xt NTAPI xbox::KeDelayExecutionThread if (Status >= 0 && Status != STATUS_ALERTED && Status != STATUS_USER_APC) { return std::nullopt; } + // If the wait was satisfied with the host, then also unwait the thread on the guest side, to be sure to remove WaitBlocks that might have been added + // to the thread. Test case: Steel Battalion + xbox::KiUnwaitThreadAndLock(xbox::KeGetCurrentThread(), Status, 0); return std::make_optional(Status); }, Interval, Alertable, WaitMode); diff --git a/src/core/kernel/exports/EmuKrnlKi.cpp b/src/core/kernel/exports/EmuKrnlKi.cpp index b48db1b94..958540554 100644 --- a/src/core/kernel/exports/EmuKrnlKi.cpp +++ b/src/core/kernel/exports/EmuKrnlKi.cpp @@ -1151,7 +1151,7 @@ xbox::void_xt xbox::KiWaitTestNoYield /* Now do the rest of the unwait */ KiUnwaitThread(WaitThread, WaitBlock->WaitKey, Increment); NextWaitEntry: - WaitEntry = WaitList->Flink; + WaitEntry = WaitEntry->Flink; } KiWaitListUnlock(); } diff --git a/src/core/kernel/exports/EmuKrnlNt.cpp b/src/core/kernel/exports/EmuKrnlNt.cpp index cb2c8f324..6767e8d9f 100644 --- a/src/core/kernel/exports/EmuKrnlNt.cpp +++ b/src/core/kernel/exports/EmuKrnlNt.cpp @@ -2229,6 +2229,18 @@ XBSYSAPI EXPORTNUM(235) xbox::ntstatus_xt NTAPI xbox::NtWaitForMultipleObjectsEx // Because user APCs from NtQueueApcThread are now handled by the kernel, we need to wait for them ourselves + if (!Timeout || (Timeout->QuadPart == 0)) { + // Use the built-in ktimer as a dummy wait object, so that KiUnwaitThreadAndLock can still work + PKTHREAD kThread = KeGetCurrentThread(); + xbox::PKWAIT_BLOCK WaitBlock = &kThread->TimerWaitBlock; + kThread->WaitBlockList = WaitBlock; + xbox::PKTIMER Timer = &kThread->Timer; + WaitBlock->NextWaitBlock = WaitBlock; + Timer->Header.WaitListHead.Flink = &WaitBlock->WaitListEntry; + Timer->Header.WaitListHead.Blink = &WaitBlock->WaitListEntry; + InsertTailList(&Timer->Header.WaitListHead, &WaitBlock->WaitListEntry); + } + xbox::ntstatus_xt ret = WaitApc([Count, &nativeHandles, WaitType, Alertable]() -> std::optional { NtDll::LARGE_INTEGER ExpireTime; ExpireTime.QuadPart = 0; @@ -2241,6 +2253,9 @@ XBSYSAPI EXPORTNUM(235) xbox::ntstatus_xt NTAPI xbox::NtWaitForMultipleObjectsEx if (Status == STATUS_TIMEOUT) { return std::nullopt; } + // If the wait was satisfied with the host, then also unwait the thread on the guest side, to be sure to remove WaitBlocks that might have been added + // to the thread. Test case: Steel Battalion + xbox::KiUnwaitThreadAndLock(xbox::KeGetCurrentThread(), Status, 0); return std::make_optional(Status); }, Timeout, Alertable, WaitMode);