diff --git a/Projects/Server.Tests/Helpers/MockAccount.cs b/Projects/Server.Tests/Helpers/MockAccount.cs new file mode 100644 index 000000000..8d8973d86 --- /dev/null +++ b/Projects/Server.Tests/Helpers/MockAccount.cs @@ -0,0 +1,43 @@ +using System; +using System.Collections.Generic; +using Server.Accounting; + +namespace Server.Tests.Network; + +/// +/// Minimal IAccount so a test NetState looks authenticated. +/// +public class MockAccount : IAccount +{ + public int TotalGold { get; } + public int TotalPlat { get; } + public bool DepositGold(int amount) => throw new NotImplementedException(); + public bool DepositPlat(int amount) => throw new NotImplementedException(); + public bool WithdrawGold(int amount) => throw new NotImplementedException(); + public bool WithdrawPlat(int amount) => throw new NotImplementedException(); + public long GetTotalGold() => throw new NotImplementedException(); + public int CompareTo(IAccount other) => throw new NotImplementedException(); + public string Username { get; } + public string Email { get; set; } + public AccessLevel AccessLevel { get; set; } + public int Length { get; } + public int Limit { get; set; } = 6; // Default to 6 character slots + public int Count { get; } + + private readonly Dictionary _mobiles = new(); + public Mobile this[int index] + { + get => _mobiles.GetValueOrDefault(index); + set => _mobiles[index] = value; + } + + public DateTime Created { get; set; } + public Serial Serial { get; } + public void Deserialize(IGenericReader reader) => throw new NotImplementedException(); + public void Serialize(IGenericWriter writer) => throw new NotImplementedException(); + public bool Deleted { get; } + public void Delete() => throw new NotImplementedException(); + public bool TrySetUsername(string username) => throw new NotImplementedException(); + public void SetPassword(string password) => throw new NotImplementedException(); + public bool CheckPassword(string password) => throw new NotImplementedException(); +} diff --git a/Projects/Server.Tests/Helpers/PacketTestUtilities.cs b/Projects/Server.Tests/Helpers/PacketTestUtilities.cs index 5c6c793d4..48d344d39 100644 --- a/Projects/Server.Tests/Helpers/PacketTestUtilities.cs +++ b/Projects/Server.Tests/Helpers/PacketTestUtilities.cs @@ -20,7 +20,13 @@ public static class PacketTestUtilities /// Uses a real Socket and RingSocket with actual buffers. /// Must be disposed after use (use 'using' statement). /// - public static NetState CreateTestNetState() + public static NetState CreateTestNetState() => CreateTestNetState(out _); + + /// + /// As , also handing back the peer socket so a test can read what the + /// server delivered. + /// + public static NetState CreateTestNetState(out Socket client) { NetState.Slice(); // Process disconnects/disposes @@ -65,6 +71,7 @@ public static class PacketTestUtilities var testSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); testSocket.Connect(IPAddress.Loopback, _testPort); _testSocketClients.Add(testSocket); + client = testSocket; // Slice until we have a new NetState instance added // AcceptEx is asynchronous, so we may need to wait/retry diff --git a/Projects/Server.Tests/Tests/Maps/ClientEnumeratorTests.cs b/Projects/Server.Tests/Tests/Maps/ClientEnumeratorTests.cs index bfbc18e94..629ec3961 100644 --- a/Projects/Server.Tests/Tests/Maps/ClientEnumeratorTests.cs +++ b/Projects/Server.Tests/Tests/Maps/ClientEnumeratorTests.cs @@ -416,41 +416,6 @@ public class ClientEnumeratorTests } } - private class MockAccount : IAccount - { - public int TotalGold { get; } - public int TotalPlat { get; } - public bool DepositGold(int amount) => throw new NotImplementedException(); - public bool DepositPlat(int amount) => throw new NotImplementedException(); - public bool WithdrawGold(int amount) => throw new NotImplementedException(); - public bool WithdrawPlat(int amount) => throw new NotImplementedException(); - public long GetTotalGold() => throw new NotImplementedException(); - public int CompareTo(IAccount other) => throw new NotImplementedException(); - public string Username { get; } - public string Email { get; set; } - public AccessLevel AccessLevel { get; set; } - public int Length { get; } - public int Limit { get; set; } = 6; // Default to 6 character slots - public int Count { get; } - - private readonly Dictionary _mobiles = new(); - public Mobile this[int index] - { - get => _mobiles.GetValueOrDefault(index); - set => _mobiles[index] = value; - } - - public DateTime Created { get; set; } - public Serial Serial { get; } - public void Deserialize(IGenericReader reader) => throw new NotImplementedException(); - public void Serialize(IGenericWriter writer) => throw new NotImplementedException(); - public bool Deleted { get; } - public void Delete() => throw new NotImplementedException(); - public bool TrySetUsername(string username) => throw new NotImplementedException(); - public void SetPassword(string password) => throw new NotImplementedException(); - public bool CheckPassword(string password) => throw new NotImplementedException(); - } - private static (NetState, Mobile) CreateClientWithMobile(Map map, Point3D location) { // Create test NetState with real socket and buffers diff --git a/Projects/Server.Tests/Tests/Network/NetStateDisconnectTests.cs b/Projects/Server.Tests/Tests/Network/NetStateDisconnectTests.cs new file mode 100644 index 000000000..adc513e2c --- /dev/null +++ b/Projects/Server.Tests/Tests/Network/NetStateDisconnectTests.cs @@ -0,0 +1,272 @@ +using System; +using System.Diagnostics; +using System.Net.Sockets; +using Server.Network; +using Xunit; + +namespace Server.Tests.Network; + +/// +/// Send-side behaviour of a NetState after its disconnect is handed to the socket. +/// +[Collection("Sequential Server Tests")] +public class NetStateDisconnectTests +{ + private static NetState CreateAuthenticatedNetState() + { + var ns = PacketTestUtilities.CreateTestNetState(); + ns.Account = new MockAccount(); // skips the unattached-socket sweep + return ns; + } + + private static (NetState ns, Mobile m) CreateClient(string name) + { + var ns = CreateAuthenticatedNetState(); + var m = new Mobile(World.NewMobile) { Name = name }; + m.DefaultMobileInit(); + ns.Mobile = m; + m.NetState = ns; + return (ns, m); + } + + [Fact] + public void Send_AfterDisconnectHandedToSocket_DropsPacket() + { + var ns = CreateAuthenticatedNetState(); + + try + { + ns.Disconnect("test"); + NetState.Slice(); // handoff; the in-flight recv keeps it pending + + Assert.True(ns.Running); + Assert.True(ns._socket.DisconnectPending); + Assert.Equal(0, ns._socket.SendBuffer.ReadableBytes); + + ns.Send([0x73, 0x00]); + + Assert.True(ns.CannotSendPackets()); + Assert.Equal(0, ns._socket.SendBuffer.ReadableBytes); + } + finally + { + ns.Dispose(); + } + } + + [Fact] + public void Send_AfterImmediateDisconnect_DropsPacket() + { + var ns = CreateAuthenticatedNetState(); + + // Immediate branch of RingSocket.Disconnect(): Connected drops, DisconnectPending never set, + // Disconnected event lands next Slice() + try + { + NetState.SocketManager.DisconnectImmediate(ns._socket); + + Assert.True(ns.Running); + Assert.False(ns._socket.Connected); + Assert.False(ns._socket.DisconnectPending); + + ns.Send([0x73, 0x00]); + + Assert.True(ns.CannotSendPackets()); + Assert.Equal(0, ns._socket.SendBuffer.ReadableBytes); + } + finally + { + ns.Dispose(); + } + } + + [Fact] + public void Send_BeforeDisconnect_IsDeliveredThenPeerSeesEof() + { + var ns = PacketTestUtilities.CreateTestNetState(out var client); + ns.Account = new MockAccount(); + + try + { + byte[] payload = [0x8C, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; + + ns.Send(payload); + ns.Disconnect("redirect"); + NetState.Slice(); // flush, then handoff + + Assert.True(ns._socket.DisconnectPending); + + var received = new byte[payload.Length]; + var total = 0; + var deadline = Stopwatch.StartNew(); + + while (total < payload.Length && deadline.ElapsedMilliseconds < 5000) + { + NetState.Slice(); + if (client.Poll(1000, SelectMode.SelectRead)) + { + var read = client.Receive(received, total, payload.Length - total, SocketFlags.None); + Assert.NotEqual(0, read); + total += read; + } + } + + Assert.Equal(payload, received); + + // FIN follows once the send completes + var eof = -1; + while (eof != 0 && deadline.ElapsedMilliseconds < 5000) + { + NetState.Slice(); + if (client.Poll(1000, SelectMode.SelectRead)) + { + eof = client.Receive(received); + } + } + + Assert.Equal(0, eof); + } + finally + { + ns.Dispose(); + client.Close(); + } + } + + [Fact] + public void Send_LargeBeforeDisconnect_IsFullyDrainedThenPeerSeesEof() + { + var ns = PacketTestUtilities.CreateTestNetState(out var client); + ns.Account = new MockAccount(); + + try + { + // Several posts' worth, larger than the loopback kernel buffers, so the drain spans completions + const int chunk = 32 * 1024; + var payload = new byte[6 * chunk]; + new System.Random(7).NextBytes(payload); + + for (var offset = 0; offset < payload.Length; offset += chunk) + { + ns.Send(payload.AsSpan(offset, chunk)); + } + + ns.Disconnect("redirect"); + NetState.Slice(); // flush, then handoff + + Assert.True(ns._socket.DisconnectPending); + + var received = new byte[payload.Length]; + var total = 0; + var deadline = Stopwatch.StartNew(); + + while (total < payload.Length && deadline.ElapsedMilliseconds < 10000) + { + NetState.Slice(); + if (client.Poll(1000, SelectMode.SelectRead)) + { + var read = client.Receive(received, total, payload.Length - total, SocketFlags.None); + Assert.NotEqual(0, read); + total += read; + } + } + + Assert.Equal(payload.Length, total); + Assert.Equal(payload, received); + + var eof = -1; + while (eof != 0 && deadline.ElapsedMilliseconds < 10000) + { + NetState.Slice(); + if (client.Poll(1000, SelectMode.SelectRead)) + { + eof = client.Receive(received); + } + } + + Assert.Equal(0, eof); + } + finally + { + ns.Dispose(); + client.Close(); + } + } + + [Fact] + public void PendingDisconnect_ForceClosesAfterDrainTimeout() + { + var ns = CreateAuthenticatedNetState(); + + try + { + ns.Disconnect("test"); + NetState.Slice(); // handoff; the in-flight recv keeps it pending and arms the deadline + + Assert.True(ns._socket.DisconnectPending); + var armed = Core.TickCount; + + ns.CheckAlive(armed + NetState.DrainTimeoutMs - 1); + Assert.True(ns._socket.Connected); + + ns.CheckAlive(armed + NetState.DrainTimeoutMs); + Assert.False(ns._socket.Connected); + } + finally + { + ns.Dispose(); + } + } + + [Fact] + public void SendBufferExhausted_WhileDisconnectQueued_KeepsFirstReason() + { + var ns = CreateAuthenticatedNetState(); + + try + { + var capacity = ns._socket.SendBuffer.PhysicalSize; + + ns.Send(new byte[capacity + 1]); // cannot fit; queues the disconnect + ns.Send(new byte[capacity + 2]); // same tick; must not re-report + + Assert.Contains($"needed {capacity + 1}", ns._disconnectReason); + } + finally + { + ns.Dispose(); + } + } + + [Fact] + public void CancelAllTrades_CancelsEveryTrade() + { + var (from, fromMobile) = CreateClient("from"); + var (to, toMobile) = CreateClient("to"); + + try + { + from.AddTrade(to); + var trade = from.FindTrade(toMobile); + Assert.NotNull(trade); + Assert.True(trade.Valid); + + from.CancelAllTrades(); + + Assert.False(trade.Valid); + Assert.Null(from.Trades); + Assert.Null(to.Trades); + Assert.Null(from.FindTrade(toMobile)); + Assert.Null(to.FindTrade(fromMobile)); + } + finally + { + from.Mobile = null; + from.Dispose(); + fromMobile.Delete(); + to.Mobile = null; + to.Dispose(); + toMobile.Delete(); + } + } +} diff --git a/Projects/Server/Network/NetState/NetState.Network.cs b/Projects/Server/Network/NetState/NetState.Network.cs index f69f92076..5bedb504c 100644 --- a/Projects/Server/Network/NetState/NetState.Network.cs +++ b/Projects/Server/Network/NetState/NetState.Network.cs @@ -61,6 +61,9 @@ public partial class NetState /// public static IIORingGroup Ring => _socketManager?.Ring; + // Test hook + internal static RingSocketManager SocketManager => _socketManager; + /// /// Waits for network I/O completions or until the specified timeout expires. /// Used by the game loop to sleep efficiently while remaining responsive to network events. @@ -107,6 +110,9 @@ public partial class NetState return; } + // Seed from a real tick; a zero default suppresses the sweep when ticks start negative + _nextAliveCheck = Core.TickCount; + // Initialize IP rate limiter _ipRateLimiter = new IPRateLimiter(10, 10000, 1000, 2.0, 3_600_000, Core.ClosingTokenSource.Token); @@ -539,6 +545,11 @@ public partial class NetState // - Waits for in-flight I/O to complete // - Ensures buffers aren't released while kernel is still using them ns._socket.Disconnect(); + + if (ns._socket.DisconnectPending) + { + ns.ArmDrainDeadline(curTicks); + } } } diff --git a/Projects/Server/Network/NetState/NetState.cs b/Projects/Server/Network/NetState/NetState.cs index 04f1a5a76..e161462fc 100755 --- a/Projects/Server/Network/NetState/NetState.cs +++ b/Projects/Server/Network/NetState/NetState.cs @@ -36,6 +36,7 @@ public partial class NetState : IComparable, IValueLinkListNode _flushPending = new(2048); private static readonly Queue _pendingDisconnects = new(256); // Processed AFTER flush @@ -54,7 +55,9 @@ public partial class NetState : IComparable, IValueLinkListNode, IValueLinkListNode= 0; --i) { - if (Trades != null) + // RemoveTrade() nulls the list once empty + if (Trades == null) { break; } @@ -489,6 +493,12 @@ public partial class NetState : IComparable, IValueLinkListNode, IValueLinkListNode private void SendBufferExhausted(int needed, int writable) { + // One report per disconnect; the first reason wins + if (_disconnectQueued) + { + return; + } + var sendBuffer = _socket?.SendBuffer; var unacked = sendBuffer?.InFlightBytes ?? 0; var capacity = sendBuffer?.PhysicalSize ?? 0; @@ -1053,31 +1069,52 @@ public partial class NetState : IComparable, IValueLinkListNode= 0) + if (_socket == null) { return; } if (_socket.DisconnectPending) { - LogInfo("Force disconnecting stuck socket..."); - _socketManager.DisconnectImmediate(_socket); - } - else - { - // Authenticated pre-game clients (login screens): send keep-alive instead of disconnecting. - // The 0xBD ClientVersionRequest resets NextActivityCheck via DataSent. - if (_account != null && Mobile == null) + ArmDrainDeadline(curTicks); // transport-initiated drains are first seen here + + if (curTicks - _drainDeadline >= 0 || NextActivityCheck - curTicks < 0) { - this.SendClientVersionRequest(); - return; + LogInfo("Force disconnecting stuck socket..."); + _socketManager.DisconnectImmediate(_socket); } - LogInfo("Disconnecting due to inactivity..."); - Disconnect("Disconnecting due to inactivity."); + return; } + + if (NextActivityCheck - curTicks >= 0) + { + return; + } + + // Authenticated pre-game clients (login screens): send keep-alive instead of disconnecting. + // The 0xBD ClientVersionRequest resets NextActivityCheck via DataSent. + if (_account != null && Mobile == null) + { + this.SendClientVersionRequest(); + return; + } + + LogInfo("Disconnecting due to inactivity..."); + Disconnect("Disconnecting due to inactivity."); } public void Trace(ReadOnlySpan buffer) @@ -1123,8 +1160,8 @@ public partial class NetState : IComparable, IValueLinkListNode - /// Requests a graceful disconnect. The disconnect is queued and processed after the flush - /// queue in Slice(), ensuring Send() calls made in the same tick are processed first. + /// Requests a graceful disconnect. Processed after the flush queue in Slice(): sends made before + /// that handoff are flushed first, sends after it are dropped (see CannotSendPackets). /// public void Disconnect(string reason) { diff --git a/Projects/Server/Network/Packets/OutgoingPackets.cs b/Projects/Server/Network/Packets/OutgoingPackets.cs index ce2cce0f4..23b8f15c6 100644 --- a/Projects/Server/Network/Packets/OutgoingPackets.cs +++ b/Projects/Server/Network/Packets/OutgoingPackets.cs @@ -6,8 +6,8 @@ public static class OutgoingPackets { [MethodImpl(MethodImplOptions.AggressiveInlining)] public static bool CannotSendPackets(this NetState ns) => - // Do not check for NetState.Running. Packets are sent to a "disconnected" socket as part of the OnDisconnect events - // up until the socket is closed. Closing the connection is done synchronously, therefore packets will not be sent - // once the Mobile.NetState is null. - ns == null || ns.SocketHandle == 0 || ns.BlockAllPackets; + // Running is not checked: sends between Disconnect() and the Slice() handoff must still go out. + // After the handoff the socket is draining (DisconnectPending) or closing (!Connected); new writes + // only keep the buffer from draining. + ns == null || ns.SocketHandle == 0 || !ns._socket.Connected || ns._socket.DisconnectPending || ns.BlockAllPackets; } diff --git a/Projects/Server/Server.csproj b/Projects/Server/Server.csproj index 8ec416218..030197955 100644 --- a/Projects/Server/Server.csproj +++ b/Projects/Server/Server.csproj @@ -34,7 +34,7 @@ - +