[Bug 60397] New: RtlWaitOnAddress: a wake racing a timeout leaves a stale thread alert that ends the thread's next wait early (Warhammer 40,000: Space Marine 2 crashes mid-mission)
http://bugs.winehq.org/show_bug.cgi?id=60397 Bug ID: 60397 Summary: RtlWaitOnAddress: a wake racing a timeout leaves a stale thread alert that ends the thread's next wait early (Warhammer 40,000: Space Marine 2 crashes mid-mission) Product: Wine Version: 11.0 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: ntdll Assignee: wine-bugs@list.winehq.org Reporter: eggnet@gmail.com Target Milestone: --- Distribution: --- RtlWakeAddressSingle() and RtlWakeAddressAll() take a waiter's entry off the queue under the queue spinlock, but only alert the waiter (NtAlertThreadByThreadId) after releasing the lock. When the waiter's wait ends in that window, three things go wrong in RtlWaitOnAddress() (dlls/ntdll/sync.c): 1. Use after return. The waiter checks entry.addr without the lock. The waker clears entry->addr and then calls list_remove() on the entry, which lives on the waiter's stack. A waiter that sees the address cleared skips the locked section, returns, and reuses that stack while the waker is still reading entry->next/prev and writing through them. This needs the waker to be preempted in a window of a few instructions, so it is rare, but it is memory corruption. 2. Lost wake. The waiter returns STATUS_TIMEOUT although a wake was aimed at it. For RtlWakeAddressSingle() no other waiter receives that wake. 3. Stale alert. The waker's NtAlertThreadByThreadId() arrives with nobody waiting for it and stays pending. The thread's next NtWaitForAlertByThreadId() then returns immediately, whatever it is waiting for: typically a SleepConditionVariableCS()/SleepConditionVariableSRW() with an infinite timeout on an unrelated condition variable, which wakes up spuriously. Zero-timeout waits make this common: they are queued like any other wait and time out immediately, so a concurrent wake easily picks one. Spinlock back-offs that convert a short random delay to whole milliseconds (gperftools' SpinLockDelay on Windows, used by tcmalloc) pass 0 most of the time. Documented behaviour - WaitOnAddress may return early, and the documented causes include "A previous wake on the same address was abandoned" (https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-wai...). Wine's per-thread alert instead ends the thread's next wait on any address or condition variable. - WakeByAddressSingle "wakes the first thread to wait", and "other waiting threads continue to wait" (https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-wak...). A wake handed to a thread that is already timing out is lost for the thread actually waiting. - For a zero timeout, the usual Win32 rule applies, e.g. SleepConditionVariableCS: "If dwMilliseconds is zero, the function tests the states of the specified objects and returns immediately" (https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-sle...). Real-world impact Warhammer 40,000: Space Marine 2 (Steam 2183900) crashes mid-mission under Proton, about once per hour of play (https://github.com/ValveSoftware/Proton/issues/8072). Its libtcmalloc_minimal.dll back-off calls WaitOnAddress() with a timeout that rounds down to 0 ms; an instrumented ntdll counted over 2,000 zero-timeout waiters dequeued as they timed out in 44 minutes of play. The game then does an unlooped SleepConditionVariableCS(INFINITE) whose early return leaves a file-read completion callback pointing into a dead stack frame (access violation in resFILE_LOADER at Retail.exe+0x2158FEE). The game is at fault for not looping, but the spurious wakeup is a Wine artifact; the crash has not been reported on Windows. With a prototype fix on my machine: about 4 hours of play without a crash, against 8 crashes in about 7 h 20 min before, same kernel and game build. Details and measurements are in the two comments linked below. Reproducers - Stress program: 8 worker threads. Each takes a tcmalloc-style spinlock (a zero-timeout WaitOnAddress() poll in a CAS loop, released with WakeByAddressSingle(); 4 more threads hammer the same lock) and then does a single, unlooped SleepConditionVariableCS(INFINITE) on its own condition variable, completed by a dedicated thread. It counts returns while the predicate is still false. In 15 s on Proton Experimental 11.0-20260924: 529,155 waits, 69,195 returned early (about 13%). With the prototype fix: 0, same throughput. - Conformance-style test: a thread loops on RtlWakeAddressSingle()/RtlWakeAddressAll() while the main thread does up to 20,000 RtlWaitOnAddress() calls on that address with a timeout of 0 or -1 (100 ns), and after each one a 100 ns wait on a different address, which must time out. On stock Wine, 1 to 26 of the 20,000 waits on the other address returned early, in 11 of 12 cases over three runs. With the prototype fix, 0. Fix approach that worked a) Check under the queue lock whether the entry was dequeued; a waker that has released the lock is done with the entry. (Fixes 1.) b) Don't queue zero-timeout waits: compare the value and return STATUS_SUCCESS or STATUS_TIMEOUT. c) A waiter that finds itself dequeued after its wait timed out reports the wake, and takes the in-flight alert with a short bounded NtWaitForAlertByThreadId() (1 ms was enough in every measured case) rather than waiting for it indefinitely. (Fixes 2, and 3 in practice.) d) An alert received while the waiter is still queued cannot be a wake for the current wait, since wakers dequeue first; ignore it and keep waiting until the original deadline (convert a relative timeout to an absolute one once). Safety net for whatever c) misses and for alerts left over from thread id reuse. Note that this also means NtAlertThreadByThreadId() no longer ends a RtlWaitOnAddress() wait, which it does on Windows; that is an undocumented interaction and no documented interface offers it. With a)-d), ntdll:sync and kernel32:sync are otherwise unchanged. Alerting under the queue lock would also close the race but would put a futex wake inside the spinlock on the path of every SRW lock, critical section and condition variable wake. Note on provenance: the investigation was done with the help of an LLM tool, and the prototype fix, stress program and test were generated with it. Per the Clean Room Guidelines I am not submitting any of that code; this report describes the problem and the approach so that the fix can be written by a Wine developer. All measurements above were taken on my machine (Bazzite 44, kernel 7.2.7, Ryzen 7 9800X3D, RX 9070 XT). References - Analysis, crash dumps and in-game data: https://github.com/ValveSoftware/Proton/issues/8072#issuecomment-5852529635 - Revised prototype fix and test results: https://github.com/ValveSoftware/Proton/issues/8072#issuecomment-5858321198 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=60397 Zeb Figura <z.figura12@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12@gmail.com --- Comment #1 from Zeb Figura <z.figura12@gmail.com> --- This is fundamentally part of the design of win32 futexes and cannot be avoided. The LLM has, as usual, misdiagnosed. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=60397 --- Comment #2 from Jason Eggleston <eggnet@gmail.com> --- To check whether the pending alert is inherent, I ran the same two programs on Windows and on Wine. Windows: Windows 11 Enterprise Evaluation 25H2, build 26200.9457, in a QEMU/KVM VM with 4 logical processors on a Ryzen 7 9800X3D. Wine: Proton Experimental 11.0-20260924 (its ntdll futex code is identical to current master), same binaries, same host. 1. Raced wakes. A second thread loops RtlWakeAddressSingle() (or RtlWakeAddressAll()) on one address while the main thread makes 20,000 RtlWaitOnAddress() calls on it with a timeout of 0. After each call, NtWaitForAlertByThreadId(NULL, 0) reports whether an alert is pending on the thread (STATUS_ALERTED) and takes it. Windows: 39 (single) and 47 (all) of the 20,000 waits timed out while racing the waker; 0 pending alerts. Wine: 147 and 126 timed out; 18 and 20 pending alerts. Variant without the check: after each raced wait, a 100 ns RtlWaitOnAddress() on an unrelated address, which must time out. Wine: 22 (single) and 27 (all) of 20,000 returned early. Windows: 0, in the 189 iterations that fit the 3 s budget (a 100 ns wait costs a timer tick there). 2. The stress program from the report: 8 workers each take a tcmalloc-style spinlock polled with WaitOnAddress(..., 0) and released with WakeByAddressSingle(), then do one unlooped SleepConditionVariableCS(INFINITE) that another thread completes; 15 s. Windows: 100,097 condition-variable waits, 0 returned while the predicate was still false. Wine: 529,155 waits, 69,195 returned early (13%). So on Windows a wake that races the end of a timed-out RtlWaitOnAddress() does not leave the thread's alert pending, and the unlooped wait never returned early in 100,000 tries, against 13% on Wine. That matches how Windows does it. Its wait block carries a flag that the two sides exchange: a waiter whose wait timed out and who finds the flag already claimed by a waker does a second, untimed NtWaitForAlertByThreadId() to take the wake that is on its way; a waker that finds the waiter already timed out does not alert it. (YY-Thunks' WaitOnAddress, written after the Windows 8 ntdll code and using ntdll's private names RtlpWaitOnAddressWithTimeout and RtlpWaitOnAddressRemoveWaitBlock, shows the same protocol.) Wine's waiter instead returns STATUS_TIMEOUT and leaves the alert for its next wait. The equivalent in Wine's structure: a waiter that finds entry.addr cleared after a timeout knows a waker has committed to alerting it, and can consume that alert with an untimed NtWaitForAlertByThreadId() before returning; no change to the wakers is needed for this. The documentation allows spurious wakeups and the game must loop; I have reported that to its developer. But Wine returns early where Windows does not, and this game, and any program with the same mistake, depends on that. Point 1 of the report is a different bug and does not depend on any of the above. In RtlWakeAddressSingle() and RtlWakeAddressAll(), after finding a matching entry under the queue lock, the code is: entry->addr = NULL; list_remove( &entry->entry ); and RtlWakeAddressAll() then also reads entry->tid. list_remove() reads entry->entry.next and .prev and writes through them. All of that reads the entry after the store to entry->addr, and the entry is a local variable on the waiter's stack. Since e00cbef06d ("ntdll: Pre-check entry->addr before taking a spin lock in RtlWaitOnAddress()") the waiter checks entry.addr without the lock when its wait returns: if (entry.addr) { spin_lock( &queue->lock ); ... A waiter whose wait timed out and who sees the NULL skips the lock, returns, and its caller reuses that stack. If the waker was preempted between its store and its reads, it then reads next/prev (and, in RtlWakeAddressAll(), the tid) from whatever now occupies that memory and writes through the two pointers. The window is a few instructions unless the waker is preempted in it, so it is rare, but it is memory corruption in a path every SRW lock, critical section and condition variable wake goes through. Before e00cbef06d the waiter always took the lock, and a waker holding it could not race the waiter's return. It is fixed either by taking the lock again unconditionally, or by making the store to entry->addr the waker's last access to the entry: read tid, unlink, then clear addr with release semantics, and read it with acquire semantics in the waiter. In the compiled Proton build the compiler has also moved RtlWakeAddressSingle()'s tid load after the store, so source order alone does not protect it. The ntdll:sync suite with a test added for this (after each raced RtlWaitOnAddress() call, NtWaitForAlertByThreadId(NULL, 0) must not find an alert pending) passes on Windows 11 (441 tests, 0 failures) and fails on Wine in 2 or 3 of its 4 cases per run, with 23 to 245 of 20,000 waits leaving an alert pending. A prototype that takes the in-flight alert as Windows does passes it, with ntdll:sync and kernel32:sync otherwise unchanged. The measurements are mine; the programs and the prototype were written with an LLM tool, as noted before, so I am not submitting them. Their source, the raw outputs and the prototype patches are in the Proton issue: https://github.com/ValveSoftware/Proton/issues/8072#issuecomment-5903294489 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=60397 --- Comment #3 from Jason Eggleston <eggnet@gmail.com> --- I am splitting this bug report into the individual issues: https://bugs.winehq.org/show_bug.cgi?id=60417 https://bugs.winehq.org/show_bug.cgi?id=60523 https://bugs.winehq.org/show_bug.cgi?id=60524 Also, a fix for the first bug in this report has already been merged: https://gitlab.winehq.org/wine/wine/-/merge_requests/12315 This bug report should be closed as duplicate. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
WineHQ Bugzilla