From 95b225aef718e21f30ad7496711c4ce2af92ec9f Mon Sep 17 00:00:00 2001
From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com>
Date: Thu, 16 Jul 2026 22:35:06 -0700
Subject: [PATCH] fix(saves): close a deadlock race in the worker pause
 handshake

Execute() signaled _stopEvent before clearing _pause and before checking the
exit flag. Wake/Sleep/Exit all run on the owning thread, so the moment
_stopEvent is set the owner can start another pause cycle - Exit() does
exactly that. Two interleavings in that window are fatal:

- The worker's late "_pause = false" clobbers the new cycle's pause request,
  so the worker consumes the wake and spins forever waiting for a pause that
  never reads true while the owner blocks in Sleep().
- The worker's late exit check observes the _exit flag written by an Exit()
  whose Sleep() is still pending, and returns without ever signaling it.

Either way the run hangs silently. The ordering predates this branch, but
production callers never sat inside the window (one Wake/Sleep pair per
save, minutes apart; Exit only long after the last handshake). The tests
added on this branch are the first callers that run pause cycles
back-to-back, and a saturated CI runner hit the window: this is what
stalled the CentOS job for 2h41m. The race is not rare under that pattern -
a churn test reproduces the deadlock in 4 out of 4 runs locally.

Fix: sample the exit condition and clear _pause (volatile) BEFORE signaling
_stopEvent. A post-signal Exit() is then always serviced by one more
wake/drain/signal round, and its pause request can no longer be clobbered.
Drain-before-exit semantics on Core.Closing are unchanged.

Adds a watchdog churn regression test (2000 wake/sleep/exit lifecycles) that
fails instead of hanging if the race is reintroduced: red 4/4 deadlocks
without the fix, green 4/4 at ~550ms with it. Server.Tests 789/789 and
UOContent.Tests 509/509 pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 ...SerializationThreadWorkerHandshakeTests.cs | 59 +++++++++++++++++++
 .../SerializationThreadWorker.cs              | 14 ++++-
 2 files changed, 70 insertions(+), 3 deletions(-)
 create mode 100644 Projects/Server.Tests/Tests/Serialization/SerializationThreadWorkerHandshakeTests.cs

diff --git a/Projects/Server.Tests/Tests/Serialization/SerializationThreadWorkerHandshakeTests.cs b/Projects/Server.Tests/Tests/Serialization/SerializationThreadWorkerHandshakeTests.cs
new file mode 100644
index 000000000..58b7f4982
--- /dev/null
+++ b/Projects/Server.Tests/Tests/Serialization/SerializationThreadWorkerHandshakeTests.cs
@@ -0,0 +1,59 @@
+using System;
+using System.Threading;
+using Xunit;
+
+namespace Server.Tests;
+
+[Collection("Sequential Server Tests")]
+public class SerializationThreadWorkerHandshakeTests
+{
+    // Regression: the pause handshake cleared _pause and checked the exit flag AFTER
+    // signaling _stopEvent. Wake/Sleep/Exit all run on the owning thread, so an Exit()
+    // issued the moment a Sleep() returned could either be clobbered (worker spins
+    // forever) or orphaned (worker returns without servicing Exit's Sleep) — a silent
+    // deadlock. Churn the full lifecycle with the racy back-to-back Sleep/Exit pattern;
+    // the watchdog turns a reintroduced deadlock into a failure instead of a hung run.
+    [Fact]
+    public void WakeSleepExitChurn_NeverDeadlocks()
+    {
+        Exception failure = null;
+        var done = new ManualResetEventSlim();
+
+        var churn = new Thread(() =>
+        {
+            try
+            {
+                for (var i = 0; i < 2000; i++)
+                {
+                    var source = new SerializationChunkSource();
+                    var worker = new SerializationThreadWorker(0, source);
+                    worker.AllocateHeap();
+
+                    worker.Wake();
+                    worker.Sleep();
+                    worker.Exit(); // Immediately after Sleep returns — the racy window.
+                }
+            }
+            catch (Exception e)
+            {
+                failure = e;
+            }
+            finally
+            {
+                done.Set();
+            }
+        })
+        {
+            IsBackground = true,
+            Name = "Handshake Churn"
+        };
+
+        churn.Start();
+
+        Assert.True(
+            done.Wait(TimeSpan.FromMinutes(2)),
+            "Worker pause/exit handshake deadlocked (owner blocked in Sleep or worker spinning)."
+        );
+        Assert.Null(failure);
+    }
+}
diff --git a/Projects/Server/Serialization/SerializationThreadWorker.cs b/Projects/Server/Serialization/SerializationThreadWorker.cs
index 8fe78911d..d936213c4 100644
--- a/Projects/Server/Serialization/SerializationThreadWorker.cs
+++ b/Projects/Server/Serialization/SerializationThreadWorker.cs
@@ -270,10 +270,18 @@ public class SerializationThreadWorker
 
             writer.Close();
 
-            worker._stopEvent.Set(); // Allow the main thread to continue now that we are finished
-            worker._pause = false;
+            // Wake/Sleep/Exit all run on the owning thread, so the moment _stopEvent is set
+            // the owner may start another pause cycle (Exit does exactly that). Clear _pause
+            // and sample the exit condition BEFORE signaling: clearing after the signal can
+            // clobber the next cycle's pause request (this thread then spins forever waiting
+            // for a pause that never reads true), and deciding to exit after the signal can
+            // return without servicing that cycle's Sleep (the owner then blocks forever).
+            var exiting = Core.Closing || worker._exit;
+            Volatile.Write(ref worker._pause, false);
 
-            if (Core.Closing || worker._exit)
+            worker._stopEvent.Set(); // Allow the main thread to continue now that we are finished
+
+            if (exiting)
             {
                 return;
             }