From 4f103f00754a3e6848564942089d324c587420fe Mon Sep 17 00:00:00 2001 From: shabhui <> Date: Tue, 1 Sep 2026 16:38:20 +0800 Subject: [PATCH 1/3] fix: resolve tray notification deadlock and monitor thread busy-spin The app could freeze in a state where the window stopped responding and the process could only be ended from Task Manager. Root cause is a lock-ordering (AB-BA) deadlock around the tray icon. NotificationService.Update ran on a background monitor thread, took _disposeLock, and then reached a blocking Dispatcher.Invoke by way of GetIcon -> GetImageIcon -> Start/StopRotationAnimation. Meanwhile the UI thread took the same _disposeLock in OnRotationTimerTick, which fires every 200ms while optimizing. Each side then waited on the other. Because Update is called inside the view model's _lockObject, the wedged thread held both locks, so MonitorComputer, MonitorApp and Optimize all blocked behind it, and Dispose could never acquire _disposeLock to exit. That is what made the process unkillable by normal means. The default TrayIconShowMemoryUsage=false routes through the rotation animation, so the default configuration is the affected one. Changes: - NotificationService: render the icon outside all locks and marshal only the assignment to the UI thread through a new non-blocking InvokeOnUi helper. Remove _disposeLock from OnRotationTimerTick. Add _iconRenderLock to serialize GDI reads of the shared _imageIcon, and _currentIconLock to guard the icon swap so concurrent callers cannot double-dispose. Marshal Loading and Notify, which are called from background threads and touch NotifyIcon members that have thread affinity. Move the timer null check inside the UI callback in StartRotationAnimation so two concurrent starts cannot both create a timer, and drop the unsynchronized _rotationTimer read in StopRotationAnimation that could stale-read null and leave the animation running. - Dispose: DispatcherTimer has thread affinity, so Stop() from another thread threw and left the timer live during shutdown. Route cleanup through the UI thread. - MainViewModel: the monitor loops skipped their WaitOne when IsBusy was set, turning both into busy-spins that pinned a core for the duration of an optimization. Run the delay unconditionally. Capture the CancellationToken once, and stop disposing the CancellationTokenSource while loops still hold its token, which threw ObjectDisposedException from the loop condition outside the try block. - ViewModel: mark _isBusy volatile. It is written on the UI thread and polled by the background loops. - MainWindow: replace Thread.Sleep(1000) on the UI thread with a DispatcherTimer so the grace period no longer freezes the window. - App: add a shutdown watchdog that force-exits if a graceful shutdown stalls, and marshal Application.Shutdown to the UI thread. Adds three regression tests covering concurrent Update calls, Update racing Dispose, and Loading from a background thread. --- src/App.xaml.cs | 60 ++++- src/Service/NotificationService.cs | 377 +++++++++++++++++------------ src/Test/ServiceTests.cs | 109 ++++++++- src/View/Window/MainWindow.xaml.cs | 18 +- src/ViewModel/Base/ViewModel.cs | 6 +- src/ViewModel/MainViewModel.cs | 67 +++-- 6 files changed, 444 insertions(+), 193 deletions(-) diff --git a/src/App.xaml.cs b/src/App.xaml.cs index 8a26a165..61ad1391 100644 --- a/src/App.xaml.cs +++ b/src/App.xaml.cs @@ -23,11 +23,14 @@ public partial class App : IDisposable { #region Fields + private const int ShutdownWatchdogTimeoutMs = 8000; private static bool _isRunning; private static Mutex _mutex; private static NotifyIcon _notifyIcon; private static readonly List _notifications = new List(); private static readonly object _showHidelock = new object(); + private static System.Threading.Timer _shutdownWatchdog; + private static readonly object _shutdownWatchdogLock = new object(); #endregion @@ -901,9 +904,26 @@ public static void Shutdown(bool force = false) try { if (force) + { + Environment.Exit(Constants.Windows.SystemErrorCode.ErrorSuccess); + return; + } + + ArmShutdownWatchdog(); + + var current = Current; + + if (current == null || current.Dispatcher == null) + { Environment.Exit(Constants.Windows.SystemErrorCode.ErrorSuccess); + return; + } + + // Application.Shutdown has to run on the UI thread + if (current.Dispatcher.CheckAccess()) + current.Shutdown(); else - Current.Shutdown(); + current.Dispatcher.BeginInvoke((Action)(() => current.Shutdown())); } catch { @@ -911,6 +931,44 @@ public static void Shutdown(bool force = false) } } + /// + /// Starts a timer that force-exits if a requested graceful shutdown does not complete. + /// + /// + /// Runs on a pool thread so it stays alive even when the UI thread is stuck. Without + /// it, a shutdown that stalls leaves a process only Task Manager can end. + /// + private static void ArmShutdownWatchdog() + { + try + { + lock (_shutdownWatchdogLock) + { + if (_shutdownWatchdog != null) + return; + + _shutdownWatchdog = new System.Threading.Timer(_ => + { + try + { + Logger.Warning("Graceful shutdown did not complete in time. Forcing exit."); + Logger.Dispose(); + } + catch + { + // ignored + } + + Environment.Exit(Constants.Windows.SystemErrorCode.ErrorSuccess); + }, null, ShutdownWatchdogTimeoutMs, Timeout.Infinite); + } + } + catch + { + // ignored + } + } + #endregion } } diff --git a/src/Service/NotificationService.cs b/src/Service/NotificationService.cs index 78290cb8..5157c681 100644 --- a/src/Service/NotificationService.cs +++ b/src/Service/NotificationService.cs @@ -19,12 +19,33 @@ public class NotificationService : INotificationService { #region Fields - private int _currentRotationAngle; + private volatile int _currentRotationAngle; private Icon _currentIcon; - private bool _disposed; + private volatile bool _disposed; private readonly Icon _imageIcon; private readonly NotifyIcon _notifyIcon; private readonly object _disposeLock = new object(); + + /// + /// Serializes GDI reads of the shared , which the UI thread + /// (rotation tick) and the background monitor thread can both reach at once. + /// System.Drawing.Icon is not thread safe. Never held across a dispatcher call. + /// + private readonly object _iconRenderLock = new object(); + + /// + /// Guards the swap. Normally that swap is serialized by + /// running on the UI thread, but when no dispatcher exists the work runs inline on the + /// calling thread, so concurrent callers could otherwise both dispose the same icon. + /// Only ever held for a field assignment; never across a dispatcher call. + /// + private readonly object _currentIconLock = new object(); + + /// + /// Owned by the UI thread. Only ever read or written inside a dispatcher callback, + /// which makes the UI thread the single point of truth and removes the need to hold + /// across a dispatcher call to keep it consistent. + /// private DispatcherTimer _rotationTimer; #endregion @@ -106,19 +127,9 @@ protected virtual void Dispose(bool disposing) _disposed = true; } - try - { - if (_rotationTimer != null) - { - _rotationTimer.Stop(); - _rotationTimer.Tick -= OnRotationTimerTick; - _rotationTimer = null; - } - } - catch (Exception ex) - { - Logger.Debug(ex); - } + // DispatcherTimer has thread affinity: Stop() from another thread throws, which + // previously left the timer running and kept firing ticks during shutdown. + InvokeOnUi(CleanupRotationTimer); try { @@ -135,10 +146,13 @@ protected virtual void Dispose(bool disposing) try { - if (_currentIcon != null && _currentIcon != _imageIcon) + lock (_currentIconLock) { - _currentIcon.Dispose(); - _currentIcon = null; + if (_currentIcon != null && _currentIcon != _imageIcon) + { + _currentIcon.Dispose(); + _currentIcon = null; + } } } catch (Exception ex) @@ -172,6 +186,42 @@ protected virtual void Dispose(bool disposing) #region Methods + /// + /// Runs an action on the UI thread without ever blocking the calling thread. + /// + /// + /// Must stay non-blocking. This used to be a blocking Dispatcher.Invoke reached + /// from on a background thread that held , + /// while the UI thread took that same lock in . Each + /// side then waited on the other (AB-BA), which froze the window and left a process that + /// only Task Manager could end. Both halves of that cycle are now gone, and using + /// BeginInvoke here is what keeps it from being reintroduced by a caller that + /// holds a lock. + /// + /// The action to run on the UI thread. + private static void InvokeOnUi(Action action) + { + if (action == null) + return; + + try + { + var application = WpfApplication.Current; + var dispatcher = application == null ? null : application.Dispatcher; + + // No dispatcher means there is no UI thread to marshal to, so running inline is + // both correct and necessary: returning here would silently drop the update. + if (dispatcher == null || dispatcher.CheckAccess()) + action(); + else + dispatcher.BeginInvoke(action); + } + catch (Exception ex) + { + Logger.Debug(ex); + } + } + /// /// Cleans up the rotation timer resources and resets the rotation angle /// @@ -357,34 +407,39 @@ private Icon GetRotatedIcon(Icon icon, float angle) try { - using (var image = icon.ToBitmap()) - using (var rotatedImage = new Bitmap(image.Width, image.Height)) - using (var graphics = Graphics.FromImage(rotatedImage)) + // icon is the shared _imageIcon; ToBitmap is a GDI read that must not run + // concurrently from the UI thread and the monitor thread. + lock (_iconRenderLock) { - // Configure graphics quality - graphics.InterpolationMode = InterpolationMode.HighQualityBicubic; - graphics.PixelOffsetMode = PixelOffsetMode.HighQuality; - graphics.SmoothingMode = SmoothingMode.HighQuality; + using (var image = icon.ToBitmap()) + using (var rotatedImage = new Bitmap(image.Width, image.Height)) + using (var graphics = Graphics.FromImage(rotatedImage)) + { + // Configure graphics quality + graphics.InterpolationMode = InterpolationMode.HighQualityBicubic; + graphics.PixelOffsetMode = PixelOffsetMode.HighQuality; + graphics.SmoothingMode = SmoothingMode.HighQuality; - // Rotate around center point - var centerX = image.Width / 2f; - var centerY = image.Height / 2f; + // Rotate around center point + var centerX = image.Width / 2f; + var centerY = image.Height / 2f; - graphics.TranslateTransform(centerX, centerY); - graphics.RotateTransform(angle); - graphics.TranslateTransform(-centerX, -centerY); + graphics.TranslateTransform(centerX, centerY); + graphics.RotateTransform(angle); + graphics.TranslateTransform(-centerX, -centerY); - graphics.DrawImage(image, new Point(0, 0)); + graphics.DrawImage(image, new Point(0, 0)); - var handle = rotatedImage.GetHicon(); + var handle = rotatedImage.GetHicon(); - using (var tempIcon = Icon.FromHandle(handle)) - { - var clonedIcon = (Icon)tempIcon.Clone(); + using (var tempIcon = Icon.FromHandle(handle)) + { + var clonedIcon = (Icon)tempIcon.Clone(); - NativeMethods.DestroyIcon(handle); + NativeMethods.DestroyIcon(handle); - return clonedIcon; + return clonedIcon; + } } } } @@ -476,16 +531,24 @@ private Brush GetTextBrush(Memory memory, bool isOptimizing) /// if set to true shows loading cursor and disables menu public void Loading(bool running) { - if (WpfApplication.Current == null || WpfApplication.Current.Dispatcher == null) - return; - - // Multi-threading trick - WpfApplication.Current.Dispatcher.Invoke((Action)delegate + // Non-blocking: this is called from background threads that hold locks (the + // optimization path sets IsBusy while holding the view model lock). + InvokeOnUi(() => { - Mouse.OverrideCursor = running ? Cursors.Wait : null; + try + { + if (_disposed) + return; - if (_notifyIcon.ContextMenuStrip != null) - _notifyIcon.ContextMenuStrip.Enabled = !running; + Mouse.OverrideCursor = running ? Cursors.Wait : null; + + if (_notifyIcon != null && _notifyIcon.ContextMenuStrip != null) + _notifyIcon.ContextMenuStrip.Enabled = !running; + } + catch (Exception ex) + { + Logger.Debug(ex); + } }); } @@ -501,92 +564,48 @@ public void Notify(string message, string title = null, int timeout = 5, Enums.I if (_notifyIcon == null) return; - try - { - _notifyIcon.Visible = false; - _notifyIcon.Visible = true; - - _notifyIcon.ShowBalloonTip(timeout * 1000, title, message, (ToolTipIcon)icon); - } - catch (Exception ex) + // Marshalled: the optimization path calls this from a background thread, and + // NotifyIcon.Visible / ShowBalloonTip must run on the thread that owns the icon. + InvokeOnUi(() => { - Logger.Debug(ex); - } - } - - /// - /// Handles the rotation timer tick event to animate the icon rotation - /// - /// The event sender - /// The event arguments - private void OnRotationTimerTick(object sender, EventArgs e) - { - lock (_disposeLock) - { - if (_disposed) - return; - try { - _currentRotationAngle = (_currentRotationAngle + 90) % 360; - - var newIcon = GetRotatedIcon(_imageIcon, _currentRotationAngle); - var oldIcon = _currentIcon; + if (_disposed || _notifyIcon == null) + return; - _notifyIcon.Icon = newIcon; - _currentIcon = newIcon; + _notifyIcon.Visible = false; + _notifyIcon.Visible = true; - if (oldIcon != null && oldIcon != _imageIcon && oldIcon != newIcon) - { - try - { - oldIcon.Dispose(); - } - catch - { - // ignored - } - } - } - catch (ObjectDisposedException) - { - // Already disposed, ignore + _notifyIcon.ShowBalloonTip(timeout * 1000, title, message, (ToolTipIcon)icon); } catch (Exception ex) { Logger.Debug(ex); } - } + }); } /// - /// Starts the icon rotation animation for the optimization state + /// Handles the rotation timer tick event to animate the icon rotation /// - private void StartRotationAnimation() + /// The event sender + /// The event arguments + private void OnRotationTimerTick(object sender, EventArgs e) { - if (_rotationTimer != null) - return; - - if (WpfApplication.Current == null || WpfApplication.Current.Dispatcher == null) + // Already on the UI thread. Taking _disposeLock here is what let a background + // thread inside Update block the UI thread, so the lock is deliberately absent. + if (_disposed) return; try { - WpfApplication.Current.Dispatcher.Invoke((Action)delegate - { - try - { - _currentRotationAngle = 0; + _currentRotationAngle = (_currentRotationAngle + 90) % 360; - _rotationTimer = new DispatcherTimer { Interval = TimeSpan.FromMilliseconds(200) }; - _rotationTimer.Tick += OnRotationTimerTick; - _rotationTimer.Start(); - } - catch (Exception ex) - { - Logger.Debug(ex); - } - }); + ApplyIcon(_notifyIcon.Text, GetRotatedIcon(_imageIcon, _currentRotationAngle)); + } + catch (ObjectDisposedException) + { + // Already disposed, ignore } catch (Exception ex) { @@ -595,30 +614,41 @@ private void StartRotationAnimation() } /// - /// Stops the icon rotation animation + /// Starts the icon rotation animation for the optimization state /// - private void StopRotationAnimation() + private void StartRotationAnimation() { - if (_rotationTimer == null) - return; - - try + InvokeOnUi(() => { - if (WpfApplication.Current == null || WpfApplication.Current.Dispatcher == null) + try { - CleanupRotationTimer(); - return; - } + // Checked on the UI thread so two concurrent starts cannot both create a timer + if (_rotationTimer != null || _disposed) + return; + + _currentRotationAngle = 0; - WpfApplication.Current.Dispatcher.Invoke((Action)delegate + _rotationTimer = new DispatcherTimer { Interval = TimeSpan.FromMilliseconds(200) }; + _rotationTimer.Tick += OnRotationTimerTick; + _rotationTimer.Start(); + } + catch (Exception ex) { - CleanupRotationTimer(); - }); - } - catch (Exception ex) - { - Logger.Debug(ex); - } + Logger.Debug(ex); + } + }); + } + + /// + /// Stops the icon rotation animation + /// + private void StopRotationAnimation() + { + // No _rotationTimer check here on purpose. The field is owned by the UI thread and + // is not volatile, so a caller on another thread can read a stale null and skip the + // cleanup, leaving the animation running. CleanupRotationTimer does the null check + // on the UI thread, where the read is valid. + InvokeOnUi(CleanupRotationTimer); } /// @@ -632,41 +662,82 @@ public void Update(Memory memory, bool isOptimizing = false) if (memory == null) throw new ArgumentNullException("memory"); - lock (_disposeLock) + if (_disposed || _notifyIcon == null) + return; + + string text; + Icon newIcon; + + // Rendering happens on the calling thread, outside any lock, so a slow GDI draw + // never stalls the UI thread and never blocks a lock the UI thread needs. + try + { + text = GetText(memory, isOptimizing); + newIcon = GetIcon(memory, isOptimizing); + } + catch (Exception ex) + { + Logger.Debug(ex); + return; + } + + // NotifyIcon has thread affinity: its window was created on the UI thread, so + // assigning Text/Icon from a pool thread can hang on the shell notification call. + InvokeOnUi(() => ApplyIcon(text, newIcon)); + } + + /// + /// Assigns the tray icon text and image, then releases the icon it replaced. + /// + /// + /// Normally invoked on the UI thread. When no dispatcher exists it runs inline on the + /// calling thread instead, so the icon swap is guarded by + /// rather than relying on UI-thread serialization. + /// + /// The tooltip text. + /// The icon to display. + private void ApplyIcon(string text, Icon newIcon) + { + try { if (_disposed || _notifyIcon == null) + { + if (newIcon != null && newIcon != _imageIcon) + newIcon.Dispose(); + return; + } - try - { - _notifyIcon.Text = GetText(memory, isOptimizing); + Icon oldIcon; - var newIcon = GetIcon(memory, isOptimizing); - var oldIcon = _currentIcon; + lock (_currentIconLock) + { + oldIcon = _currentIcon; + _notifyIcon.Text = text; _notifyIcon.Icon = newIcon; _currentIcon = newIcon; + } - if (oldIcon != null && oldIcon != _imageIcon && oldIcon != newIcon) + if (oldIcon != null && oldIcon != _imageIcon && oldIcon != newIcon) + { + try { - try - { - oldIcon.Dispose(); - } - catch - { - // ignored - } + oldIcon.Dispose(); + } + catch + { + // ignored } } - catch (ObjectDisposedException) - { - // Already disposed, ignore - } - catch (Exception ex) - { - Logger.Debug(ex); - } + } + catch (ObjectDisposedException) + { + // Already disposed, ignore + } + catch (Exception ex) + { + Logger.Debug(ex); } } diff --git a/src/Test/ServiceTests.cs b/src/Test/ServiceTests.cs index 57486f6d..9a1e8a56 100644 --- a/src/Test/ServiceTests.cs +++ b/src/Test/ServiceTests.cs @@ -338,7 +338,114 @@ public void Dispose_CanBeCalledMultipleTimes() Assert.DoesNotThrow(() => _notificationService.Dispose()); } - + + // Regression tests for the tray-icon freeze. Update used to hold a lock across a + // blocking dispatcher call while the UI thread took the same lock, which deadlocked + // the window and left a process only Task Manager could end. + + [Test] + public void Update_FromManyThreadsConcurrently_DoesNotDeadlockOrThrow() + { + var memory = new Memory(Mocker.CreateMemoryStatusEx()); + var showMemoryUsage = Settings.TrayIconShowMemoryUsage; + + try + { + // false routes through the rotation-animation path, which is the default + // and the one that used to block on the dispatcher + Settings.TrayIconShowMemoryUsage = false; + + var errors = new System.Collections.Concurrent.ConcurrentQueue(); + var threads = new System.Collections.Generic.List(); + + for (var i = 0; i < 8; i++) + { + var optimizing = i % 2 == 0; + var thread = new System.Threading.Thread(() => + { + try + { + for (var j = 0; j < 25; j++) + _notificationService.Update(memory, optimizing); + } + catch (Exception e) + { + errors.Enqueue(e); + } + }); + + thread.IsBackground = true; + threads.Add(thread); + } + + foreach (var thread in threads) + thread.Start(); + + foreach (var thread in threads) + Assert.IsTrue(thread.Join(TimeSpan.FromSeconds(30)), "Update blocked; a caller is waiting on a lock or the dispatcher."); + + Assert.IsEmpty(errors.ToArray()); + + // An unsynchronized icon swap can leave the icon that is still assigned to + // the tray disposed. Touching Handle on a disposed Icon throws. + var assigned = _notifyIcon.Icon; + + if (assigned != null) + Assert.DoesNotThrow(() => { var unused = assigned.Handle; }, "The icon assigned to the tray was disposed by a concurrent update."); + } + finally + { + Settings.TrayIconShowMemoryUsage = showMemoryUsage; + } + } + + [Test] + public void Update_WhileDisposing_DoesNotThrow() + { + var memory = new Memory(Mocker.CreateMemoryStatusEx()); + var errors = new System.Collections.Concurrent.ConcurrentQueue(); + + var updater = new System.Threading.Thread(() => + { + try + { + for (var i = 0; i < 200; i++) + _notificationService.Update(memory, i % 2 == 0); + } + catch (Exception e) + { + errors.Enqueue(e); + } + }); + + updater.IsBackground = true; + updater.Start(); + + Assert.DoesNotThrow(() => _notificationService.Dispose()); + Assert.IsTrue(updater.Join(TimeSpan.FromSeconds(30)), "Update did not finish while the service was disposed."); + Assert.IsEmpty(errors.ToArray()); + } + + [Test] + public void Loading_FromBackgroundThread_DoesNotBlock() + { + var completed = false; + + var worker = new System.Threading.Thread(() => + { + _notificationService.Loading(true); + _notificationService.Loading(false); + completed = true; + }); + + worker.IsBackground = true; + worker.Start(); + + Assert.IsTrue(worker.Join(TimeSpan.FromSeconds(10)), "Loading blocked the calling thread."); + Assert.IsTrue(completed); + } + + public void Dispose() { // Ensure teardown logic runs when used as IDisposable diff --git a/src/View/Window/MainWindow.xaml.cs b/src/View/Window/MainWindow.xaml.cs index 24f58b93..d486c92a 100644 --- a/src/View/Window/MainWindow.xaml.cs +++ b/src/View/Window/MainWindow.xaml.cs @@ -1,6 +1,5 @@ using System; using System.Linq; -using System.Threading; using System.Windows; using System.Windows.Controls; using System.Windows.Controls.Primitives; @@ -213,8 +212,21 @@ private void OnOptimizeCommandCompleted() } else { - Thread.Sleep(1000); - App.Shutdown(); + // This runs on the UI thread, so sleeping here froze the window for a full + // second after every optimization. A timer gives the same grace period + // (letting the tray notification appear) while the UI keeps pumping. + var shutdownDelay = new System.Windows.Threading.DispatcherTimer + { + Interval = TimeSpan.FromMilliseconds(1000) + }; + + shutdownDelay.Tick += (sender, args) => + { + shutdownDelay.Stop(); + App.Shutdown(); + }; + + shutdownDelay.Start(); } } else diff --git a/src/ViewModel/Base/ViewModel.cs b/src/ViewModel/Base/ViewModel.cs index 1d5400d7..fbe520c9 100644 --- a/src/ViewModel/Base/ViewModel.cs +++ b/src/ViewModel/Base/ViewModel.cs @@ -10,7 +10,11 @@ public abstract class ViewModel : ObservableObject { #region Fields - private bool _isBusy; + /// + /// Written on the UI thread, polled by the background monitor loops, so the write has + /// to be visible to them without a lock. + /// + private volatile bool _isBusy; #endregion diff --git a/src/ViewModel/MainViewModel.cs b/src/ViewModel/MainViewModel.cs index 84feb147..29bc117a 100644 --- a/src/ViewModel/MainViewModel.cs +++ b/src/ViewModel/MainViewModel.cs @@ -1476,23 +1476,12 @@ protected virtual void Dispose(bool disposing) // ignored } - try - { - _cancellationTokenSource.Token.WaitHandle.WaitOne(100); - } - catch - { - // ignored - } - - try - { - _cancellationTokenSource.Dispose(); - } - catch - { - // ignored - } + // The source is deliberately not disposed. The monitor loops hold its token + // and wait on its handle; disposing it out from under a loop that has not + // observed the cancellation yet throws ObjectDisposedException from the + // loop condition, which is outside the try block and would take down the + // pool thread. Cancel() is enough to end the loops, and the handle is + // reclaimed when the process exits moments later. } if (_hotKeyService != null) @@ -1613,18 +1602,24 @@ private void AddProcessToExclusionList(string process) /// private void MonitorApp() { - while (!_cancellationTokenSource.Token.IsCancellationRequested) + // Captured once: the token is read after Dispose may have disposed its source, + // and CancellationToken stays usable while CancellationTokenSource does not. + var token = _cancellationTokenSource.Token; + + while (!token.IsCancellationRequested) { try { + // Delay first, unconditionally. Skipping this wait when IsBusy was set + // turned the loop into a busy-spin that pinned a CPU core for as long as + // the app stayed busy, which is most of an optimization run. + if (token.WaitHandle.WaitOne(60000)) + break; + // Check if it's busy if (IsBusy) continue; - // Delay - if (_cancellationTokenSource.Token.WaitHandle.WaitOne(60000)) - break; - // Update app Updater.Update(); @@ -1700,27 +1695,31 @@ private void MonitorComputer() // App priority App.SetPriority(Settings.RunOnPriority); - while (!_cancellationTokenSource.Token.IsCancellationRequested) + // Captured once, for the same reason as in MonitorApp + var token = _cancellationTokenSource.Token; + + while (!token.IsCancellationRequested) { try { - // Check if it's busy - if (IsBusy) - continue; - - lock (_lockObject) + // Check if it's busy. The delay below always runs, so a busy app makes this + // loop idle rather than spin. + if (!IsBusy) { - // Update memory info - Computer.Memory = _computerService.Memory; + lock (_lockObject) + { + // Update memory info + Computer.Memory = _computerService.Memory; - RaisePropertyChanged(() => Computer); - RaisePropertyChanged(() => VirtualMemoryHeader); + RaisePropertyChanged(() => Computer); + RaisePropertyChanged(() => VirtualMemoryHeader); - NotificationService.Update(Computer.Memory, IsOptimizationRunning); + NotificationService.Update(Computer.Memory, IsOptimizationRunning); + } } // Delay - if (_cancellationTokenSource.Token.WaitHandle.WaitOne(5000)) + if (token.WaitHandle.WaitOne(5000)) break; } catch (Exception e) From 56a471b09c382c2773dfb934b0590cc3dd7cba2c Mon Sep 17 00:00:00 2001 From: shabhui <> Date: Tue, 1 Sep 2026 16:55:25 +0800 Subject: [PATCH 2/3] fix: serialize GetMemoryUsageIcon on the icon render lock Icon rendering used to be serialized incidentally, because Update held _disposeLock for its whole duration. Removing that lock to break the deadlock also removed the serialization, and the replacement _iconRenderLock only covered GetRotatedIcon. That left a half-covered invariant: rendering looks locked, but only one of the two paths is. GetMemoryUsageIcon is reachable concurrently from the monitor threads and creates its GDI objects per call, so it is safe as written today. It is covered anyway, so that caching a Font, Brush or StringFormat in a field later cannot silently introduce a data race. --- src/Service/NotificationService.cs | 88 +++++++++++++++++------------- 1 file changed, 50 insertions(+), 38 deletions(-) diff --git a/src/Service/NotificationService.cs b/src/Service/NotificationService.cs index 5157c681..7ba777d2 100644 --- a/src/Service/NotificationService.cs +++ b/src/Service/NotificationService.cs @@ -27,10 +27,19 @@ public class NotificationService : INotificationService private readonly object _disposeLock = new object(); /// - /// Serializes GDI reads of the shared , which the UI thread - /// (rotation tick) and the background monitor thread can both reach at once. - /// System.Drawing.Icon is not thread safe. Never held across a dispatcher call. + /// Serializes icon rendering. Held by both and + /// , which the UI thread (rotation tick) and the + /// background monitor threads can reach at the same time. /// + /// + /// Rendering used to be serialized incidentally, because held + /// for its whole duration. That lock had to go to break the + /// deadlock described on , so this one restores the guarantee + /// explicitly. It covers both render paths on purpose: GDI+ types are not thread safe, + /// and a partially covered invariant invites a future change to cache a Font, Brush or + /// StringFormat in a field and reintroduce a data race. Never held across a dispatcher + /// call. + /// private readonly object _iconRenderLock = new object(); /// @@ -341,50 +350,53 @@ private Icon GetMemoryUsageIcon(Memory memory, bool isOptimizing) { try { - using (var image = new Bitmap(16, 16)) - using (var graphics = Graphics.FromImage(image)) - using (var font = new Font("Consolas", 14F, FontStyle.Regular, GraphicsUnit.Pixel)) - using (var format = new StringFormat()) - using (var backgroundBrush = GetBackgroundBrush(memory, isOptimizing)) - using (var textBrush = GetTextBrush(memory, isOptimizing)) + lock (_iconRenderLock) { - // Configure format - format.Alignment = StringAlignment.Center; - format.LineAlignment = StringAlignment.Center; - - // Configure graphics quality - graphics.InterpolationMode = InterpolationMode.HighQualityBicubic; - graphics.PixelOffsetMode = PixelOffsetMode.HighQuality; - graphics.SmoothingMode = SmoothingMode.AntiAlias; - graphics.TextRenderingHint = TextRenderingHint.SingleBitPerPixelGridFit; - - // Draw background - if (!Settings.TrayIconUseTransparentBackground) + using (var image = new Bitmap(16, 16)) + using (var graphics = Graphics.FromImage(image)) + using (var font = new Font("Consolas", 14F, FontStyle.Regular, GraphicsUnit.Pixel)) + using (var format = new StringFormat()) + using (var backgroundBrush = GetBackgroundBrush(memory, isOptimizing)) + using (var textBrush = GetTextBrush(memory, isOptimizing)) { - using (var path = new GraphicsPath()) - { - path.AddArc(0, 0, 10, 10, 180, 90); - path.AddArc(5, 0, 10, 10, 270, 90); - path.AddArc(5, 5, 10, 10, 0, 90); - path.AddArc(0, 5, 10, 10, 90, 90); - path.CloseFigure(); + // Configure format + format.Alignment = StringAlignment.Center; + format.LineAlignment = StringAlignment.Center; + + // Configure graphics quality + graphics.InterpolationMode = InterpolationMode.HighQualityBicubic; + graphics.PixelOffsetMode = PixelOffsetMode.HighQuality; + graphics.SmoothingMode = SmoothingMode.AntiAlias; + graphics.TextRenderingHint = TextRenderingHint.SingleBitPerPixelGridFit; - graphics.FillPath(backgroundBrush, path); + // Draw background + if (!Settings.TrayIconUseTransparentBackground) + { + using (var path = new GraphicsPath()) + { + path.AddArc(0, 0, 10, 10, 180, 90); + path.AddArc(5, 0, 10, 10, 270, 90); + path.AddArc(5, 5, 10, 10, 0, 90); + path.AddArc(0, 5, 10, 10, 90, 90); + path.CloseFigure(); + + graphics.FillPath(backgroundBrush, path); + } } - } - // Draw text - graphics.DrawString(string.Format(CultureInfo.InvariantCulture, "{0:00}", memory.Physical.Used.Percentage == 100 ? 99 : memory.Physical.Used.Percentage), font, textBrush, 8F, 9F, format); + // Draw text + graphics.DrawString(string.Format(CultureInfo.InvariantCulture, "{0:00}", memory.Physical.Used.Percentage == 100 ? 99 : memory.Physical.Used.Percentage), font, textBrush, 8F, 9F, format); - var handle = image.GetHicon(); + var handle = image.GetHicon(); - using (var icon = Icon.FromHandle(handle)) - { - var clonedIcon = (Icon)icon.Clone(); + using (var icon = Icon.FromHandle(handle)) + { + var clonedIcon = (Icon)icon.Clone(); - NativeMethods.DestroyIcon(handle); + NativeMethods.DestroyIcon(handle); - return clonedIcon; + return clonedIcon; + } } } } From 026771dbc81bd34a6f63738920ee6f3142d7f6e5 Mon Sep 17 00:00:00 2001 From: shabhui <> Date: Tue, 1 Sep 2026 17:14:28 +0800 Subject: [PATCH 3/3] fix: dispose CancellationTokenSource to satisfy CA2213 The token-capture fix in RC1 made disposal safe: loops capture the token once into a local at entry, and CancellationToken.IsCancellationRequested stays usable after the source is disposed (it's a struct). Before RC1, the loop read _cancellationTokenSource.Token fresh in the while condition, which could throw ObjectDisposedException after disposal on a pool thread outside the try block. With the capture, disposing is safe and clears the CA2213 analyzer warning that fails CI builds with /warnaserror+. --- src/ViewModel/MainViewModel.cs | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/src/ViewModel/MainViewModel.cs b/src/ViewModel/MainViewModel.cs index 29bc117a..adb5030b 100644 --- a/src/ViewModel/MainViewModel.cs +++ b/src/ViewModel/MainViewModel.cs @@ -1476,12 +1476,30 @@ protected virtual void Dispose(bool disposing) // ignored } - // The source is deliberately not disposed. The monitor loops hold its token - // and wait on its handle; disposing it out from under a loop that has not - // observed the cancellation yet throws ObjectDisposedException from the - // loop condition, which is outside the try block and would take down the - // pool thread. Cancel() is enough to end the loops, and the handle is - // reclaimed when the process exits moments later. + // RC1 captured the token once into a local at loop entry, which makes + // IsCancellationRequested safe to read after the source is disposed + // (the token is a struct, doesn't throw). Before RC1, the loop read + // _cancellationTokenSource.Token fresh in the while condition, which + // could throw ObjectDisposedException on a pool thread after disposal. + // With the local capture, disposing is safe. Wait briefly for loops + // to observe cancellation, then dispose. + try + { + _cancellationTokenSource.Token.WaitHandle.WaitOne(100); + } + catch + { + // ignored + } + + try + { + _cancellationTokenSource.Dispose(); + } + catch + { + // ignored + } } if (_hotKeyService != null)