ThreadingWin: force Thread::suspend to be synchronous

Source/WTF/wtf/win/ThreadingWin.cpp

@@ -217,9 +217,23 @@ auto Thread::suspend(const ThreadSuspendLocker&) -> Expected<void, PlatformSuspe
     // the heap lock, and currentSingleton would lazy-allocate a Thread for it.
     RELEASE_ASSERT_WITH_MESSAGE(this != Thread::currentMayBeNull(), "We do not support suspending the current thread itself.");
     DWORD result = SuspendThread(m_handle);
-    if (result != (DWORD)-1)
-        return { };
-    return makeUnexpected(result);
+    if (result == (DWORD)-1)
+        return makeUnexpected(result);
+    // SuspendThread only requests a suspension; on multi-core Windows the target may
+    // continue to execute in user mode briefly after SuspendThread returns. Callers that
+    // read registers (MachineStackMarker) happen to force a sync via GetThreadContext,
+    // but callers that only need "target is stopped" -- notably the libpas scavenger's
+    // pasSuspenderBeginSuspend -- would otherwise decommit TLC pages while the mutator
+    // is still writing to them. Force the suspension to complete synchronously here so
+    // every Thread::suspend caller can rely on the invariant.
+    CONTEXT ctx;
+    ctx.ContextFlags = CONTEXT_INTEGER;
+    if (!GetThreadContext(m_handle, &ctx)) {
+        DWORD error = GetLastError();
+        ResumeThread(m_handle);
+        return makeUnexpected(error);
+    }
+    return { };
 }
 
 // During resume, suspend or resume should not be executed from the other threads.

Tools/TestWebKitAPI/Tests/WTF/Threading.cpp

@@ -25,6 +25,7 @@
 
 #include "config.h"
 
+#include <atomic>
 #include <wtf/Threading.h>
 #include <wtf/Vector.h>
 #include <wtf/threads/BinarySemaphore.h>
@@ -121,4 +122,53 @@ TEST(WTF_Thread, ThreadSafetyAnalysisAssertIsCurrentWorks)
         EXPECT_EQ(2u, holder.result);
 }
 
+// Regression test for the libpas scavenger's thread-suspension contract:
+// Thread::suspend must guarantee the target is actually stopped by the time it
+// returns. On Windows this requires an explicit GetThreadContext sync after
+// SuspendThread (which is otherwise asynchronous on multi-core systems). A
+// non-synchronous Thread::suspend would let the libpas scavenger decommit TLC
+// pages while the mutator is still writing to them. The test observes a
+// volatile counter that a busy-looping target thread increments; while
+// suspended, the counter must not advance.
+TEST(WTF_Thread, SuspendIsSynchronous)
+{
+    std::atomic<uint64_t> counter { 0 };
+    std::atomic<bool> stop { false };
+    BinarySemaphore workerStarted;
+
+    RefPtr<Thread> worker = Thread::create("Thread::suspend sync test"_s, [&] {
+        workerStarted.signal();
+        while (!stop.load(std::memory_order_relaxed))
+            counter.fetch_add(1, std::memory_order_relaxed);
+    });
+    ASSERT_TRUE(worker);
+    workerStarted.wait();
+
+    constexpr int iterations = 50;
+    constexpr int observeSpin = 200'000;
+    int offenses = 0;
+
+    for (int i = 0; i < iterations; ++i) {
+        ThreadSuspendLocker locker;
+        auto suspendResult = worker->suspend(locker);
+        ASSERT_TRUE(suspendResult.has_value());
+
+        uint64_t before = counter.load(std::memory_order_relaxed);
+        // Busy-loop to give a non-synchronous suspend time to leak.
+        for (volatile int spin = 0; spin < observeSpin; ++spin) { }
+        uint64_t after = counter.load(std::memory_order_relaxed);
+
+        worker->resume(locker);
+
+        if (before != after)
+            ++offenses;
+    }
+
+    stop.store(true, std::memory_order_relaxed);
+    worker->waitForCompletion();
+
+    EXPECT_EQ(0, offenses) << "Target thread advanced during " << offenses
+        << " of " << iterations << " suspend windows -- Thread::suspend is not synchronous";
+}
+
 } // namespace TestWebKitAPI