Repository navigation
fix(libkernel): stop losing and deadlocking sceKernelRaiseException deliveries on Windows - #1421
Conversation
A raised exception redirects the target thread to a stub that runs the handler and returns through RtlRestoreContext. On Windows 10 (19045) RtlRestoreContext restores in user mode only when the context flags are exactly CONTEXT_FULL | CONTEXT_SEGMENTS (CONTEXT_XSTATE aside); for the CONTEXT_FULL | CONTEXT_FLOATING_POINT context it was given, it calls NtContinue. When the next raise suspended the thread inside that system call, NtContinue then replaced the redirected context with the one it was restoring, so the handler never ran although sceKernelRaiseException had returned 0. guest_raise_exception lost such a delivery to its busy thread in about one run in five and hung until the ctest timeout. The context is now read with CONTEXT_SEGMENTS, so the handler returns through the user-mode restore, and a raise that finds the thread inside NtContinue or NtContinueEx (an APC or an exception handler returning) resumes it and suspends it again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…EH exceptions The new phase's vectored handler continues every exception the thread raises, so the thread spends much of its time in NtContinue, where a redirect is lost unless the raise retries. Without the retry the phase hangs in every run; with it the test passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…elivery SuspendThread returns before the thread has stopped, and GetThreadContext waits until it has. waitCount was read right after SuspendThread, so a thread that was leaving its wait could still decrement it and drain its APCs with NtTestAlert before QueueUserAPC queued the delivery, which then waited for an alertable wait that never came. The context is now read first, so waitCount is read from a stopped thread. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hreads MinGW's thread_local goes through emutls and winpthreads' pthread_getspecific, which holds a per-thread spinlock. A raise that redirected a thread holding it ran the handler on that same thread, and a handler reading any thread_local, scePthreadSelf for one, spun on the lock forever. A raise that finds the target inside libwinpthread-1.dll now resumes it and suspends it again, as one inside NtContinue does. guest_raise_exception's new phase raises at a thread that calls scePthreadSelf in a loop and checks that the handler sees its own thread. It hung in every run without the retry. TechnicalDebt records that a handler can still reenter other host code the thread was interrupted in. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
adriadam10
left a comment
There was a problem hiding this comment.
Checked the diff, the guest_raise_exception registration (Windows only) and CI on the current head (windows, linux, conventions, report green; Linux ctest 457/457 locally, the test itself doesn't run there). Two things before merging:
- The retry in
RaiseOn(Exception.cpp:219-243) has no bound: a thread that stays insideNtContinue/NtContinueExor winpthreads makes the raise spin onSwitchToThreadforever, a hang instead of an error. Please bound it (a retry count or a deadline) and throw when it runs out, with the test covering that path if it can be forced. - #1562 fixes the same MinGW TLS deadlock in a different way and conflicts with this one. Please say in the description how the two relate: whether this one is still needed with #1562, or #1562 with this one. If they're alternatives, we'll take only one.
Also, the third change (reading the context before waitCount) has no before/after measurement; if you have one, add it to Tested.
…inpthreads The retry that waits for a target to leave NtContinue, NtContinueEx or libwinpthread-1.dll had no bound, so a target that stayed there made sceKernelRaiseException spin forever. It now throws after 1 s. A new guest_raise_exception phase keeps a thread inside NtContinue by continuing a context that calls NtContinue again, and checks that the raise throws and that the handler did not run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@adriadam10 thanks for the review. I pushed 24dc98d and updated the description.
🤖 Addressed by Claude Code |
|
The 🤖 Addressed by Claude Code |
|
I updated the comparison with #1562 in the description. Its current head 🤖 Addressed by Claude Code |
…ption-ntcontinue Keeps this branch's CONTEXT_FULL | CONTEXT_SEGMENTS read, context-first order and bounded retry in RaiseOn, and adds boykopovar#1478's ymm save fields to both deliveries. The test keeps both phase sets; the vectors phase runs after the continuing, selfing and looping phases and counts their deliveries. The TechnicalDebt entry about ymm upper halves is dropped, since boykopovar#1478 preserves them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ption-ntcontinue Keeps both new TechnicalDebt entries: this branch's note that a raise can still reenter host code it interrupted, and boykopovar#2107's sigaltstack note. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
adriadam10
left a comment
There was a problem hiding this comment.
Checked the new bound (Exception.cpp:230-255, 1 s deadline that throws), the guest_raise_exception phases covering it (NtContinue loop, scePthreadSelf in winpthreads, SEH continue) and the TechnicalDebt line; merges cleanly with current main and the Linux build and ctest are green (the test itself is Windows-only, so the Windows behaviour rests on your runs). Merging.
…exception-ntcontinue fix(libkernel): stop losing and deadlocking sceKernelRaiseException deliveries on Windows
What
On Windows,
sceKernelRaiseExceptioncould lose a delivery: it returned 0, but the handler never ran on the target thread.guest_raise_exceptionhits this in its busy-thread phase in about one run in five onmain(9619edf, after #1375) and then hangs until the 30 s ctest timeout. Three paths lost deliveries, and a fourth deadlocked them.The handler's own return went through
NtContinue. A raise suspends the target, points its context atAps5RedirectedEntryStub, and the stub returns throughRtlRestoreContext. On Windows 10 (19045),RtlRestoreContextrestores in user mode, withiretq, only whenContextFlagswithoutCONTEXT_XSTATEis exactlyCONTEXT_FULL | CONTEXT_SEGMENTS(0x10000F, read from the ntdll disassembly); otherwise it callsNtContinue. The context was read withCONTEXT_FULL | CONTEXT_FLOATING_POINT(0x10000B), so every handler returned throughNtContinue. When the next raise suspended the thread inside that system call,SetThreadContextchanged the saved user context, andNtContinuethen replaced it with the context it was restoring, so the thread went back to its loop without running the handler.CONTEXT_FULL | CONTEXT_SEGMENTS, so the handler returns through the user-mode restore, which builds itsiretqframe below the saved context and never enters the kernel.Other code returns through
NtContinuetoo. An APC or an exception handler that continues execution resumes the thread throughNtContinueorNtContinueEx.The APC path was chosen before the thread had stopped.
SuspendThreadreturns before the thread is suspended;GetThreadContextwaits until it is.waitCountwas read right afterSuspendThread, so a thread leaving its wait could still decrement it and drain its APCs withNtTestAlertbeforeQueueUserAPCqueued the delivery, which then waited for an alertable wait that never came.waitCountis read from a stopped thread.A raise could interrupt the target inside winpthreads. MinGW's
thread_localgoes through emutls and winpthreads'pthread_getspecific, which holds a per-thread spinlock. A handler that reads athread_local, such asscePthreadSelfor theTimedWaitwait state, on a thread interrupted while it held that lock spun on it forever.libwinpthread-1.dllresumes it and suspends it again, as one insideNtContinuedoes.Both retries give up after 1 s. A target that stays inside those
NtContinuestubs or winpthreads for that long makes the raise throw, instead of spinning forever. Inguest_raise_exception, the longest retry took 7.4 ms.TechnicalDebt: a new entry records two things: a handler can still reenter other host code the thread was interrupted in, such as the C runtime's heap, and the raise gives up after 1 s. The YMM entry is gone since #1478, which saves the upper halves; with
mainmerged in, its vectors phase passes with this PR'sCONTEXT_FULL | CONTEXT_SEGMENTSread.How this relates to #1562
Since
07610212, #1562 no longer changesException.cpp. It now moves libkernel's handler-visible state (the current thread, pthread keys and theTimedWaitstate) to fiber-local storage, and makes contended mutex and rwlock waits interruptible on Windows. The two PRs complement each other: with #1562, libkernel calls from a handler no longer take winpthreads' TLS lock, and this PR's retry also covers athread_localin libc or the other prx modules.In this probe (not part of this PR), a worker loops on libc's
strerror, whose buffer is a MinGWthread_local, and the handler callsstrerrortoo:mainde12706aTested
Windows 10 (19045), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release, Intel Core i7-12700H.
guest_raise_exceptionfailed 2 of 5 runs on its own (ctest timeout), and a loop over the test binary hung on runs 12, 3 and 3. gdb on a hung process showed the main thread still waiting for the 9th delivery to the busy thread and the busy thread back in its loop. A 1 ms sleep takes 15.6 ms there, soExpectDelivery's 5000 attempts outlast the 30 s timeout instead of aborting.guest_raise_exception: a thread raises SEH exceptions in a loop and a vectored handler continues each one, so the thread spends much of its time inNtContinue. With the first change only, it hung in 4 of 4 runs; with the retry it passes.SuspendThreadbeing asynchronous, not from a measured before and after.SuspendThreadis asynchronous.scePthreadSelfin a loop while it is raised at, and the handler checks it sees its own thread. Without the winpthreads retry it hung in every run (3 of 3 with the final 100 rounds, 10 of 10 in an earlier version); with it, it passes.ctest -R ^guest_raise_exception$ --repeat until-fail:10passed all 10 runs (at most 8.7 s each, the 30 s timeout unchanged), and 20 of 20 runs passed with a 200-round version of the new phase.agc_driver_dcc_keys_first_write. That test fails without this change too (AGC: on Windows (NVIDIA) a new host import's pages are marked written after ImportWatched's window, so write-backs drop GPU results #1434) and never raises an exception.NtContinueand checks that the raise throws after 1 s without running the handler. The thread continues a context whoseRipisNtContinueand whoseRcxpoints at that same context, so each call restores itself.guest_raise_exceptionpassed 10 of 10 runs on the branch, and 10 of 10 merged ontomain63c57d2.main63c57d2: 452 of 454 pass. The failures areagc_driver_dcc_keys_first_writeandagc_driver_srgb8_color_target, the NVIDIA import flakes in AGC: on Windows (NVIDIA) a new host import's pages are marked written after ImportWatched's window, so write-backs drop GPU results #1434.No title tested.
Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)🤖 Generated with Claude Code