From 24dff3cc2ebfc8087f0b78fdc20a6492004c4fe6 Mon Sep 17 00:00:00 2001
From: Jon Kazama <kontakt@hellion-media.de>
Date: Mon, 17 Aug 2026 07:27:21 +0200
Subject: [PATCH] fix(messages): snapshot the tab list before delivering a
 message

ProcessMessage walked Config.Tabs live on the worker thread while SaveConfig's
strip and the auto-tell spawn mutated the same list under TabsListLock. The
resulting "collection was modified" was caught by the pending-message handler
and only logged -- so the message was dropped entirely: no tab entry, no sound,
and MessageProcessed never fired, which also meant no tell tab and no routing.
Silent message loss, exactly under the load where it hurts.

The loop now runs over a snapshot taken under the lock. AddMessage stays
outside it, so the lock order (list outer, MessageList inner) is unchanged.

SelectNotificationSound reports which tab it picked, so playback can skip a tab
that disappeared between snapshot and sound -- otherwise the snapshot would let
an evicted tab still make noise.

While here: the current tab was read twice despite the comment claiming it was
snapshotted once.
---
 HellionChat/MessageManager.cs | 38 +++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/HellionChat/MessageManager.cs b/HellionChat/MessageManager.cs
index dcb10a2..31f8b1d 100644
--- a/HellionChat/MessageManager.cs
+++ b/HellionChat/MessageManager.cs
@@ -359,12 +359,21 @@ internal class MessageManager : IAsyncDisposable
         if (Plugin.Config.DatabaseBattleMessages || !message.Code.IsBattle())
             Store.UpsertMessage(message);
 
+        // Snapshot the list, not just the active tab. This loop runs on the worker
+        // thread while SaveConfig's strip and the auto-tell spawn mutate Config.Tabs
+        // under TabsListLock — enumerating it live throws "collection was modified",
+        // and the catch in ProcessPendingMessages swallows that, silently dropping
+        // the whole message: no tab entry, no sound, no MessageProcessed.
+        List<Tab> tabsSnapshot;
+        lock (Plugin.TabsListLock)
+            tabsSnapshot = Plugin.Config.Tabs.ToList();
+
         // Snapshot the active tab and whether it shows this message ONCE, so the
         // whole loop sees a consistent value (the getter is a cross-thread read of
         // MainWindow.ActiveTab).
         var currentTab = Plugin.CurrentTab;
         var currentTabMatches = currentTab.Matches(message);
-        foreach (var tab in Plugin.Config.Tabs)
+        foreach (var tab in tabsSnapshot)
         {
             if (tab.Matches(message))
                 tab.AddMessage(message, ShouldCountUnread(tab, currentTab, currentTabMatches));
@@ -374,12 +383,24 @@ internal class MessageManager : IAsyncDisposable
         // stays pure and SelfTest-able; AddMessage above and playback below keep
         // the side effects.
         var notificationSound = SelectNotificationSound(
-            Plugin.Config.Tabs,
-            Plugin.CurrentTab,
+            tabsSnapshot,
+            currentTab,
             message,
-            Plugin.Config.PlaySounds
+            Plugin.Config.PlaySounds,
+            out var soundSource
         );
 
+        // The snapshot can outlive a tab (eviction, logout). Playing its sound would
+        // be an audible artefact for a tab that is already gone, so re-check first.
+        if (notificationSound is not null && soundSource is not null)
+        {
+            bool sourceStillPresent;
+            lock (Plugin.TabsListLock)
+                sourceStillPresent = Plugin.Config.Tabs.Contains(soundSource);
+            if (!sourceStillPresent)
+                notificationSound = null;
+        }
+
         if (notificationSound is { } soundId)
         {
             if (soundId is >= 1 and <= 16)
@@ -427,14 +448,18 @@ internal class MessageManager : IAsyncDisposable
             && currentTabMatches
         );
 
+    // Reports the tab the sound came from, so the caller can drop it if that tab
+    // disappeared between snapshot and playback.
     internal static uint? SelectNotificationSound(
         IEnumerable<Tab> tabs,
         Tab currentTab,
         Message probe,
-        bool playSounds
+        bool playSounds,
+        out Tab? source
     )
     {
         uint? picked = null;
+        source = null;
         foreach (var tab in tabs)
         {
             if (!tab.Matches(probe))
@@ -449,6 +474,7 @@ internal class MessageManager : IAsyncDisposable
             )
             {
                 picked = tab.NotificationSoundId;
+                source = tab;
             }
         }
         return picked;
@@ -460,7 +486,7 @@ internal class MessageManager : IAsyncDisposable
         Tab currentTab,
         Message probe,
         bool playSounds
-    ) => SelectNotificationSound(tabs, currentTab, probe, playSounds);
+    ) => SelectNotificationSound(tabs, currentTab, probe, playSounds, out _);
 
     internal class NameFormatting
     {