fix: harden notification sound playback against hangs and dropped sounds

TopNotify/Daemon/Daemon.cs

@@ -6,14 +6,16 @@
 using System.Text;
 using System.Threading.Tasks;
 using System.Diagnostics;
+using System.IO.Pipes;
 using TopNotify.Common;
 using TopNotify.GUI;
-using KdSoft.MailSlot;
 
 namespace TopNotify.Daemon
 {
     public class Daemon
     {
+        const string PipeName = "samsidparty_topnotify";
+
         public static Daemon Instance;
 
         public InterceptorManager Manager;
@@ -26,43 +28,96 @@ public Daemon() {
             Thread managerThread = new Thread(CreateManager);
             managerThread.Start();
 
-            Task.Run(MailSlotListener);
+            Task.Run(PipeListener);
 
             TrayIcon.MainLoop();
         }
 
+        // Per-connection ceiling on the daemon side. Bounds how long one stuck client
+        // can hold up the single-instance server loop before it gives up and accepts
+        // the next connection.
+        static readonly TimeSpan ServerConnectionTimeout = TimeSpan.FromSeconds(5);
+
         /// <summary>
-        /// Listens for messages that affect the app lifecycle
+        /// Listens for messages that affect the app lifecycle. Named pipe replaces the old
+        /// mailslot transport, which had no delivery confirmation - a settings change sent
+        /// while the daemon wasn't yet listening (or under any other timing edge case) would
+        /// silently vanish, leaving the daemon on stale config with no indication to the user.
+        ///
+        /// Uses raw ReadAsync/WriteAsync directly on the PipeStream rather than
+        /// StreamReader/StreamWriter - AutoFlush on a StreamWriter wrapping an async-mode
+        /// pipe can fall back to a synchronous Flush() internally, which issues an
+        /// uncancellable blocking Win32 WriteFile call with no timeout. Every I/O call here
+        /// is bound to a CancellationToken so a stuck peer can never hang this loop forever.
         /// </summary>
-        async Task MailSlotListener()
+        async Task PipeListener()
         {
-            var listener = new AsyncMailSlotListener("samsidparty_topnotify", Encoding.ASCII.GetBytes("\n")[0]);
-            await foreach (var msgBytes in listener.GetNextMessage())
+            while (true)
             {
-                var msg = Encoding.UTF8.GetString(msgBytes);
-
-                if (msg == "UpdateConfig") // Runs when the user changes a setting from the GUI
+                try
                 {
-                    InterceptorManager.Instance.OnSettingsChanged();
+                    using (var server = new NamedPipeServerStream(PipeName, PipeDirection.InOut, 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous))
+                    {
+                        await server.WaitForConnectionAsync();
+
+                        using (var cts = new CancellationTokenSource(ServerConnectionTimeout))
+                        {
+                            var buffer = new byte[1024];
+                            var bytesRead = await server.ReadAsync(buffer, 0, buffer.Length, cts.Token);
+                            var msg = Encoding.UTF8.GetString(buffer, 0, bytesRead);
+
+                            if (msg == "UpdateConfig") // Runs when the user changes a setting from the GUI
+                            {
+                                InterceptorManager.Instance.OnSettingsChanged();
+                            }
+
+                            var ackBytes = Encoding.UTF8.GetBytes("ACK");
+                            await server.WriteAsync(ackBytes, 0, ackBytes.Length, cts.Token);
+                            await server.FlushAsync(cts.Token);
+                        }
+                    }
+                }
+                catch (Exception ex)
+                {
+                    Program.Logger.Warning(ex, "Daemon pipe listener iteration failed, restarting listener");
                 }
             }
         }
 
         /// <summary>
-        /// This should be called from an external (non-daemon) process to send a message to the daemon
+        /// This should be called from an external (non-daemon) process to send a message to the daemon.
+        /// Returns true only once the daemon has acknowledged receipt - callers can use this to warn
+        /// the user if a settings change didn't actually reach the running daemon.
+        ///
+        /// Every step past Connect() is bound to timeoutMs via a CancellationToken, and the pipe
+        /// is explicitly opened with PipeOptions.Asynchronous so that bound is actually enforceable -
+        /// a synchronously-opened pipe's Write can't be cancelled once issued, which is what turned a
+        /// transient stall into a permanent freeze of the calling (GUI/UI) thread previously.
         /// </summary>
-        public static void SendCommandToDaemon(string message)
+        public static bool SendCommandToDaemon(string message, int timeoutMs = 2000)
         {
             try
             {
-                var buffer = new byte[1024];
-                using (var client = MailSlot.CreateClient("samsidparty_topnotify"))
+                using (var client = new NamedPipeClientStream(".", PipeName, PipeDirection.InOut, PipeOptions.Asynchronous))
+                using (var cts = new CancellationTokenSource(timeoutMs))
                 {
-                    var bytes = Encoding.UTF8.GetBytes(message + "\n");
-                    client.Write(bytes, 0, bytes.Length);
+                    client.Connect(timeoutMs);
+
+                    var msgBytes = Encoding.UTF8.GetBytes(message);
+                    client.WriteAsync(msgBytes, 0, msgBytes.Length, cts.Token).GetAwaiter().GetResult();
+                    client.FlushAsync(cts.Token).GetAwaiter().GetResult();
+
+                    var buffer = new byte[1024];
+                    var bytesRead = client.ReadAsync(buffer, 0, buffer.Length, cts.Token).GetAwaiter().GetResult();
+                    var response = Encoding.UTF8.GetString(buffer, 0, bytesRead);
+                    return response == "ACK";
                 }
             }
-            catch (Exception ex) { }
+            catch (Exception ex)
+            {
+                Program.Logger.Warning(ex, $"SendCommandToDaemon('{message}') failed to reach the daemon");
+                return false;
+            }
         }
 
         public void CreateManager()

TopNotify/Daemon/InterceptorManager.cs

@@ -32,6 +32,24 @@ public class InterceptorManager
         public UserNotificationListener Listener;
         public bool CanListenToNotifications = false;
 
+        // Update() Runs Every ~10ms Via MainLoop; A Persistently-Throwing Interceptor Would
+        // Otherwise Flood daemon.log At That Rate. Log The First Occurrence Immediately, Then
+        // At Most Once Per 30s Per (Interceptor, Method) Pair.
+        readonly ConcurrentDictionary<string, DateTime> lastLoggedFailure = new ConcurrentDictionary<string, DateTime>();
+        static readonly TimeSpan FailureLogThrottle = TimeSpan.FromSeconds(30);
+
+        void LogThrottled(string interceptorName, string methodName, Exception ex)
+        {
+            var key = $"{interceptorName}.{methodName}";
+            var now = DateTime.UtcNow;
+
+            if (!lastLoggedFailure.TryGetValue(key, out var last) || now - last >= FailureLogThrottle)
+            {
+                lastLoggedFailure[key] = now;
+                Program.Logger.Warning(ex, $"{key}() failed");
+            }
+        }
+
         public static Interceptor[] InstalledInterceptors =
         {
             new NativeInterceptor(),
@@ -119,7 +137,7 @@ public void Reflow()
                 {
                     i.Reflow();
                 }
-                catch (Exception ex) { }
+                catch (Exception ex) { LogThrottled(i.GetType().Name, "Reflow", ex); }
             }
         }
 
@@ -131,7 +149,7 @@ public void Update()
                 {
                     i.Update();
                 }
-                catch (Exception ex) { }
+                catch (Exception ex) { LogThrottled(i.GetType().Name, "Update", ex); }
             }
         }
 
