Wait for IOP threads in WaitSema outside a thread context - #266
Open
drakolordx7 wants to merge 1 commit into
Open
drakolordx7 wants to merge 1 commit into
drakolordx7 wants to merge 1 commit into
Conversation
RPC server functions and module start routines run synchronously (IopRpcBridge::handleRpc / loadImage -> callFunction), outside any IOP thread. WaitSema there found the count at 0, had no thread to block, and returned at once without taking the semaphore. The caller then ran its critical section while the IOP thread that holds the semaphore was preempted inside its own, because IOP threads are cut off wherever their slice ends. Found in Killzone's PFILE_R.IRX: the read-request RPC appends to a streaming queue under a semaphore, and the streaming thread removes finished requests under the same one. An append that landed between the streaming thread's `next = cur->next` and `if (next == 0) tail = 0` was dropped, the completion SIF command was never sent, and the EE waited forever (intermittent boot/loading hang). With the thread states traced at each contended wait, the holder was preempted inside the critical section in every hung boot. 5 of 12 headless boots hung before this change, 0 of 12 after. Do what the real IOP does: the waiting code blocks and the holder runs until it signals. waitSemaphoreOutsideThread runs the IOP scheduler (threads, due interrupts, callbacks, timers) until the semaphore count is above 0, then WaitSema takes it as usual. While such a wait is active, SignalSema keeps the count for the waiter and ends the signalling thread's slice, so another thread cannot take the semaphore back first. The wait is skipped from interrupt handlers and guest callbacks and while the scheduler is already running, and it gives up after one second of IOP time with a warning (the previous behaviour). The body of runCycles' loop moves into scheduleStep(), shared with the wait. The new test starts a module whose start routine calls WaitSema on an empty semaphore that a thread signals after writing a marker; it fails without the change. A second case with no signaller checks the timeout. Made by drakolord and assisted with Claude Code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RPC server functions and module start routines run synchronously (
callFunction), outside any IOP thread.WaitSemathere found the count at 0, had no thread to block, and returned at once without taking the semaphore. The caller then ran its critical section while the IOP thread holding the semaphore was preempted inside its own.Seen with a game's IRX (
PFILE_R.IRXin Killzone, SCUS-97402): the read-request RPC appends to a streaming queue under a semaphore, and the streaming thread removes finished requests under the same one. An append landing between the streaming thread'snext = cur->nextandif (next == 0) tail = 0was dropped, so no completion SIF command was sent and the EE waited forever (intermittent boot/loading hang). In the Killzone port's runtime, 5 of 12 headless boots hung before the equivalent change and 0 of 12 after.Fix:
waitSemaphoreOutsideThreadruns the IOP scheduler (threads, due interrupts, callbacks, timers) until the semaphore count is above 0, thenWaitSematakes it as usual. While such a wait is active,SignalSemakeeps the count for the waiter and ends the signalling thread's slice. The wait is skipped from interrupt handlers, guest callbacks and while the scheduler is already running, and gives up after 1 s of IOP time with a warning (the old behaviour). The loop body ofrunCyclesmoves intoscheduleStep(), shared with the wait.Tests: new
ps2_iop_emulator_testscase (a module start routine waits on an empty semaphore that a thread signals; fails without the change) plus a no-signaller timeout case. ps2xIOP ctest 4/4, gs_cache ctest 45/45 andps2x_testsunchanged from main (built with-DPS2X_BUILD_TEST=ON -DPS2X_IOP_BUILD_TESTS=ON).Behaviour change: a
WaitSemaoutside a thread with no signaller now advances up to 1 s of IOP time before returning.Made by drakolord and assisted with Claude Code.