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.
This commit is contained in:
Kamron Batman 2026-08-08 11:20:36 -07:00
parent 8f76a3ac31
commit cf15beab76
No known key found for this signature in database
GPG key ID: 7D81DF26D9A5D94A

View file

@ -83,11 +83,20 @@ internal sealed class PasswordVerificationWorker
private static readonly ILogger logger = LogFactory.GetLogger(typeof(PasswordVerificationWorker));
/// <summary>
/// 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 <c>SentFirstPacket</c>, so a connection can hold at
/// most one pending verify, and the engine caps concurrent connections at 4096
/// (<c>NetState.Network.cs</c>). 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.
/// </summary>
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