@@ -150,29 +168,63 @@ public void OnKeyUpdate()
                 {
                     i.OnKeyUpdate();
                 }
-                catch (Exception ex) { }
+                catch (Exception ex) { LogThrottled(i.GetType().Name, "OnKeyUpdate", ex); }
             }
         }
 
-        // Runs When A New Notification Is Added Or Removed
-        public async void OnNotificationChanged(UserNotificationListener sender, UserNotificationChangedEventArgs args)
+        // Runs When A New Notification Is Added Or Removed.
+        // WinRT Event Handlers Must Be void, So This Stays A Thin Wrapper Around An
+        // async Task Body That Owns All Its Own Exceptions - An Unhandled Exception
+        // Inside An `async void` Method Can't Be Caught By Anything Upstream And Would
+        // Otherwise Take Down The Whole Notification Listener Silently.
+        public void OnNotificationChanged(UserNotificationListener sender, UserNotificationChangedEventArgs args)
         {
-            var userNotifications = await Listener.GetNotificationsAsync(NotificationKinds.Toast);
-            var userNotification = userNotifications.Where((n) => n.Id == args.UserNotificationId).FirstOrDefault();
+            _ = OnNotificationChangedAsync(sender, args);
+        }
 
-            if (args.ChangeKind == UserNotificationChangedKind.Added)
+        async Task OnNotificationChangedAsync(UserNotificationListener sender, UserNotificationChangedEventArgs args)
+        {
+            try
             {
+                if (args.ChangeKind != UserNotificationChangedKind.Added)
+                {
+                    return;
+                }
+
+                // GetNotificationsAsync Is The Only WinRT Call Available - There's No
+                // Single-Item Lookup By Id, So This Full Re-Fetch Isn't Avoidable.
+                var userNotifications = await Listener.GetNotificationsAsync(NotificationKinds.Toast);
+                var userNotification = userNotifications.FirstOrDefault((n) => n.Id == args.UserNotificationId);
+
+                if (userNotification == null)
+                {
+                    // Can Happen If The Notification Was Already Dismissed/Replaced By The
+                    // Time This Round Trip Resolves. Previously This Silently Passed null
+                    // Into Every Interceptor; Now It's A Logged, Explicit Skip.
+                    Program.Logger.Warning($"OnNotificationChanged: notification {args.UserNotificationId} not found in snapshot, skipping");
+                    return;
+                }
+
                 foreach (Interceptor i in Interceptors)
                 {
                     try
                     {
                         i.OnNotification(userNotification);
                     }
-                    catch { }
+                    catch (Exception ex)
+                    {
+                        Program.Logger.Warning(ex, $"{i.GetType().Name}.OnNotification() failed");
+                    }
                 }
-            }
 
-            Update();
+                Update();
+            }
+            catch (Exception ex)
+            {
+                // This Is The Critical Catch: A COM/WinRT Hiccup In GetNotificationsAsync
+                // No Longer Kills The Listener For Every Notification After It.
+                DaemonErrorHandler.ThrowNonCritical(new DaemonError("notification_changed_failure", "Failed to process an incoming notification: " + ex.Message));
+            }
         }
 
         public void OnSettingsChanged()
