diff --git a/Core/HLE/sceDisplay.cpp b/Core/HLE/sceDisplay.cpp index 4f81dd093d..d657610979 100644 --- a/Core/HLE/sceDisplay.cpp +++ b/Core/HLE/sceDisplay.cpp @@ -319,8 +319,8 @@ void __DisplayShutdown() { void __DisplayVblankBeginCallback(SceUID threadID, SceUID prevCallbackId) { SceUID pauseKey = prevCallbackId == 0 ? threadID : prevCallbackId; - // This means two callbacks in a row. PSP crashes if the same callback waits inside itself (may need more testing.) - // TODO: Handle this better? + // Shouldn't happen: each nesting level pauses under its own key, and on hardware a callback can + // nest only one level (a CB wait that would go deeper never returns.) if (vblankPausedWaits.find(pauseKey) != vblankPausedWaits.end()) { return; } diff --git a/Core/HLE/sceKernelThread.cpp b/Core/HLE/sceKernelThread.cpp index 895774e174..b4132b0f9a 100644 --- a/Core/HLE/sceKernelThread.cpp +++ b/Core/HLE/sceKernelThread.cpp @@ -480,8 +480,8 @@ bool __KernelCheckThreadCallbacks(PSPThread *thread, bool force); ////////////////////////////////////////////////////////////////////////// //STATE BEGIN ////////////////////////////////////////////////////////////////////////// +// No longer used (callback nesting is tracked per thread), only kept for savestates. static int g_inCbCount = 0; -// Normally, the same as currentThread. In an interrupt, remembers the callback's thread id. static SceUID currentCallbackThreadID = 0; static int readyCallbacksCount = 0; static SceUID currentThread; @@ -1539,11 +1539,6 @@ u32 __KernelDeleteThread(SceUID threadID, int exitStatus, const char *reason) if (currentThread == threadID) __SetCurrentThread(NULL, 0, NULL); - if (currentCallbackThreadID == threadID) - { - currentCallbackThreadID = 0; - g_inCbCount = 0; - } u32 error; PSPThread *t = kernelObjects.Get(threadID, error); @@ -2120,8 +2115,6 @@ int sceKernelExitDeleteThread(int exitStatus) { uint32_t thread_attr = thread->nt.attr; uint32_t uid = thread->GetUID(); __KernelDeleteThread(currentThread, exitStatus, "thread exited with delete"); - // Temporary hack since we don't reschedule within callbacks. - g_inCbCount = 0; hleReSchedule("thread exited with delete"); @@ -3083,15 +3076,39 @@ const char *ThreadStatusToString(ThreadStatus status) { // Set while a returning mipscall's after-action runs, which may queue the next callback. static bool g_inMipsCallReturn = false; +// How many callbacks the thread is running, nested in each other (on hardware, at most two.) +static int __KernelCallbackDepth(const PSPThread *thread) { + int depth = 0; + u32 id = thread ? thread->currentMipscallId : 0; + while (id != 0 && id != (u32)-1) { + MipsCall *call = mipsCalls.get(id); + if (!call) + break; + if (call->cbId > 0) + depth++; + id = call->savedId; + } + return depth; +} + static bool __KernelIsInCallbackOnThread(const PSPThread *thread) { - return g_inCbCount > 0 && thread && thread->GetUID() == currentCallbackThreadID; + return __KernelCallbackDepth(thread) > 0; } static bool __CanExecuteCallbackNow(PSPThread *thread) { - if (currentCallbackThreadID == 0 && g_inCbCount == 0) + if (g_inMipsCallReturn) + return false; + if (!thread) + thread = __GetCurrentThread(); + if (!thread) return true; - // A thread inside a callback runs its own pending callbacks nested, from a CB wait. - return !g_inMipsCallReturn && __KernelIsInCallbackOnThread(thread); + int depth = __KernelCallbackDepth(thread); + // Not in the middle of some other mipscall. + if (depth == 0) + return thread->currentMipscallId == 0 || thread->currentMipscallId == (u32)-1; + // A thread inside a callback runs its own pending callbacks nested, from a CB wait, but only + // one level deep: going deeper hangs the thread on hardware. + return depth == 1; } // Takes ownership of afterAction. @@ -3130,9 +3147,12 @@ void __KernelCallAddress(PSPThread *thread, u32 entryPoint, PSPAction *afterActi thread->nt.waitType = WAITTYPE_NONE; } - // A wait paused by a callback nested in this one is keyed by this callback's id. - if (cbId > 0) + if (cbId > 0) { + // A wait paused by a callback nested in this one is keyed by this callback's id. thread->currentCallbackId = cbId; + // Only a CB wait inside the callback processes callbacks, not the one it interrupted. + thread->isProcessingCallbacks = false; + } __KernelChangeThreadState(thread, THREADSTATUS_READY); } @@ -3182,8 +3202,8 @@ bool __KernelExecuteMipsCallOnCurrentThread(u32 callId, bool reschedAfter) return false; } - if (g_inCbCount > 0 && !__KernelIsInCallbackOnThread(cur)) { - WARN_LOG_REPORT(Log::sceKernel, "__KernelExecuteMipsCallOnCurrentThread(): Already in a callback!"); + if (__KernelCallbackDepth(cur) > 1) { + WARN_LOG_REPORT(Log::sceKernel, "__KernelExecuteMipsCallOnCurrentThread(): Already two callbacks deep!"); } DEBUG_LOG(Log::sceKernel, "Executing mipscall %i", callId); MipsCall *call = mipsCalls.get(callId); @@ -3221,10 +3241,6 @@ bool __KernelExecuteMipsCallOnCurrentThread(u32 callId, bool reschedAfter) currentMIPS->r[MIPS_REG_A0 + i] = call->args[i]; } - if (call->cbId != 0) - g_inCbCount++; - currentCallbackThreadID = currentThread; - return true; } @@ -3277,13 +3293,6 @@ void __KernelReturnFromMipsCall() { currentMIPS->r[MIPS_REG_V1] = call->savedV1; cur->currentMipscallId = call->savedId; - // If the thread called ExitDelete, we might've already decreased g_inCbCount. - if (call->cbId != 0 && g_inCbCount > 0) { - g_inCbCount--; - } - // Returning from a nested callback leaves us inside the outer one. - currentCallbackThreadID = g_inCbCount > 0 ? cur->GetUID() : 0; - if (cur->nt.waitType != WAITTYPE_NONE && (cur->nt.status & THREADSTATUS_WAIT) != 0 && call->cbId > 0) { if (waitTypeFuncs[cur->nt.waitType].endFunc != NULL) waitTypeFuncs[cur->nt.waitType].endFunc(cur->GetUID(), cur->currentCallbackId); @@ -3437,31 +3446,31 @@ bool __KernelCheckCallbacks() { if (__IsInInterrupt() || !__KernelIsDispatchEnabled()) { return false; } - if (__KernelInCallback()) { - // A thread inside a callback runs its own callbacks nested, from a CB wait. - // TODO: Technically, other threads' callbacks can also run when a thread within a callback is waiting. - PSPThread *thread = __GetCurrentThread(); - if (__KernelIsInCallbackOnThread(thread) && thread->isWaiting() && __KernelCheckThreadCallbacks(thread, false)) { - return __KernelExecutePendingMipsCalls(thread, true); - } - return false; - } bool processed = false; - // A waiting thread takes its callbacks when it would get to run. While the current thread - // keeps running, that's only threads of better priority; the rest stay pending (and can - // still be counted or canceled.) + // A waiting thread takes its callbacks when it would get to run: only if its priority beats + // the running thread and every ready one. The rest stay pending (and can still be counted or + // canceled.) PSPThread *cur = __GetCurrentThread(); - const bool curRunning = cur && cur->isRunning(); - u32 error; + u32 bestPriority = 0xFFFFFFFF; + if (cur && cur->isRunning()) + bestPriority = cur->nt.currentPriority; + SceUID bestReady = threadReadyQueue.peek_first(); + PSPThread *bestReadyThread = bestReady != 0 ? kernelObjects.Get(bestReady, error) : nullptr; + if (bestReadyThread && bestReadyThread->nt.currentPriority < bestPriority) + bestPriority = bestReadyThread->nt.currentPriority; + for (auto iter = threadqueue.begin(); iter != threadqueue.end(); ++iter) { PSPThread *thread = kernelObjects.Get(*iter, error); - if (thread && curRunning && thread != cur && thread->nt.currentPriority >= cur->nt.currentPriority) { + if (!thread || !thread->isWaiting()) continue; - } - if (thread && __KernelCheckThreadCallbacks(thread, false)) { + if (thread != cur && thread->nt.currentPriority >= bestPriority) + continue; + if (!__CanExecuteCallbackNow(thread)) + continue; + if (__KernelCheckThreadCallbacks(thread, false)) { processed = true; } } @@ -3525,7 +3534,7 @@ void sceKernelCheckCallback() { bool __KernelInCallback() { - return (g_inCbCount != 0); + return __KernelIsInCallbackOnThread(__GetCurrentThread()); } void __KernelNotifyCallback(SceUID cbId, int notifyArg) diff --git a/Core/HLE/sceUmd.cpp b/Core/HLE/sceUmd.cpp index 40b67d3852..d9fc257606 100644 --- a/Core/HLE/sceUmd.cpp +++ b/Core/HLE/sceUmd.cpp @@ -201,8 +201,8 @@ static void __UmdBeginCallback(SceUID threadID, SceUID prevCallbackId) if (HLEKernel::VerifyWait(threadID, WAITTYPE_UMD, 1)) { - // This means two callbacks in a row. PSP crashes if the same callback runs inside itself. - // TODO: Handle this better? + // Shouldn't happen: each nesting level pauses under its own key, and on hardware a callback can + // nest only one level (a CB wait that would go deeper never returns.) if (umdPausedWaits.find(pauseKey) != umdPausedWaits.end()) return; diff --git a/pspautotests b/pspautotests index 2a58a15610..d2097ed7d8 160000 --- a/pspautotests +++ b/pspautotests @@ -1 +1 @@ -Subproject commit 2a58a15610739ee7d0cf95044f9888c399d79d5e +Subproject commit d2097ed7d840d39dddf37433726790cd2a446a1d diff --git a/test.py b/test.py index b0f26916f4..7515cbd36b 100755 --- a/test.py +++ b/test.py @@ -312,15 +312,20 @@ tests_good = [ "threads/callbacks/afterwait", "threads/callbacks/callbacks", "threads/callbacks/cancel", + "threads/callbacks/cbtimeout", "threads/callbacks/check", "threads/callbacks/count", "threads/callbacks/create", "threads/callbacks/delete", "threads/callbacks/delivery", "threads/callbacks/exit", + "threads/callbacks/intrnotify", "threads/callbacks/nested", "threads/callbacks/notify", + "threads/callbacks/otherthread", + "threads/callbacks/recursion", "threads/callbacks/refer", + "threads/callbacks/waittypes", "threads/events/events", "threads/events/cancel/cancel", "threads/events/clear/clear",