[Bug 60523] New: ntdll: Take the in-flight alert when a RtlWaitOnAddress() wait times out after being dequeued

[WineHQ Bugzilla]

http://bugs.winehq.org/show_bug.cgi?id=60523

            Bug ID: 60523
           Summary: ntdll: Take the in-flight alert when a
                    RtlWaitOnAddress() wait times out after being dequeued
           Product: Wine
           Version: 11.20
          Hardware: x86-64
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: ntdll
          Assignee: [e-mail]
          Reporter: [e-mail]
  Target Milestone: ---
      Distribution: ---

A wake that races the end of a timed-out `RtlWaitOnAddress()` leaves the thread
alerted. The wakers dequeue the waiter under the queue lock and alert it after
releasing t
he lock; a waiter whose timeout expires in between is already dequeued but
returns `STATUS_TIMEOUT`, and the alert arrives with nobody waiting for it and
stays pending. T
he thread's next `NtWaitForAlertByThreadId()` then returns at once, whatever it
waits for: an infinite `SleepConditionVariableCS()` or
`SleepConditionVariableSRW()` on an
 unrelated condition variable, an SRW lock, a critical section.

On Windows the thread has no alert pending after such a race. The new test
shows it: a thread loops `RtlWakeAddressSingle()` or `RtlWakeAddressAll()` on
an address while 
the test makes up to 20,000 `RtlWaitOnAddress()` calls on it with a timeout of
0 or 100 ns, and after each call `NtWaitForAlertByThreadId(NULL, 0)` must not
find an alert
 pending. It passes on Windows 11 (0 pending alerts in each of the four cases)
and fails on current Wine in 2 or 3 of the four cases per run, with 23 to 245
of the 20,000
 waits leaving an alert pending.

The fix is on the waiter's side only. A waiter that finds its entry already
dequeued when its wait returns `STATUS_TIMEOUT` has been claimed by a waker
that is about to a
lert it, so it takes that alert with an untimed `NtWaitForAlertByThreadId()`
and reports the wake. The wakers don't change, nothing new runs under the queue
lock, and the
re are no timing constants: the waker alerts within a few instructions of
releasing the lock, which bounds the second wait. Alternatives I dropped:
alerting under the que
ue lock (a futex wake inside the spinlock on every lock and condition-variable
wake), ignoring alerts that arrive while the entry is still queued (an alert
does end a `Rt
lWaitOnAddress()` wait on Windows), and a bounded grace period for the second
wait (a constant, and Windows needs none).

!3929 fixed the other way a thread could be left alerted after its wait ended,
a waiter alerted twice by `RtlWakeAddressAll()`, for the same symptom (spurious
`SleepConditionVariableCS()` wakeups crashing Resident Evil). This is the
remaining case.

What hits it in practice is zero-timeout waits: they are queued like any other
wait and time out at once, so spinlock back-offs that poll with
`WaitOnAddress(..., 0)` (gperftools' `SpinLockDelay`, used by tcmalloc) go
through the race constantly. Warhammer 40,000: Space Marine 2 crashes
mid-mission because of it
(https://github.com/ValveSoftware/Proton/issues/8072): its tcmalloc back-off
collects a stale alert, an unlooped `SleepConditionVariableCS(INFINITE)`
waiting for a batch of file reads then returns early, and the completion
callback writes into the dead stack frame. A stress program with 8 threads
doing that sequence returns early in 13% of about 530,000 waits in 15 s on
current Wine, in 0 of about 100,000 on Windows 11, and in 0 of about 530,000
with this change; the game no longer crashes with it.

The test comes after the fix: the race is probabilistic, so a `todo_wine` would
sometimes succeed on the unfixed code.

This bug report replaces https://bugs.winehq.org/show_bug.cgi?id=60397 and is
narrowly scoped to this issue.

One way to fix this is here, written by an LLM:
https://github.com/jegglest/wine/commit/waitonaddress-stale-alert

This test code demonstrates the difference between Windows and wine behavior:
https://github.com/jegglest/wine/commit/waitonaddress-stale-alert-test

-- 
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.