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.
This commit is contained in:
Kamron Batman 2026-08-08 21:54:03 -07:00
parent b34c8b32ef
commit 2d9d83b2fd
No known key found for this signature in database
GPG key ID: 7D81DF26D9A5D94A
6 changed files with 52 additions and 14 deletions

View file

@ -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 <newPassword> <repeatPassword>")]

View file

@ -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 (<c>NetState.Network.cs</c>), 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.
/// </summary>
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.
/// </summary>
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()
/// <summary>
/// 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.
/// </summary>
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);
}
}
}
/// <summary>
/// Crash. There is no usable game thread, so nothing may be applied -- stop the thread and let
/// whatever was pending go. Subscribed separately because <c>HandleClosed</c> skips
/// <c>InvokeShutdown</c> when the server crashed.
/// </summary>
internal static void OnCrashed(ServerCrashedEventArgs e) => _instance?.StopThread();
private void StopThread()
{
_exit = true;
_work.Set();
_thread.Join(TimeSpan.FromSeconds(5));
}
}

View file

@ -1,6 +1,5 @@
using System;
using System.Buffers;
using System.Collections.Generic;
using System.Globalization;
using System.Numerics;
using System.Runtime.CompilerServices;

View file

@ -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;

View file

@ -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,

View file

@ -1,6 +1,5 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Globalization;
using System.Reflection;
using System.Runtime.CompilerServices;