From cf15beab76077669ea6a6389951ea6b46ce0a56c Mon Sep 17 00:00:00 2001 From: Kamron Batman <3953314+kamronbatman@users.noreply.github.com> Date: Sat, 8 Aug 2026 11:20:36 -0700 Subject: [PATCH] perf(login): drop dead connections before hashing, size the cap to reality Two fixes to the pending queue. A queued job whose client has gone was still costing a full ~9 ms verify for a verdict nobody would receive. It is now dropped at dequeue, which reclaims the slot immediately -- and that matters most during the reconnect rush that builds a queue in the first place. Running only ever transitions true to false, so reading it off-thread can waste a hash but can never skip a live connection. The 128 cap was a second, far tighter bound on something the engine already bounds. AccountLogin rejects a duplicate 0x80 via SentFirstPacket, so a connection holds at most one pending verify, and concurrent connections are capped at 4096. The cap now matches that, making it a backstop against the one-per-connection invariant breaking rather than a policy that fires first on real players -- an 800-client boot rush was eight times over the old limit. It was never a flood defence and no longer claims to be. A cap low enough to blunt an attack rejects legitimate logins for its duration, which is the denial of service arriving by another route; a cap high enough to avoid that blunts nothing. That belongs at the connection layer, where IP bans and the accept gate stop an attacker before he costs a hash. --- .../Security/PasswordVerificationWorker.cs | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/Projects/UOContent/Accounting/Security/PasswordVerificationWorker.cs b/Projects/UOContent/Accounting/Security/PasswordVerificationWorker.cs index ea08cc3dc..7e5e148bc 100644 --- a/Projects/UOContent/Accounting/Security/PasswordVerificationWorker.cs +++ b/Projects/UOContent/Accounting/Security/PasswordVerificationWorker.cs @@ -83,11 +83,20 @@ internal sealed class PasswordVerificationWorker private static readonly ILogger logger = LogFactory.GetLogger(typeof(PasswordVerificationWorker)); /// - /// Pending cap. Overflow rejects the login rather than verifying it inline: falling back to the - /// loop would let anyone who fills the queue steer the work back onto the thread this exists to - /// protect. + /// Backstop, not a policy. The queue is already bounded by construction: AccountLogin rejects a + /// second 0x80 on the same connection via SentFirstPacket, so a connection can hold at + /// most one pending verify, and the engine caps concurrent connections at 4096 + /// (NetState.Network.cs). This matches that bound so it can only trip if the + /// one-per-connection invariant is ever broken. + /// + /// Deliberately not a flood defence. Any cap low enough to blunt an attack rejects real players + /// first -- during a mass reconnect they are the queue -- and a cap high enough not to do that + /// blunts nothing. Refusing logins for the duration of an attack *is* the denial of service, + /// just self-inflicted. That defence belongs at the connection layer, where IP bans, the + /// blocklist and the accept gate already sit and where an attacker is stopped before costing a + /// hash at all. /// - internal const int MaxPending = 128; + internal const int MaxPending = 4096; // Nothing signals the worker when a save freeze ends, so it re-checks on this interval -- but // only while a save is in progress, never in steady state. @@ -188,6 +197,16 @@ internal sealed class PasswordVerificationWorker Interlocked.Decrement(ref _pending); + // Gone while it waited. Skipping here reclaims the slot without spending ~9 ms on a + // verdict nobody will receive, which matters most during exactly the reconnect rush + // that builds a queue in the first place. Running only ever goes true -> false, so a + // stale read costs a wasted hash that Apply discards -- it can never skip a live + // connection. + if (job.State?.Running != true) + { + continue; + } + PasswordVerificationOutcome outcome; try