From 4a5847bccffdb2632742df03353d21c683251ab1 Mon Sep 17 00:00:00 2001 From: Grayson Adams Date: Thu, 30 Jul 2026 15:15:06 -0500 Subject: [PATCH 1/3] fix: harden notification reliability (queue sounds, safe async, pipe ack, crash cleanup) --- TopNotify/Daemon/Daemon.cs | 67 +++++++++++----- TopNotify/Daemon/InterceptorManager.cs | 76 ++++++++++++++++--- .../Daemon/Interceptors/SoundInterceptor.cs | 58 ++++++++++---- TopNotify/GUI/MainCommands.cs | 8 +- TopNotify/Program.cs | 20 +++++ 5 files changed, 185 insertions(+), 44 deletions(-) diff --git a/TopNotify/Daemon/Daemon.cs b/TopNotify/Daemon/Daemon.cs index 6c829f2..2752956 100644 --- a/TopNotify/Daemon/Daemon.cs +++ b/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,74 @@ public Daemon() { Thread managerThread = new Thread(CreateManager); managerThread.Start(); - Task.Run(MailSlotListener); + Task.Run(PipeListener); TrayIcon.MainLoop(); } /// - /// 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. /// - 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 reader = new StreamReader(server, Encoding.UTF8, false, 1024, leaveOpen: true)) + using (var writer = new StreamWriter(server, Encoding.UTF8, 1024, leaveOpen: true) { AutoFlush = true }) + { + var msg = await reader.ReadLineAsync(); + + if (msg == "UpdateConfig") // Runs when the user changes a setting from the GUI + { + InterceptorManager.Instance.OnSettingsChanged(); + } + + await writer.WriteLineAsync("ACK"); + } + } + } + catch (Exception ex) + { + Program.Logger.Warning(ex, "Daemon pipe listener iteration failed, restarting listener"); } } } /// - /// 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. /// - public static void SendCommandToDaemon(string message) + public static bool SendCommandToDaemon(string message, int timeoutMs = 1000) { try { - var buffer = new byte[1024]; - using (var client = MailSlot.CreateClient("samsidparty_topnotify")) + using (var client = new NamedPipeClientStream(".", PipeName, PipeDirection.InOut)) { - var bytes = Encoding.UTF8.GetBytes(message + "\n"); - client.Write(bytes, 0, bytes.Length); + client.Connect(timeoutMs); + + using (var writer = new StreamWriter(client, Encoding.UTF8, 1024, leaveOpen: true) { AutoFlush = true }) + using (var reader = new StreamReader(client, Encoding.UTF8, false, 1024, leaveOpen: true)) + { + writer.WriteLine(message); + return reader.ReadLine() == "ACK"; + } } } - catch (Exception ex) { } + catch (Exception ex) + { + Program.Logger.Warning(ex, $"SendCommandToDaemon('{message}') failed to reach the daemon"); + return false; + } } public void CreateManager() diff --git a/TopNotify/Daemon/InterceptorManager.cs b/TopNotify/Daemon/InterceptorManager.cs index 8906e36..0eb09d4 100644 --- a/TopNotify/Daemon/InterceptorManager.cs +++ b/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 lastLoggedFailure = new ConcurrentDictionary(); + 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"); } } } } diff --git a/TopNotify/Daemon/Interceptors/SoundInterceptor.cs b/TopNotify/Daemon/Interceptors/SoundInterceptor.cs index b8e460a..9b6a17f 100644 --- a/TopNotify/Daemon/Interceptors/SoundInterceptor.cs +++ b/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 soundQueue = new BlockingCollection(MAX_QUEUED_SOUNDS); + bool consumerStarted = false; bool allowedToPlaySound = false; @@ -193,24 +197,42 @@ 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. + if (!soundQueue.TryAdd(soundFilePath)) { - 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); } + /// + /// Dedicated Background Worker That Plays Queued Sounds One At A Time, In Order. + /// Runs For The Lifetime Of The Daemon Process. + /// + 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}"); + } + } + } + /// /// Plays a sound without any delay or timeout /// @@ -241,6 +263,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(); } } diff --git a/TopNotify/GUI/MainCommands.cs b/TopNotify/GUI/MainCommands.cs index 733a1b0..7e00ddd 100644 --- a/TopNotify/GUI/MainCommands.cs +++ b/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; } diff --git a/TopNotify/Program.cs b/TopNotify/Program.cs index 1bd3a25..58e8090 100644 --- a/TopNotify/Program.cs +++ b/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 From 0a18aca2bc1de004a6eeb16021b5b77347e4b1ba Mon Sep 17 00:00:00 2001 From: Grayson Adams Date: Thu, 30 Jul 2026 18:53:50 -0500 Subject: [PATCH 2/3] fix: BlockingCollection.TryAdd without a timeout blocks, not skips --- TopNotify/Daemon/Interceptors/SoundInterceptor.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/TopNotify/Daemon/Interceptors/SoundInterceptor.cs b/TopNotify/Daemon/Interceptors/SoundInterceptor.cs index 9b6a17f..9fa18d6 100644 --- a/TopNotify/Daemon/Interceptors/SoundInterceptor.cs +++ b/TopNotify/Daemon/Interceptors/SoundInterceptor.cs @@ -200,7 +200,12 @@ public override void OnNotification(UserNotification notification) // 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. - if (!soundQueue.TryAdd(soundFilePath)) + // + // NOTE: TryAdd(item) WITHOUT a timeout is NOT a non-blocking call on + // BlockingCollection - 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)) { Program.Logger.Warning($"SoundInterceptor: queue full ({MAX_QUEUED_SOUNDS} pending), dropping sound for {appRef.ID}"); } From 20f07affd3ffab065ca0cebd0e777e4adf81a756 Mon Sep 17 00:00:00 2001 From: Grayson Adams Date: Thu, 30 Jul 2026 19:15:10 -0500 Subject: [PATCH 3/3] fix: pipe I-O used uncancellable sync writes, could hang GUI thread forever --- TopNotify/Daemon/Daemon.cs | 46 ++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 12 deletions(-) diff --git a/TopNotify/Daemon/Daemon.cs b/TopNotify/Daemon/Daemon.cs index 2752956..e5fbd3c 100644 --- a/TopNotify/Daemon/Daemon.cs +++ b/TopNotify/Daemon/Daemon.cs @@ -33,11 +33,22 @@ public Daemon() { 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); + /// /// 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. /// async Task PipeListener() { @@ -49,17 +60,20 @@ async Task PipeListener() { await server.WaitForConnectionAsync(); - using (var reader = new StreamReader(server, Encoding.UTF8, false, 1024, leaveOpen: true)) - using (var writer = new StreamWriter(server, Encoding.UTF8, 1024, leaveOpen: true) { AutoFlush = true }) + using (var cts = new CancellationTokenSource(ServerConnectionTimeout)) { - var msg = await reader.ReadLineAsync(); + 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(); } - await writer.WriteLineAsync("ACK"); + var ackBytes = Encoding.UTF8.GetBytes("ACK"); + await server.WriteAsync(ackBytes, 0, ackBytes.Length, cts.Token); + await server.FlushAsync(cts.Token); } } } @@ -74,21 +88,29 @@ async Task PipeListener() /// 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. /// - public static bool SendCommandToDaemon(string message, int timeoutMs = 1000) + public static bool SendCommandToDaemon(string message, int timeoutMs = 2000) { try { - using (var client = new NamedPipeClientStream(".", PipeName, PipeDirection.InOut)) + using (var client = new NamedPipeClientStream(".", PipeName, PipeDirection.InOut, PipeOptions.Asynchronous)) + using (var cts = new CancellationTokenSource(timeoutMs)) { client.Connect(timeoutMs); - using (var writer = new StreamWriter(client, Encoding.UTF8, 1024, leaveOpen: true) { AutoFlush = true }) - using (var reader = new StreamReader(client, Encoding.UTF8, false, 1024, leaveOpen: true)) - { - writer.WriteLine(message); - return reader.ReadLine() == "ACK"; - } + 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)