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.
This commit is contained in:
@@ -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
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user