From 2d9d83b2fdd2ca456195396475ea7fccb3869bf3 Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Sat, 8 Aug 2026 21:54:03 -0700 Subject: [PATCH] fix(login): finish pending password writes on a normal shutdown Exit() was never wired, so it was dead code either way. Wiring it correctly depends on which teardown path is running. On a normal shutdown EventSink.Shutdown fires on the game thread after the loop has stopped, which is the last chance pending work gets: results the worker already posted are sitting on a loop context nothing will pump again. So the thread is stopped, the context drained once, and queued writes are computed and applied in place. Verifies are dropped instead -- they only decide a login, and every connection is closing. On a crash there is no usable game thread, so nothing may be applied and the thread is simply stopped. Subscribed separately because HandleClosed skips InvokeShutdown when _crashed is set, which would otherwise leave the crash path unhandled entirely. Wired from AccountHandler.Initialize rather than a Configure on the worker: AssemblyHandler.AddMethods binds Static | Public, so a Configure on an internal type is never discovered. --- .../UOContent/Accounting/AccountHandler.cs | 5 ++ .../Accounting/Security/PasswordWorker.cs | 54 +++++++++++++++---- .../AdvancedSearchUtilities.cs | 1 - .../UOContent/Engines/Pathing/MovementPath.cs | 2 - Projects/UOContent/Gumps/AdminGump.cs | 3 +- Projects/UOContent/Utilities/Types.cs | 1 - 6 files changed, 52 insertions(+), 14 deletions(-) diff --git a/Projects/UOContent/Accounting/AccountHandler.cs b/Projects/UOContent/Accounting/AccountHandler.cs index 6c2f9f53a..c4ba03361 100644 --- a/Projects/UOContent/Accounting/AccountHandler.cs +++ b/Projects/UOContent/Accounting/AccountHandler.cs @@ -70,6 +70,11 @@ public static class AccountHandler public static void Initialize() { EventSink.AccountLogin += EventSink_AccountLogin; + + // Wired here because AssemblyHandler only discovers *public* static Configure/Initialize, + // and PasswordWorker is internal. + EventSink.Shutdown += PasswordWorker.Shutdown; + EventSink.ServerCrashed += PasswordWorker.OnCrashed; } [Usage("Password ")] diff --git a/Projects/UOContent/Accounting/Security/PasswordWorker.cs b/Projects/UOContent/Accounting/Security/PasswordWorker.cs index e2f251659..112ff3524 100644 --- a/Projects/UOContent/Accounting/Security/PasswordWorker.cs +++ b/Projects/UOContent/Accounting/Security/PasswordWorker.cs @@ -17,7 +17,6 @@ using System; using System.Collections.Concurrent; using System.Threading; using Server.Logging; -using Server.Misc; using Server.Network; namespace Server.Accounting.Security; @@ -90,7 +89,7 @@ internal sealed class PasswordWorker /// verify and the engine caps connections at 4096 (NetState.Network.cs), so this matches /// that bound and can only trip if the one-per-connection invariant breaks. A cap low enough to /// blunt an attack would reject real players first -- during a mass reconnect they are the - /// queue. Flood defence belongs at the connection layer. + /// queue. Flood defense belongs at the connection layer. /// internal const int MaxPending = 4096; @@ -161,8 +160,7 @@ internal sealed class PasswordWorker /// the freeze holds the loop, so nothing new can be queued during it. PendingSave counts too -- /// the serialization threads are already awake and spinning on an empty queue by then. /// - private static bool CanRunNow() => - World.WorldState is WorldState.Running or WorldState.WritingSave; + private static bool CanRunNow() => World.WorldState is WorldState.Running or WorldState.WritingSave; private void Execute() { @@ -283,7 +281,14 @@ internal sealed class PasswordWorker internal static PasswordOutcome ComputeInline(PasswordJob job) => Instance.Compute(job); - internal static void Exit() + /// + /// Normal shutdown. The loop has stopped but this runs on the game thread, so pending work can + /// be finished in place -- which is the only chance it gets, since nothing will pump the loop + /// context again. + /// + /// Only writes are finished. A verify decides a login, and every connection is closing. + /// + internal static void Shutdown() { var instance = _instance; @@ -292,9 +297,40 @@ internal sealed class PasswordWorker return; } - instance._exit = true; - instance._work.Set(); - instance._thread.Join(TimeSpan.FromSeconds(5)); - _instance = null; + instance.StopThread(); + + // Results posted before the thread stopped are still queued on a loop that has exited. + Core.LoopContext.ExecuteTasks(); + + while (instance._queue.TryDequeue(out var job)) + { + Interlocked.Decrement(ref instance._pending); + + if (job.HashPhrase == null) + { + continue; + } + + var outcome = instance.Compute(job); + + if (outcome.Verified && outcome.Hash != null) + { + job.Account.ApplyPasswordWrite(job.Sequence, outcome.Hash, job.TargetAlgorithm); + } + } + } + + /// + /// Crash. There is no usable game thread, so nothing may be applied -- stop the thread and let + /// whatever was pending go. Subscribed separately because HandleClosed skips + /// InvokeShutdown when the server crashed. + /// + internal static void OnCrashed(ServerCrashedEventArgs e) => _instance?.StopThread(); + + private void StopThread() + { + _exit = true; + _work.Set(); + _thread.Join(TimeSpan.FromSeconds(5)); } } diff --git a/Projects/UOContent/Engines/Advanced Search/AdvancedSearchUtilities.cs b/Projects/UOContent/Engines/Advanced Search/AdvancedSearchUtilities.cs index 2c349847d..6775dcaef 100644 --- a/Projects/UOContent/Engines/Advanced Search/AdvancedSearchUtilities.cs +++ b/Projects/UOContent/Engines/Advanced Search/AdvancedSearchUtilities.cs @@ -1,6 +1,5 @@ using System; using System.Buffers; -using System.Collections.Generic; using System.Globalization; using System.Numerics; using System.Runtime.CompilerServices; diff --git a/Projects/UOContent/Engines/Pathing/MovementPath.cs b/Projects/UOContent/Engines/Pathing/MovementPath.cs index 32aa8ba11..d1ef874fc 100644 --- a/Projects/UOContent/Engines/Pathing/MovementPath.cs +++ b/Projects/UOContent/Engines/Pathing/MovementPath.cs @@ -1,7 +1,5 @@ using System; using System.Diagnostics; -using Server.Engines.Pathing; -using Server.Engines.Pathing.Cache; using Server.Items; using Server.PathAlgorithms; using Server.Spells; diff --git a/Projects/UOContent/Gumps/AdminGump.cs b/Projects/UOContent/Gumps/AdminGump.cs index f212a6796..3a48bc5aa 100644 --- a/Projects/UOContent/Gumps/AdminGump.cs +++ b/Projects/UOContent/Gumps/AdminGump.cs @@ -5,6 +5,7 @@ using System.Net; using System.Runtime.InteropServices; using System.Threading; using Server.Accounting; +using Server.Accounting.Security; using Server.Collections; using Server.Commands; using Server.Maps; @@ -2903,7 +2904,7 @@ namespace Server.Gumps else { notice = "The password has been changed."; - Server.Accounting.Security.PasswordWorker.SetPassword(a, password, null); + PasswordWorker.SetPassword(a, password, null); page = AdminGumpPage.AccountDetails_Information; CommandLogging.WriteLine( from, diff --git a/Projects/UOContent/Utilities/Types.cs b/Projects/UOContent/Utilities/Types.cs index fa669d96a..ca5b9422d 100644 --- a/Projects/UOContent/Utilities/Types.cs +++ b/Projects/UOContent/Utilities/Types.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Concurrent; -using System.Collections.Generic; using System.Globalization; using System.Reflection; using System.Runtime.CompilerServices;