harden session lifecycle and prevent double disconnects

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<SendOp, byte[]> 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();
         }
     }

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<GameStorage.Request> 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<Buff> buffs = Buffs.GetSaveCacheBuffs();
             IList<SkillCooldown> 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