From 43ce44caf036d6f9e8e184956ef088118e672431 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=82ngelo=20Tadeucci?= Date: Sat, 27 Sep 2025 21:10:07 -0300 Subject: [PATCH] harden session lifecycle and prevent double disconnects --- Maple2.Server.Core/Network/Session.cs | 64 ++++++++++++------- Maple2.Server.Game/Session/GameSession.cs | 75 +++++++++++++---------- 2 files changed, 85 insertions(+), 54 deletions(-) diff --git a/Maple2.Server.Core/Network/Session.cs b/Maple2.Server.Core/Network/Session.cs index 8537c6789..54c0f5683 100644 --- a/Maple2.Server.Core/Network/Session.cs +++ b/Maple2.Server.Core/Network/Session.cs @@ -2,6 +2,7 @@ using System.Collections.Concurrent; using System.IO.Pipelines; using System.Net.Sockets; +using System.Runtime.CompilerServices; using System.Security.Cryptography; using Maple2.Model.Enum; using Maple2.PacketLib.Crypto; @@ -32,6 +33,7 @@ public abstract class Session : IDisposable { public Action? OnLoop; private bool disposed; + private int disconnecting; // 0 = not disconnecting, 1 = disconnect in progress/already triggered (reentrancy guard) private readonly uint siv; private readonly uint riv; @@ -45,6 +47,8 @@ public abstract class Session : IDisposable { private readonly QueuedPipeScheduler pipeScheduler; private readonly Pipe recvPipe; + public long AccountId { get; protected set; } + public long CharacterId { get; protected set; } private readonly ConcurrentDictionary lastSentPackets = []; protected abstract PatchType Type { get; } @@ -87,10 +91,21 @@ protected virtual void Dispose(bool disposing) { disposed = true; State = SessionState.Disconnected; - Complete(); - thread.Join(STOP_TIMEOUT); - - CloseClient(); + try { + Complete(); + } catch (Exception ex) { + Logger.Debug(ex, "Complete() threw during Dispose"); + } + try { + thread.Join(STOP_TIMEOUT); + } catch (Exception ex) { + Logger.Debug(ex, "thread.Join failed"); + } + try { + CloseClient(); + } catch (Exception ex) { + Logger.Debug(ex, "CloseClient failed"); + } } protected void Complete() { @@ -99,10 +114,11 @@ protected void Complete() { pipeScheduler.Complete(); } - public void Disconnect() { + public void Disconnect([CallerMemberName] string caller = "", [CallerLineNumber] int line = 0, [CallerFilePath] string filePath = "") { if (disposed) return; - Logger.Information("Disconnected {Session}", this); + Logger.Information("Disconnected {Session} at {Caller} in {FilePath} on line {LineNumber}", this, caller, filePath, line); + if (Interlocked.Exchange(ref disconnecting, 1) == 1) return; Dispose(); } @@ -140,7 +156,12 @@ private void StartInternal() { // Pipeline tasks can be run asynchronously Task writeTask = WriteRecvPipe(client.Client, recvPipe.Writer); Task readTask = ReadRecvPipe(recvPipe.Reader); - Task.WhenAll(writeTask, readTask).ContinueWith(_ => CloseClient()); + Task.WhenAll(writeTask, readTask).ContinueWith(t => { + if (t.IsFaulted) { + Logger.Debug(t.Exception, "Pipeline aggregate fault account={AccountId} char={CharacterId}", AccountId, CharacterId); + } + CloseClient(); + }); while (!disposed && pipeScheduler.OutputAvailableAsync().Result) { pipeScheduler.ProcessQueue(); @@ -150,9 +171,7 @@ private void StartInternal() { if (!disposed) { Logger.Error(ex, "Exception on session thread"); } - } finally { - Disconnect(); - } + } finally { Disconnect(); } } private void PerformHandshake() { @@ -183,9 +202,7 @@ private async Task WriteRecvPipe(Socket socket, PipeWriter writer) { result = await writer.FlushAsync(); } while (!disposed && !result.IsCompleted); - } catch (Exception) { - Disconnect(); - } + } catch (Exception ex) { Logger.Debug(ex, "WriteRecvPipe exception account={AccountId} char={CharacterId}", AccountId, CharacterId); Disconnect(); } } private async Task ReadRecvPipe(PipeReader reader) { @@ -211,17 +228,20 @@ private async Task ReadRecvPipe(PipeReader reader) { reader.AdvanceTo(buffer.Start, buffer.End); } while (!disposed && !result.IsCompleted); } catch (Exception ex) { + // Stop web crawlers + if (ex.Message.StartsWith("Packet has invalid sequence header")) { + return; + } + if (ex is InvalidOperationException invalidOperation && invalidOperation.Message.Contains("reader was completed")) { // Ignore this exception, it happens when the reader is completed, either by the client closing or by the session being disposed. // it's not a real error return; } if (!disposed) { - Logger.Error(ex, "Exception reading recv packet"); + Logger.Error(ex, "Exception reading recv packet || AccountId: {AccountId}, CharacterId: {CharacterId}", AccountId, CharacterId); } - } finally { - Disconnect(); - } + } finally { Disconnect(); } } @@ -231,7 +251,7 @@ private async Task ReadRecvPipe(PipeReader reader) { * length: length of packet that only includes data */ private void SendInternal(byte[] packet, int length) { - if (disposed) return; + if (disposed || disconnecting == 1) return; #if DEBUG LogSend(packet, length); #endif @@ -243,17 +263,19 @@ private void SendInternal(byte[] packet, int length) { } lock (sendCipher) { + // re-check after potential delay acquiring lock + if (disposed || disconnecting == 1) return; using PoolByteWriter encryptedPacket = sendCipher.Encrypt(packet, 0, length); SendRaw(encryptedPacket); } } private void SendRaw(ByteWriter packet) { - if (disposed) return; - + if (disposed || disconnecting == 1) return; try { networkStream.Write(packet.Buffer, 0, packet.Length); - } catch (Exception) { + } catch (Exception ex) { + Logger.Debug(ex, "[LIFECYCLE] SendRaw write failed account={AccountId} char={CharacterId}", AccountId, CharacterId); Disconnect(); } } diff --git a/Maple2.Server.Game/Session/GameSession.cs b/Maple2.Server.Game/Session/GameSession.cs index 411d4a18d..5a98557a0 100644 --- a/Maple2.Server.Game/Session/GameSession.cs +++ b/Maple2.Server.Game/Session/GameSession.cs @@ -37,7 +37,8 @@ public sealed partial class GameSession : Core.Network.Session { protected override PatchType Type => PatchType.Ignore; public const int FIELD_KEY = 0x1234; - private bool disposed; + // gameDisposeState: 0 = active, 1 = disposing, 2 = disposed + private int gameDisposeState; private readonly GameServer server; public readonly CommandRouter CommandHandler; @@ -47,9 +48,6 @@ public sealed partial class GameSession : Core.Network.Session { public int ClientTick; public int Latency; - - public long AccountId { get; private set; } - public long CharacterId { get; private set; } public string PlayerName => Player.Value.Character.Name; public Guid MachineId { get; private set; } @@ -742,9 +740,12 @@ private void ReleaseLock(long accountId) { ~GameSession() => Dispose(false); protected override void Dispose(bool disposing) { - if (disposed) return; - if (Field is null) return; - disposed = true; + // Ensure dispose is only run once + if (Interlocked.CompareExchange(ref gameDisposeState, 1, 0) != 0) return; + // begin dispose + + // Snapshot values needed after teardown + long fieldTickSnapshot = Field?.FieldTick ?? Environment.TickCount64; if (State == SessionState.Connected) { PlayerInfo.SendUpdate(new PlayerUpdateRequest { @@ -761,57 +762,67 @@ protected override void Dispose(bool disposing) { try { Scheduler.Stop(); - OnLoop -= Scheduler.InvokeAll; + if (OnLoop != null) OnLoop -= Scheduler.InvokeAll; server.OnDisconnected(this); LeaveField(); Player.Value.Character.Channel = -1; Player.Value.Account.Online = false; State = SessionState.Disconnected; - Complete(); + // Early base dispose to stop further network sends + base.Dispose(disposing); + + // Cache config & persistence SaveCacheConfig(); AcquireLock(CharacterId); using GameStorage.Request db = GameStorage.Context(); db.BeginTransaction(); db.SavePlayer(Player); - UgcMarket.Save(db); - Config.Save(db); - Shop.Save(db); - Item.Save(db); - Survival.Save(db); - Housing.Save(db); - GameEvent.Save(db); - Achievement.Save(db); - Quest.Save(db); - Dungeon.Save(db); + TrySaveComponent(db, UgcMarket.Save); + TrySaveComponent(db, Config.Save); + TrySaveComponent(db, Shop.Save); + TrySaveComponent(db, Item.Save); + TrySaveComponent(db, Survival.Save); + TrySaveComponent(db, Housing.Save); + TrySaveComponent(db, GameEvent.Save); + TrySaveComponent(db, Achievement.Save); + TrySaveComponent(db, Quest.Save); + TrySaveComponent(db, Dungeon.Save); db.Commit(); db.SaveChanges(); } catch (Exception ex) { Logger.Error(ex, "Error during session cleanup for {Player}", PlayerName); } finally { - ReleaseLock(CharacterId); - Guild.Dispose(); - Buddy.Dispose(); - Party.Dispose(); + try { ReleaseLock(CharacterId); } catch (Exception ex) { Logger.Error(ex, "Error releasing lock for {Player}", PlayerName); } + SafeDispose(Guild); + SafeDispose(Buddy); + SafeDispose(Party); foreach ((int groupChatId, GroupChatManager groupChat) in GroupChats) { - groupChat.CheckDisband(); + try { groupChat.CheckDisband(); } catch (Exception ex) { Logger.Error(ex, "Error disbanding group chat {Id} for {Player}", groupChatId, PlayerName); } } - foreach ((long clubId, ClubManager club) in Clubs) { - club.Dispose(); + SafeDispose(club); } - - Player.Dispose(); - base.Dispose(disposing); + try { Player.Dispose(); } catch (Exception ex) { Logger.Error(ex, "Error disposing player for {Player}", PlayerName); } + Interlocked.Exchange(ref gameDisposeState, 2); } return; + void TrySaveComponent(GameStorage.Request db, Action action) { + try { action(db); } catch (Exception ex) { Logger.Error(ex, "Error saving component for {Player}", PlayerName); } + } + + void SafeDispose(IDisposable? disp) { + if (disp == null) return; + try { disp.Dispose(); } catch (Exception ex) { Logger.Error(ex, "Error disposing component for {Player}", PlayerName); } + } + void SaveCacheConfig() { List buffs = Buffs.GetSaveCacheBuffs(); IList skillCooldowns = Config.GetCurrentSkillCooldowns(); long stopTime = DateTime.Now.ToEpochSeconds(); - long fieldTick = Field.FieldTick; + long fieldTick = fieldTickSnapshot; try { PlayerConfigResponse _ = World.PlayerConfig(new PlayerConfigRequest { Save = new PlayerConfigRequest.Types.Save { @@ -843,9 +854,7 @@ void SaveCacheConfig() { }, RequesterId = CharacterId, }); - } catch (Exception ex) { - Logger.Error(ex, "Error saving buffs for {Player}", PlayerName); - } + } catch (Exception ex) { Logger.Error(ex, "Error saving buffs for {Player}", PlayerName); } } } #endregion