@@ -186,7 +238,7 @@ public void OnSettingsChanged()
                     i.Restart();
                     i.Reflow();
                 }
-                catch (Exception ex) { }
+                catch (Exception ex) { Program.Logger.Warning(ex, $"{i.GetType().Name} failed to apply settings change"); }
             }
         }
     }

TopNotify/Daemon/Interceptors/SoundInterceptor.cs

@@ -1,6 +1,7 @@
 using IgniteView.Core;
 using Microsoft.Win32;
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Data;
 using System.Diagnostics;
@@ -23,8 +24,11 @@ public class SoundInterceptor : Interceptor
         // This File Is Used To Replace The Default Notification Sounds, So That TopNotify Can Play A Different Sound
         const string FAKE_SOUND = "internal/silent";
 
-        SoundPlayer Player;
-        bool isPlaying = false;
+        // Bounded so a genuine flood can't grow this unbounded; normal back-to-back
+        // notifications (a couple per second) never get close to this ceiling.
+        const int MAX_QUEUED_SOUNDS = 8;
+        readonly BlockingCollection<string> soundQueue = new BlockingCollection<string>(MAX_QUEUED_SOUNDS);
+        bool consumerStarted = false;
 
         bool allowedToPlaySound = false;
 
@@ -193,24 +197,47 @@ public override void OnNotification(UserNotification notification)
             var appRef = AppReference.FromNotification(notification);
             var soundFilePath = GetSoundPath(appRef.SoundPath);
 
