acdream/tests/AcDream.Core.Net.Tests/NetClientTests.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

99 lines
4.2 KiB
C#

using System.Net;
using System.Net.Sockets;
using System.Reflection;
using AcDream.Core.Net;
namespace AcDream.Core.Net.Tests;
public class NetClientTests
{
[Fact]
public void Construction_AppliesWindowsConnResetMitigationWithoutThrowing()
{
// 2026-07-24 audit fix: SIO_UDP_CONNRESET must be disabled on
// Windows so a delayed ICMP port-unreachable from an earlier Send
// can't surface as a WSAECONNRESET SocketException on a later,
// unrelated Receive call. This is a smoke test — if the IOControl
// call regresses (wrong control code / wrong overload), the
// constructor itself throws here instead of only manifesting much
// later as a silently dead receive loop under real network churn.
using var client = new NetClient(new IPEndPoint(IPAddress.Loopback, 0));
Assert.NotNull(client.LocalEndPoint);
}
[Fact]
public void Receive_RepeatedSameTimeout_CachesLastAppliedValue()
{
// The setsockopt-equivalent (Socket.ReceiveTimeout assignment) used
// to run on every single Receive call (~4x/sec at the 250ms
// heartbeat cadence). It's now hoisted behind a last-applied-value
// cache. This test can't observe the syscall directly, but it does
// prove the cache field converges on the requested value and the
// underlying socket option stays in sync — a regression that
// stopped updating the cache (or stopped applying to the socket)
// would fail this.
using var client = new NetClient(new IPEndPoint(IPAddress.Loopback, 1));
client.Receive(TimeSpan.FromMilliseconds(50), out _);
client.Receive(TimeSpan.FromMilliseconds(50), out _);
FieldInfo cacheField = typeof(NetClient).GetField(
"_lastAppliedReceiveTimeoutMs", BindingFlags.NonPublic | BindingFlags.Instance)!;
Assert.Equal(50, (int)cacheField.GetValue(client)!);
FieldInfo udpField = typeof(NetClient).GetField(
"_udp", BindingFlags.NonPublic | BindingFlags.Instance)!;
var udp = (UdpClient)udpField.GetValue(client)!;
Assert.Equal(50, udp.Client.ReceiveTimeout);
}
[Fact]
public void Receive_DifferingTimeouts_UpdatesCachedValueEachTime()
{
using var client = new NetClient(new IPEndPoint(IPAddress.Loopback, 1));
client.Receive(TimeSpan.FromMilliseconds(40), out _);
client.Receive(TimeSpan.FromMilliseconds(120), out _);
FieldInfo cacheField = typeof(NetClient).GetField(
"_lastAppliedReceiveTimeoutMs", BindingFlags.NonPublic | BindingFlags.Instance)!;
Assert.Equal(120, (int)cacheField.GetValue(client)!);
FieldInfo udpField = typeof(NetClient).GetField(
"_udp", BindingFlags.NonPublic | BindingFlags.Instance)!;
var udp = (UdpClient)udpField.GetValue(client)!;
Assert.Equal(120, udp.Client.ReceiveTimeout);
}
[Fact]
public void SendReceive_LoopbackSelfTest_EchoRoundTrip()
{
// Two NetClients on loopback that send messages to each other.
// Proves Send + Receive actually work end-to-end over real UDP
// without depending on any remote server.
using var serverSide = new NetClient(new IPEndPoint(IPAddress.Loopback, 0));
using var clientSide = new NetClient(new IPEndPoint(IPAddress.Loopback, serverSide.LocalEndPoint.Port));
byte[] outbound = { 0xDE, 0xAD, 0xBE, 0xEF };
clientSide.Send(outbound);
var received = serverSide.Receive(TimeSpan.FromSeconds(2), out var from);
Assert.NotNull(received);
Assert.Equal(outbound, received);
Assert.NotNull(from);
Assert.Equal(IPAddress.Loopback, from!.Address);
Assert.Equal(clientSide.LocalEndPoint.Port, from.Port);
}
[Fact]
public void Receive_Timeout_ReturnsNullAndNoException()
{
// Listen for something that will never arrive; the timeout path
// must return null cleanly rather than leaking a SocketException.
using var client = new NetClient(new IPEndPoint(IPAddress.Loopback, 1));
var result = client.Receive(TimeSpan.FromMilliseconds(100), out var from);
Assert.Null(result);
Assert.Null(from);
}
}