mirror of
https://github.com/Droid-Deck/DroidDeck-Proton.git
synced 2026-11-07 10:46:12 -10:00
patches: ntdll: update the RtlWaitOnAddress() stale alert fix, add its test
98efe1cd took the first version of Proton-JE's fix for the alert that a
wake leaves pending when it races a timed-out RtlWaitOnAddress() (the
Space Marine 2 crash of Proton #8072). This replaces it with the version
Proton-JE has carried since 2026-10-28, written for upstream Wine (bug
60523). The behaviour is the same: a waiter that finds itself dequeued
after its wait timed out takes the in-flight alert and reports the wake.
The code is shorter: it returns the status of the untimed wait,
STATUS_ALERTED, which RtlWaitOnAddress() already turns into
STATUS_SUCCESS. The comment in RtlWakeAddressAll() that accepted the
spurious wake as the price of alerting outside the spinlock now says
that the waiter takes the alert.
The second patch is the conformance test, test_wait_on_address_raced_wake()
in ntdll:sync: up to 20000 RtlWaitOnAddress() calls with a timeout of 0
or 100 ns race a thread that wakes the address, and no alert may be
left pending after any of them. It fails without the fix, and passes
with it and on Windows 11.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
(cherry picked from commit 41d5803780930421423a2f7931f9ba923b5a49e4)
This commit is contained in:
1 parent
cd8a0be1c2
commit
960acd0a29
3 files changed
+155
-35
No files matched your search
@@ -594,8 +594,10 @@ apply_all_in_dir() {
|
||||
apply_patch "../patches/wine-hotfixes/pending/ntdll-srwlock-skip-needless-shared-release-wake.patch"
|
||||
|
||||
# https://github.com/ValveSoftware/Proton/issues/8072
|
||||
# https://bugs.winehq.org/show_bug.cgi?id=60523
|
||||
echo "WINE: -GAME FIXES- ntdll: don't leave a stale alert after a raced RtlWaitOnAddress() timeout (Space Marine 2 mid-mission crash)"
|
||||
apply_patch "../patches/wine-hotfixes/pending/ntdll-take-in-flight-alert-when-wait-on-address-times-out.patch"
|
||||
apply_patch "../patches/wine-hotfixes/pending/ntdll-tests-wait-on-address-wake-racing-timeout.patch"
|
||||
|
||||
# https://github.com/GloriousEggroll/proton-ge-custom/issues/870
|
||||
echo "WINE: -GAME FIXES- ntdll: run NCSOFT PURPLE's VMProtect-packed purple-agent.exe in new WoW64 mode (Initialization Error 1)"
|
||||
|
||||
+49
-35
@@ -1,39 +1,47 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Jason Eggleston <[email protected]>
|
||||
Date: Tue, 29 Sep 2026 19:36:30 -0700
|
||||
Subject: [PATCH] ntdll: Take the in-flight alert when a RtlWaitOnAddress()
|
||||
wait times out after being dequeued.
|
||||
Subject: [PATCH] ntdll: Take the alert of a RtlWaitOnAddress() wake that races
|
||||
a timeout.
|
||||
|
||||
RtlWakeAddressSingle() and RtlWakeAddressAll() dequeue a waiter under the
|
||||
queue lock and alert it after releasing the lock. A waiter whose wait
|
||||
times out in between finds itself dequeued but returns STATUS_TIMEOUT.
|
||||
Its alert then arrives with nobody waiting for it and stays pending, and
|
||||
the thread's next NtWaitForAlertByThreadId() returns at once, whatever it
|
||||
waits for: typically a SleepConditionVariableCS() or
|
||||
SleepConditionVariableSRW() with an infinite timeout on an unrelated
|
||||
condition variable, which wakes up spuriously.
|
||||
RtlWakeAddressSingle() and RtlWakeAddressAll() dequeue a waiter under
|
||||
the queue lock and alert it after releasing the lock. A waiter whose
|
||||
wait times out in between finds itself dequeued but returns
|
||||
STATUS_TIMEOUT. Its alert then arrives with nobody waiting for it and
|
||||
stays pending on the thread, and the thread's next
|
||||
NtWaitForAlertByThreadId() returns at once whatever it waits for:
|
||||
typically a SleepConditionVariableCS() or SleepConditionVariableSRW()
|
||||
with an infinite timeout on an unrelated condition variable, which
|
||||
wakes up spuriously. The comment in RtlWakeAddressAll() accepts that
|
||||
as the price of not alerting under the spinlock.
|
||||
|
||||
Windows doesn't leave that alert behind: a waiter that finds itself
|
||||
claimed by a waker after its timeout waits again for the wake before
|
||||
returning. Do the same. A dequeued waiter has been woken, so take the
|
||||
alert and report the wake.
|
||||
Windows leaves no alert pending after such a race: a waiter that a
|
||||
waker has claimed takes the wake before returning. Do the same: a
|
||||
waiter that finds itself dequeued when its wait timed out has been
|
||||
woken, so take the in-flight alert with an untimed
|
||||
NtWaitForAlertByThreadId() and report the wake. The wakers still alert
|
||||
outside the lock, and the second wait lasts only until the waker's
|
||||
alert, which follows its unlock. Update the comment.
|
||||
|
||||
Warhammer 40,000: Space Marine 2 crashes under Wine because of this. Its tcmalloc spinlock back-off polls with zero-timeout
|
||||
WaitOnAddress() calls, whose timeouts race the releasing
|
||||
WakeByAddressSingle(), and the game's unlooped
|
||||
SleepConditionVariableCS(INFINITE) then returns early. On Windows 11 the
|
||||
same stress program shows no early return in 100,000 waits, against 13%
|
||||
under Wine.
|
||||
Zero-timeout waits make the race common: they are queued like any
|
||||
other wait and time out immediately, and spinlock back-offs that poll
|
||||
with WaitOnAddress(..., 0), such as tcmalloc's, collect the stale
|
||||
alert. Warhammer 40,000: Space Marine 2 crashes because of this when
|
||||
an unlooped SleepConditionVariableCS(INFINITE) returns while file
|
||||
reads are still pending.
|
||||
|
||||
Backported from https://github.com/jegglest/wine/commit/dee180fa83 and refreshed for
|
||||
the ReadPointerAcquire() cast in Valve's b2a60dcd145.
|
||||
|
||||
https://github.com/ValveSoftware/Proton/issues/8072
|
||||
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=60523
|
||||
Co-Authored-By: Claude Fable 5.1 <[email protected]>
|
||||
Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
||||
---
|
||||
dlls/ntdll/sync.c | 11 ++++++++---
|
||||
1 file changed, 8 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c
|
||||
index 729d7cbd847..cc05308c715 100644
|
||||
--- a/dlls/ntdll/sync.c
|
||||
+++ b/dlls/ntdll/sync.c
|
||||
@@ -883,6 +883,7 @@
|
||||
@@ -883,6 +883,7 @@ NTSTATUS WINAPI RtlWaitOnAddress( const void *addr, const void *cmp, SIZE_T size
|
||||
{
|
||||
struct futex_queue *queue = get_futex_queue( addr );
|
||||
struct futex_entry entry;
|
||||
@@ -41,25 +49,31 @@ diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c
|
||||
NTSTATUS ret;
|
||||
|
||||
TRACE("addr %p cmp %p size %#Ix timeout %s\n", addr, cmp, size, debugstr_timeout( timeout ));
|
||||
@@ -915,11 +916,20 @@
|
||||
@@ -915,11 +916,15 @@ NTSTATUS WINAPI RtlWaitOnAddress( const void *addr, const void *cmp, SIZE_T size
|
||||
if (ReadPointerAcquire( (void * volatile *)&entry.addr ))
|
||||
{
|
||||
spin_lock( &queue->lock );
|
||||
- if (entry.addr)
|
||||
+ if ((queued = entry.addr != NULL))
|
||||
+ if ((queued = !!entry.addr))
|
||||
list_remove( &entry.entry );
|
||||
spin_unlock( &queue->lock );
|
||||
}
|
||||
|
||||
+ if (!queued && ret == STATUS_TIMEOUT)
|
||||
+ {
|
||||
+ /* A waker removed us as the wait timed out: we have been woken, and the waker alerts us right after
|
||||
+ * releasing the queue lock. Take that alert, as Windows does, or it would end this thread's next wait
|
||||
+ * early, whatever it waits for. */
|
||||
+ NtWaitForAlertByThreadId( NULL, NULL );
|
||||
+ ret = STATUS_SUCCESS;
|
||||
+ }
|
||||
+ /* A waker took us off the queue and alerts us after releasing the lock. If our wait timed out first, take
|
||||
+ * that alert, or it would end this thread's next wait early. */
|
||||
+ if (!queued && ret == STATUS_TIMEOUT) ret = NtWaitForAlertByThreadId( NULL, NULL );
|
||||
+
|
||||
TRACE("returning %#lx\n", ret);
|
||||
|
||||
if (ret == STATUS_ALERTED) ret = STATUS_SUCCESS;
|
||||
@@ -962,8 +967,8 @@ void WINAPI RtlWakeAddressAll( const void *addr )
|
||||
}
|
||||
}
|
||||
|
||||
- /* Try not to make a system call while holding a spinlock (even if that can be responsible for spurious wake
|
||||
- * up scenario). */
|
||||
+ /* Try not to make a system call while holding a spinlock. A waiter whose wait times out before the alert
|
||||
+ * arrives takes it in RtlWaitOnAddress(). */
|
||||
spin_unlock( &queue->lock );
|
||||
if (count)
|
||||
NtAlertMultipleThreadByThreadId( tids, count, NULL, NULL );
|
||||
@@ -0,0 +1,104 @@
|
||||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Jason Eggleston <[email protected]>
|
||||
Date: Tue, 29 Sep 2026 19:36:30 -0700
|
||||
Subject: [PATCH] ntdll/tests: Test a RtlWaitOnAddress() wake racing a
|
||||
timed-out wait.
|
||||
|
||||
A wake that races the end of a short wait may be taken by that wait,
|
||||
but it must not leave an alert pending on the thread, which would end
|
||||
the thread's next wait on anything early.
|
||||
|
||||
Co-Authored-By: Claude Fable 5.1 <[email protected]>
|
||||
Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
||||
---
|
||||
dlls/ntdll/tests/sync.c | 70 +++++++++++++++++++++++++++++++++++++++++
|
||||
1 file changed, 70 insertions(+)
|
||||
|
||||
diff --git a/dlls/ntdll/tests/sync.c b/dlls/ntdll/tests/sync.c
|
||||
index dcbb8b18c3f..5b8f5dd987a 100644
|
||||
--- a/dlls/ntdll/tests/sync.c
|
||||
+++ b/dlls/ntdll/tests/sync.c
|
||||
@@ -615,6 +615,75 @@ static void test_wait_on_address(void)
|
||||
ok(address == 0, "got %s\n", wine_dbgstr_longlong(address));
|
||||
}
|
||||
|
||||
+struct wait_on_address_waker
|
||||
+{
|
||||
+ LONG address;
|
||||
+ volatile LONG stop;
|
||||
+ BOOL wake_all;
|
||||
+};
|
||||
+
|
||||
+static DWORD WINAPI wait_on_address_waker_thread( void *arg )
|
||||
+{
|
||||
+ struct wait_on_address_waker *waker = arg;
|
||||
+
|
||||
+ while (!waker->stop)
|
||||
+ {
|
||||
+ if (waker->wake_all) pRtlWakeAddressAll( &waker->address );
|
||||
+ else pRtlWakeAddressSingle( &waker->address );
|
||||
+ }
|
||||
+ return 0;
|
||||
+}
|
||||
+
|
||||
+static void test_wait_on_address_raced_wake(void)
|
||||
+{
|
||||
+ static const LONGLONG timeouts[] = {0, -1};
|
||||
+ static const LARGE_INTEGER zero;
|
||||
+ struct wait_on_address_waker waker;
|
||||
+ unsigned int i, count, pending, failed;
|
||||
+ LARGE_INTEGER timeout;
|
||||
+ LONG compare = 0;
|
||||
+ NTSTATUS status;
|
||||
+ HANDLE thread;
|
||||
+ DWORD start;
|
||||
+
|
||||
+ if (!pRtlWaitOnAddress || !pNtWaitForAlertByThreadId)
|
||||
+ {
|
||||
+ win_skip("RtlWaitOnAddress or NtWaitForAlertByThreadId not supported, skipping test\n");
|
||||
+ return;
|
||||
+ }
|
||||
+
|
||||
+ /* A wake that races the end of a short wait may be taken by that wait, but it must not leave an alert
|
||||
+ * pending on the thread, which would end its next wait on anything early. */
|
||||
+ for (i = 0; i < ARRAY_SIZE(timeouts) * 2; i++)
|
||||
+ {
|
||||
+ winetest_push_context( "timeout %s, %s", wine_dbgstr_longlong( timeouts[i / 2] ),
|
||||
+ i % 2 ? "wake all" : "wake single" );
|
||||
+
|
||||
+ waker.address = 0;
|
||||
+ waker.stop = 0;
|
||||
+ waker.wake_all = i % 2;
|
||||
+ thread = CreateThread( NULL, 0, wait_on_address_waker_thread, &waker, 0, NULL );
|
||||
+
|
||||
+ pending = failed = 0;
|
||||
+ timeout.QuadPart = timeouts[i / 2];
|
||||
+ start = GetTickCount();
|
||||
+ for (count = 0; count < 20000 && GetTickCount() - start < 1000; count++)
|
||||
+ {
|
||||
+ status = pRtlWaitOnAddress( &waker.address, &compare, sizeof(compare), &timeout );
|
||||
+ if (status != STATUS_SUCCESS && status != STATUS_TIMEOUT) failed++;
|
||||
+ if (pNtWaitForAlertByThreadId( NULL, &zero ) == STATUS_ALERTED) pending++;
|
||||
+ }
|
||||
+
|
||||
+ waker.stop = 1;
|
||||
+ WaitForSingleObject( thread, INFINITE );
|
||||
+ CloseHandle( thread );
|
||||
+
|
||||
+ ok( !failed, "%u of %u waits failed\n", failed, count );
|
||||
+ ok( !pending, "%u of %u waits left an alert pending\n", pending, count );
|
||||
+ winetest_pop_context();
|
||||
+ }
|
||||
+}
|
||||
+
|
||||
static HANDLE thread_ready, thread_done;
|
||||
|
||||
static DWORD WINAPI resource_shared_thread(void *arg)
|
||||
@@ -1474,6 +1543,7 @@ START_TEST(sync)
|
||||
pRtlBarrier = (void *)GetProcAddress(module, "RtlBarrier");
|
||||
|
||||
test_wait_on_address();
|
||||
+ test_wait_on_address_raced_wake();
|
||||
test_event();
|
||||
test_mutant();
|
||||
test_semaphore();
|
||||
Reference in new issue
Block a user