-            if (!isPlaying)
+            // Enqueue Instead Of Dropping: Two Notifications Arriving Close Together
+            // (E.g. Two Chat Apps Within The Same Second) Now Both Get A Sound Cue,
+            // Played In Order, Instead Of The Second One Silently Vanishing.
+            //
+            // NOTE: TryAdd(item) WITHOUT a timeout is NOT a non-blocking call on
+            // BlockingCollection<T> - it behaves like Add() and blocks the calling
+            // thread (here, the WinRT notification-event thread) until space frees up.
+            // Passing TimeSpan.Zero is what actually makes this a non-blocking attempt.
+            if (!soundQueue.TryAdd(soundFilePath, TimeSpan.Zero))
             {
-                isPlaying = true;
-
-                // Play Sound Without Blocking The Main Thread
-                Task.Run(() =>
-                {
-                    Player = new SoundPlayer(soundFilePath);
-                    Player.Load();
-                    Player.PlaySync();
-                    Player.Dispose();
-                    isPlaying = false;
-                });
+                Program.Logger.Warning($"SoundInterceptor: queue full ({MAX_QUEUED_SOUNDS} pending), dropping sound for {appRef.ID}");
             }
 
             base.OnNotification(notification);
         }
 
+        /// <summary>
+        /// Dedicated Background Worker That Plays Queued Sounds One At A Time, In Order.
+        /// Runs For The Lifetime Of The Daemon Process.
+        /// </summary>
+        void RunPlaybackConsumer()
+        {
+            foreach (var soundFilePath in soundQueue.GetConsumingEnumerable())
+            {
+                try
+                {
+                    using (var player = new SoundPlayer(soundFilePath))
+                    {
+                        player.Load();
+                        player.PlaySync();
+                    }
+                }
+                catch (Exception ex)
+                {
+                    // Non-critical: a single bad/locked sound file shouldn't take down
+                    // playback for every notification after it, so log and keep consuming.
+                    Program.Logger.Warning(ex, $"SoundInterceptor: failed to play {soundFilePath}");
+                }
+            }
+        }
+
         /// <summary>
         /// Plays a sound without any delay or timeout
         /// </summary>
@@ -241,6 +268,14 @@ public override void Restart()
         public override void Start()
         {
             Restart();
+
+            if (!consumerStarted)
+            {
+                consumerStarted = true;
+                var worker = new Thread(RunPlaybackConsumer) { IsBackground = true, Name = "SoundInterceptor.Playback" };
+                worker.Start();
+            }
+
             base.Start();
         }
     }

TopNotify/GUI/MainCommands.cs

@@ -103,7 +103,13 @@ public static void WriteConfigFile(WebWindow target, string data)
             Thread.Sleep(100); // Prevent Crashing Daemon From Spamming Button
 
             // Tell The Daemon The Config Has Changed
-            Daemon.Daemon.SendCommandToDaemon("UpdateConfig");
+            var delivered = Daemon.Daemon.SendCommandToDaemon("UpdateConfig");
+            if (!delivered)
+            {
+                // Settings Were Saved To Disk Either Way, But The Running Daemon May Not
+                // Have Picked Them Up - Surface This Rather Than Failing Silently.
+                Program.Logger.Warning("Settings were saved but the daemon did not acknowledge the update - it may be using stale config until restarted");
+            }
 
             isSaving = false;
         }

TopNotify/Program.cs

@@ -52,9 +52,29 @@ public static void Main(string[] args)
         {
             AppDomain.CurrentDomain.UnhandledException += (object sender, UnhandledExceptionEventArgs e) =>
             {
+                // Best-Effort: If This Is The Daemon Process And It's About To Die From An
+                // Unhandled Exception, Revert The Global Notification-Sound Registry Mute
+                // So The User Isn't Left With Silent Windows Notifications System-Wide.
+                // Note This Cannot Help Against A Hard Kill (taskkill /F, Task Manager "End
+                // Task", BSOD) - Those Terminate Before Any Managed Code Runs, In Any App.
+                if (Background != null)
+                {
+                    SoundInterceptor.UninstallSoundInRegistry();
+                }
+
                 NotificationTester.MessageBox("Something went wrong with TopNotify", "Unfortunately, TopNotify has crashed. Details: " + e.ExceptionObject.ToString());
             };
 
+            AppDomain.CurrentDomain.ProcessExit += (sender, e) =>
+            {
+                // Covers Normal/Graceful Exits (E.g. User Quits From The Tray Icon) That
+                // Don't Go Through The Unhandled-Exception Path Above.
+                if (Background != null)
+                {
+                    SoundInterceptor.UninstallSoundInRegistry();
+                }
+            };
+
             //By Default, The App Will Be Launched In Daemon Mode
             //Daemon Mode Is A Background Process That Handles Changing The Position Of Notifications
             //If The "--settings" Arg Is Used, Then The App Will Launch In Settings Mode