acdream/src/AcDream.Core.Net/NetClient.cs
Erik cf25330458 fix(net): survive transient socket errors in the receive loop (2026-07-24 audit review)
WorldSession.NetReceiveLoop wrapped its entire while loop in a single
try/catch, so ANY non-timeout SocketException permanently killed the
background receive thread: the catch block at the loop's end was empty
(misattributed the error to "socket closed during shutdown"), and the
finally called _inboundQueue.Writer.TryComplete(), which silently and
irrecoverably stopped all inbound processing for the rest of the
session — no log line, no recovery path, and LiveSessionHost.Reconnect
has zero production callers to notice.

The realistic trigger is a well-known Windows UdpClient quirk: an ICMP
"port unreachable" reply to an EARLIER Send (e.g. against a stale ACE
session that already tore down its socket) surfaces as a
WSAECONNRESET SocketException on this socket's NEXT, completely
unrelated Receive call. NetClient.Receive already swallows the
expected SocketError.TimedOut heartbeat case; anything else reaching
WorldSession was a real, transient, per-datagram error being treated
as session-fatal.

Three changes, root-cause not a band-aid:

- NetClient's constructor now disables SIO_UDP_CONNRESET reporting on
  Windows, so a delayed ICMP error can't poison receives at all.
- NetReceiveLoop now catches SocketException PER ITERATION, logs it,
  and continues polling instead of exiting. The existing clean-shutdown
  paths (cancellation, ObjectDisposedException during Dispose) are
  unchanged — only the non-timeout-socket-error case that used to kill
  the loop is now recoverable.
- NetClient.Receive no longer calls the ReceiveTimeout setter (a
  setsockopt syscall) on every single call — only when the requested
  timeout differs from the last-applied value, cached in a new field.
  This was an unrelated but adjacent finding (4x/sec syscall churn at
  the 250ms heartbeat cadence) in the same audit.

No change to outbound wire behavior, ack cadence, heartbeat interval,
or datagram ordering — this is purely receive-loop resilience.

Tests: NetClientTests gained a SIO_UDP_CONNRESET construction smoke
test and two ReceiveTimeout-caching tests. A new
WorldSessionNetReceiveLoopResilienceTests drives the actual private
NetReceiveLoop method (via the existing internal
IWorldSessionTransport seam + reflection) with a scripted transport
that throws a non-timeout SocketException on the first call, proving
the loop survives it and keeps enqueueing subsequent datagrams — fully
deterministic, no real sockets. Full solution suite green: 3204/3206
Core, 3462/3465 App, 552/552 Core.Net (skips pre-existing).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit c72ce028a927e15b8a54a86bbd0661a723dc84ca)
2026-07-24 11:49:40 +02:00

131 lines
4.7 KiB
C#

using System.Net;
using System.Net.Sockets;
namespace AcDream.Core.Net;
/// <summary>
/// Minimum-viable UDP transport for acdream. Wraps a <see cref="UdpClient"/>
/// with synchronous send + timeout-based receive — good enough for the
/// Phase 4.6 handshake smoke test and early state-machine bring-up.
///
/// <para>
/// <b>Not yet provided</b> (deferred to a later phase once the handshake
/// actually works): background receive thread, outbound queue, ack/retransmit
/// window, heartbeat timer, concurrent send/receive. The acdream game loop
/// will need a real async pump eventually but building that now would be
/// debugging two things at once when we hit the first protocol mismatch.
/// </para>
/// </summary>
public sealed class NetClient : IDisposable
{
private readonly UdpClient _udp;
private readonly IPEndPoint _remote;
/// <summary>Last <see cref="Socket.ReceiveTimeout"/> value applied to the
/// underlying socket, so repeated <see cref="Receive"/> calls with the
/// same timeout (every 250ms heartbeat tick) skip the setsockopt syscall.</summary>
private int _lastAppliedReceiveTimeoutMs = int.MinValue;
public NetClient(IPEndPoint remote)
{
_remote = remote;
// Bind to an OS-assigned local port; server will reply to it.
_udp = new UdpClient(new IPEndPoint(IPAddress.Any, 0));
if (OperatingSystem.IsWindows())
{
// SIO_UDP_CONNRESET: Windows' UDP stack otherwise surfaces an
// ICMP "port unreachable" reply to an EARLIER Send (e.g. a stale
// server session that already tore down its socket) as a
// WSAECONNRESET SocketException on this socket's NEXT unrelated
// Receive call. An ICMP error from one send must not poison
// receiving for the rest of the session — disable that reporting.
const int SioUdpConnReset = -1744830452; // 0x9800000C
_udp.Client.IOControl((IOControlCode)SioUdpConnReset, new byte[] { 0 }, null);
}
}
/// <summary>The local endpoint the OS assigned us.</summary>
public IPEndPoint LocalEndPoint => (IPEndPoint)_udp.Client.LocalEndPoint!;
/// <summary>The remote endpoint we're talking to.</summary>
public IPEndPoint RemoteEndPoint => _remote;
/// <summary>
/// Send a datagram to the configured default remote. Blocks until the
/// OS has accepted the bytes (fast — just a kernel buffer copy on loopback).
/// </summary>
public void Send(ReadOnlySpan<byte> datagram)
{
_udp.Send(datagram.ToArray(), datagram.Length, _remote);
}
/// <summary>
/// Send a datagram to an arbitrary remote endpoint. Needed for the AC
/// handshake because the server binds separate listeners on port 9000
/// (LoginRequest) and port 9001 (ConnectResponse), so the second
/// handshake leg targets a different port than the first.
/// </summary>
public void Send(IPEndPoint remote, ReadOnlySpan<byte> datagram)
{
_udp.Send(datagram.ToArray(), datagram.Length, remote);
}
/// <summary>
/// Block until a datagram arrives or <paramref name="timeout"/> elapses.
/// Returns the raw bytes, or <c>null</c> on timeout. The sender's
/// endpoint is recorded in <paramref name="from"/> so the caller can
/// verify it matches the expected remote.
/// </summary>
public byte[]? Receive(TimeSpan timeout, out IPEndPoint? from)
{
int timeoutMs = (int)timeout.TotalMilliseconds;
if (timeoutMs != _lastAppliedReceiveTimeoutMs)
{
_udp.Client.ReceiveTimeout = timeoutMs;
_lastAppliedReceiveTimeoutMs = timeoutMs;
}
try
{
IPEndPoint any = new(IPAddress.Any, 0);
var bytes = _udp.Receive(ref any);
from = any;
return bytes;
}
catch (SocketException ex) when (ex.SocketErrorCode == SocketError.TimedOut)
{
from = null;
return null;
}
}
/// <summary>
/// Non-blocking receive: returns the next pending datagram if one is
/// already in the kernel buffer, or <c>null</c> if not. Never blocks,
/// so it's safe to call once per game-loop frame. Uses
/// <see cref="UdpClient.Available"/> under the hood.
/// </summary>
public byte[]? TryReceive(out IPEndPoint? from)
{
if (_udp.Available == 0)
{
from = null;
return null;
}
try
{
IPEndPoint any = new(IPAddress.Any, 0);
var bytes = _udp.Receive(ref any);
from = any;
return bytes;
}
catch (SocketException)
{
from = null;
return null;
}
}
public void Dispose() => _udp.Dispose();
}