From 3a8e74f37ddba7bd06db05ad6ed68c50a670d7d1 Mon Sep 17 00:00:00 2001 From: Kool <79905997+eakkawut@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:44:07 +0700 Subject: [PATCH 1/7] fix: repair data-loss and correctness defects found by audit Four defects that were visible to users, the first of which could destroy saved configuration: - PersistentProcessRuleJsonStore cached the result of a failed read. A transient lock from antivirus, a backup agent or a sync client left an empty rule set pinned for the rest of the session, and the next save then overwrote every saved rule with it. A failed read is no longer cached, and the unreadable file is preserved once as *.unreadable so the data stays recoverable even if a later save succeeds. - SettingsViewModel discarded the user's unsaved edits whenever any background write touched settings, for example the startup update check recording LastUpdateCheckUtc. While edits are pending only the saved snapshot is re-based now. - The quiet-hours comparison in SmartNotificationService was inverted relative to its own comments: the shipped 22:00-08:00 default never suppressed anything, and a daytime window suppressed everything around the clock. An explicit Do Not Disturb request was also overridden by the schedule. Extracted IsWithinQuietHours with correct wrap-around handling, gave the explicit request precedence, and made auto-expiry raise DoNotDisturbChanged. - Default hotkeys sat behind a null check on settings.KeyboardShortcuts, which is never null because the model initialises an empty list and CopyFrom keeps it non-null, so a fresh install registered no global hotkeys at all. The condition now tests Count. Also fixes the notification retry ladder, which was unreachable behind a hardcoded success flag, and the throttle history race between caller threads, the processing timer and the hourly cleanup timer. SettingsViewModel gains its OnDispose override here rather than in the disposal commit, to keep the file's changes in one place. --- Services/KeyboardShortcutService.cs | 84 +++++--- Services/PersistentProcessRuleJsonStore.cs | 39 +++- Services/SmartNotificationService.cs | 197 +++++++++++------- .../KeyboardShortcutDefaultsTests.cs | 95 +++++++++ .../PersistentProcessRuleJsonStoreTests.cs | 48 +++++ .../SmartNotificationQuietHoursTests.cs | 46 ++++ ViewModels/SettingsViewModel.cs | 25 ++- 7 files changed, 425 insertions(+), 109 deletions(-) create mode 100644 Tests/ThreadPilot.Core.Tests/KeyboardShortcutDefaultsTests.cs create mode 100644 Tests/ThreadPilot.Core.Tests/SmartNotificationQuietHoursTests.cs diff --git a/Services/KeyboardShortcutService.cs b/Services/KeyboardShortcutService.cs index 44f0dd5..711a62c 100644 --- a/Services/KeyboardShortcutService.cs +++ b/Services/KeyboardShortcutService.cs @@ -53,6 +53,14 @@ public async Task RegisterShortcutAsync(string actionName, Key key, Modifi return false; } + if (this.windowHandle == IntPtr.Zero) + { + this.logger.LogWarning( + "Skipped registering shortcut for action {Action} because no window handle is available yet", + actionName); + return false; + } + // Check if shortcut is already registered if (await this.IsShortcutRegisteredAsync(key, modifiers)) { @@ -106,7 +114,10 @@ public async Task RegisterShortcutAsync(string actionName, Key key, Modifi } } - public async Task UnregisterShortcutAsync(string actionName) + public Task UnregisterShortcutAsync(string actionName) => + Task.FromResult(this.UnregisterShortcut(actionName)); + + private bool UnregisterShortcut(string actionName) { try { @@ -123,23 +134,23 @@ public async Task UnregisterShortcutAsync(string actionName) } // Unregister from Windows API - if (UnregisterHotKey(this.windowHandle, hotkeyId)) - { - this.registeredShortcuts.Remove(actionName); - this.hotkeyIdToAction.Remove(hotkeyId); + var unregistered = UnregisterHotKey(this.windowHandle, hotkeyId); + + this.registeredShortcuts.Remove(actionName); + this.hotkeyIdToAction.Remove(hotkeyId); + if (unregistered) + { this.logger.LogInformation( "Unregistered shortcut {Shortcut} for action {Action}", shortcut.ToString(), actionName); return true; } - else - { - this.logger.LogError( - "Failed to unregister shortcut {Shortcut} for action {Action}", - shortcut.ToString(), actionName); - return false; - } + + this.logger.LogError( + "Failed to unregister shortcut {Shortcut} for action {Action}", + shortcut.ToString(), actionName); + return false; } catch (Exception ex) { @@ -169,10 +180,11 @@ public async Task LoadShortcutsFromSettingsAsync() { try { - var settings = this.settingsService.Settings; - if (settings.KeyboardShortcuts != null) + var configuredShortcuts = this.settingsService.Settings.KeyboardShortcuts; + + if (configuredShortcuts != null && configuredShortcuts.Count > 0) { - foreach (var shortcutSetting in settings.KeyboardShortcuts) + foreach (var shortcutSetting in configuredShortcuts) { if (shortcutSetting.IsEnabled) { @@ -210,12 +222,17 @@ public async Task SaveShortcutsToSettingsAsync() } } - public async Task ClearAllShortcutsAsync() + public Task ClearAllShortcutsAsync() + { + this.ClearAllShortcuts(); + return Task.CompletedTask; + } + + private void ClearAllShortcuts() { - var actions = this.registeredShortcuts.Keys.ToList(); - foreach (var action in actions) + foreach (var action in this.registeredShortcuts.Keys.ToList()) { - await this.UnregisterShortcutAsync(action); + this.UnregisterShortcut(action); } } @@ -264,6 +281,17 @@ public Dictionary GetDefaultShortcuts() public void SetWindowHandle(IntPtr windowHandle) { + if (this.windowHandle == windowHandle && this.hwndSource != null) + { + return; + } + + if (this.hwndSource != null) + { + this.hwndSource.RemoveHook(this.WndProc); + this.hwndSource = null; + } + this.windowHandle = windowHandle; // Set up message hook for hotkey messages @@ -350,17 +378,19 @@ private string GetActionDescription(string actionName) public void Dispose() { - if (!this.disposed) + if (this.disposed) { - this.ClearAllShortcutsAsync().Wait(); + return; + } - if (this.hwndSource != null) - { - this.hwndSource.RemoveHook(this.WndProc); - this.hwndSource = null; - } + this.disposed = true; - this.disposed = true; + this.ClearAllShortcuts(); + + if (this.hwndSource != null) + { + this.hwndSource.RemoveHook(this.WndProc); + this.hwndSource = null; } } } diff --git a/Services/PersistentProcessRuleJsonStore.cs b/Services/PersistentProcessRuleJsonStore.cs index 0811312..aa2f3f9 100644 --- a/Services/PersistentProcessRuleJsonStore.cs +++ b/Services/PersistentProcessRuleJsonStore.cs @@ -19,6 +19,7 @@ public sealed class PersistentProcessRuleJsonStore : IPersistentProcessRuleStore private readonly ILogger? logger; private readonly SemaphoreSlim cacheLock = new(1, 1); private volatile IReadOnlyList? cachedRules; + private bool loadFailed; public PersistentProcessRuleJsonStore(ILogger? logger = null) : this(() => StoragePaths.PersistentRulesFilePath, logger) @@ -61,12 +62,15 @@ public async Task> LoadAsync() var json = await File.ReadAllTextAsync(filePath).ConfigureAwait(false); var rules = JsonSerializer.Deserialize>(json, JsonOptions) ?? []; this.logger?.LogDebug("Loaded {RuleCount} persistent process rules from {FilePath}", rules.Count, filePath); + this.loadFailed = false; return this.cachedRules = rules.ToArray(); } catch (Exception ex) when (ex is JsonException or IOException or UnauthorizedAccessException) { - this.logger?.LogWarning(ex, "Could not load persistent process rules from {FilePath}", filePath); - return this.cachedRules = []; + this.loadFailed = true; + this.TryPreserveUnreadableFile(filePath, ex); + this.logger?.LogWarning(ex, "Could not load persistent process rules from {FilePath}. The rule set will be re-read on the next access.", filePath); + return []; } } finally @@ -84,6 +88,14 @@ public async Task SaveAsync(IReadOnlyList rules) { var filePath = this.filePathProvider(); this.logger?.LogDebug("Saving {RuleCount} persistent process rules to {FilePath}", rules.Count, filePath); + if (this.loadFailed) + { + this.logger?.LogWarning( + "Saving {RuleCount} persistent process rules to {FilePath} after a failed read. A copy of the previous file was preserved next to it.", + rules.Count, + filePath); + } + try { var json = JsonSerializer.Serialize(rules, JsonOptions); @@ -102,5 +114,28 @@ public async Task SaveAsync(IReadOnlyList rules) this.cacheLock.Release(); } } + + private void TryPreserveUnreadableFile(string filePath, Exception readException) + { + var backupPath = filePath + ".unreadable"; + + try + { + if (File.Exists(backupPath)) + { + return; + } + + File.Copy(filePath, backupPath, overwrite: false); + this.logger?.LogWarning( + readException, + "Preserved unreadable persistent process rules file as {BackupPath}", + backupPath); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + this.logger?.LogDebug(ex, "Could not preserve unreadable persistent process rules file {FilePath}", filePath); + } + } } } diff --git a/Services/SmartNotificationService.cs b/Services/SmartNotificationService.cs index 4484357..b0408c6 100644 --- a/Services/SmartNotificationService.cs +++ b/Services/SmartNotificationService.cs @@ -18,13 +18,16 @@ public class SmartNotificationService : ISmartNotificationService, IDisposable private readonly ConcurrentDictionary lastNotificationTimes = new(); private readonly ConcurrentDictionary> notificationHistory = new(); private readonly List sentNotifications = new(); + + private readonly object historyLock = new(); private readonly System.Threading.Timer processingTimer; private readonly System.Threading.Timer cleanupTimer; private readonly SemaphoreSlim processingLock = new(1, 1); private NotificationPreferences preferences = new(); private DateTime? doNotDisturbUntil; - private bool disposed; + + private volatile bool disposed; public event EventHandler? NotificationSent; @@ -41,6 +44,8 @@ public SmartNotificationService( this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); this.baseNotificationService = baseNotificationService ?? throw new ArgumentNullException(nameof(baseNotificationService)); + this.preferences = this.CreateDefaultPreferences(); + // Set up processing timer (process queue every 2 seconds) this.processingTimer = new System.Threading.Timer(this.ProcessQueueCallback, null, TimeSpan.FromSeconds(2), TimeSpan.FromSeconds(2)); @@ -157,16 +162,19 @@ public async Task SendNotificationAsync(string title, string message, return await this.SendNotificationAsync(notification); } - public async Task ScheduleNotificationAsync(SmartNotification notification, DateTime deliveryTime) + public Task ScheduleNotificationAsync(SmartNotification notification, DateTime deliveryTime) { + ArgumentNullException.ThrowIfNull(notification); + notification.ScheduledFor = deliveryTime; - this.scheduledNotifications.TryAdd(notification.Id, notification); + + this.scheduledNotifications[notification.Id] = notification; this.logger.LogDebug( "Scheduled notification {Id} for delivery at {DeliveryTime}", notification.Id, deliveryTime); - return true; + return Task.FromResult(true); } public async Task CancelNotificationAsync(string notificationId) @@ -205,15 +213,21 @@ public async Task> GetNotificationHistoryAsync(TimeSpan? } } - public async Task ClearHistoryAsync() + public Task ClearHistoryAsync() { lock (this.sentNotifications) { this.sentNotifications.Clear(); } - this.notificationHistory.Clear(); + lock (this.historyLock) + { + this.notificationHistory.Clear(); + } + + this.lastNotificationTimes.Clear(); this.logger.LogInformation("Cleared notification history"); + return Task.CompletedTask; } public async Task UpdatePreferencesAsync(NotificationPreferences preferences) @@ -260,25 +274,35 @@ public bool IsDoNotDisturbActive() return false; } - if (this.doNotDisturbUntil.HasValue && DateTime.UtcNow > this.doNotDisturbUntil.Value) + if (this.doNotDisturbUntil.HasValue) { + if (DateTime.UtcNow <= this.doNotDisturbUntil.Value) + { + return true; + } + this.preferences.DoNotDisturbMode = false; this.doNotDisturbUntil = null; + this.DoNotDisturbChanged?.Invoke(this, false); return false; } - // Check time-based DND - var now = DateTime.Now.TimeOfDay; - if (this.preferences.DoNotDisturbStart < this.preferences.DoNotDisturbEnd) - { - // Same day range (e.g., 10 PM to 8 AM next day) - return now >= this.preferences.DoNotDisturbStart || now <= this.preferences.DoNotDisturbEnd; - } - else + return IsWithinQuietHours( + DateTime.Now.TimeOfDay, + this.preferences.DoNotDisturbStart, + this.preferences.DoNotDisturbEnd); + } + + internal static bool IsWithinQuietHours(TimeSpan timeOfDay, TimeSpan start, TimeSpan end) + { + if (start == end) { - // Cross-midnight range (e.g., 10 PM to 8 AM) - return now >= this.preferences.DoNotDisturbStart && now <= this.preferences.DoNotDisturbEnd; + return false; } + + return start < end + ? timeOfDay >= start && timeOfDay <= end + : timeOfDay >= start || timeOfDay <= end; } public async Task> GetStatisticsAsync() @@ -340,7 +364,15 @@ private async Task ProcessQueueCallbackAsync() return; } - await this.processingLock.WaitAsync(); + try + { + await this.processingLock.WaitAsync().ConfigureAwait(false); + } + catch (ObjectDisposedException) + { + return; + } + try { var processedCount = 0; @@ -366,7 +398,13 @@ private async Task ProcessQueueCallbackAsync() } finally { - this.processingLock.Release(); + try + { + this.processingLock.Release(); + } + catch (ObjectDisposedException) + { + } } } @@ -388,41 +426,33 @@ await this.baseNotificationService.ShowNotificationAsync( notification.Message, this.ConvertToNotificationType(notification.Priority)); - // Assume success since no exception was thrown - var success = true; + this.RecordNotificationSent(notification); - if (success) + this.NotificationSent?.Invoke(this, new SmartNotificationEventArgs { - // Record successful delivery - this.RecordNotificationSent(notification); + Notification = notification, + Reason = "Successfully delivered", + }); - this.NotificationSent?.Invoke(this, new SmartNotificationEventArgs - { - Notification = notification, - Reason = "Successfully delivered", - }); - - this.logger.LogDebug("Successfully sent notification: {Title}", notification.Title); - } - else if (notification.RetryCount < notification.MaxRetries) + this.logger.LogDebug("Successfully sent notification: {Title}", notification.Title); + } + catch (Exception ex) + { + if (notification.RetryCount < notification.MaxRetries) { - // Retry failed notification notification.RetryCount++; this.notificationQueue.Enqueue(notification); - this.logger.LogDebug( + this.logger.LogWarning( + ex, "Retrying notification: {Title} (Attempt {Retry}/{Max})", notification.Title, notification.RetryCount, notification.MaxRetries); + return; } - else - { - this.logger.LogWarning( - "Failed to send notification after {MaxRetries} attempts: {Title}", - notification.MaxRetries, notification.Title); - } - } - catch (Exception ex) - { - this.logger.LogError(ex, "Error processing notification: {Title}", notification.Title); + + this.logger.LogError( + ex, + "Failed to send notification after {MaxRetries} attempts: {Title}", + notification.MaxRetries, notification.Title); } } @@ -446,21 +476,20 @@ private bool IsThrottled(SmartNotification notification) } // Check hourly and daily limits - if (!this.notificationHistory.TryGetValue(key, out var history)) - { - history = new List(); - this.notificationHistory[key] = history; - } - - // Clean old entries var oneHourAgo = now.AddHours(-1); var oneDayAgo = now.AddDays(-1); - history.RemoveAll(t => t < oneDayAgo); - var hourlyCount = history.Count(t => t >= oneHourAgo); - var dailyCount = history.Count; + lock (this.historyLock) + { + var history = this.notificationHistory.GetOrAdd(key, _ => new List()); + + history.RemoveAll(t => t < oneDayAgo); + + var hourlyCount = history.Count(t => t >= oneHourAgo); + var dailyCount = history.Count; - return hourlyCount >= config.MaxPerHour || dailyCount >= config.MaxPerDay; + return hourlyCount >= config.MaxPerHour || dailyCount >= config.MaxPerDay; + } } private bool IsDuplicate(SmartNotification notification) @@ -492,12 +521,10 @@ private void RecordNotificationSent(SmartNotification notification) this.lastNotificationTimes[key] = now; - if (!this.notificationHistory.TryGetValue(key, out var history)) + lock (this.historyLock) { - history = new List(); - this.notificationHistory[key] = history; + this.notificationHistory.GetOrAdd(key, _ => new List()).Add(now); } - history.Add(now); lock (this.sentNotifications) { @@ -573,21 +600,36 @@ private async Task CleanupCallbackAsync() // Clean notification history var keysToRemove = new List(); - foreach (var kvp in this.notificationHistory) + lock (this.historyLock) { - kvp.Value.RemoveAll(t => t < cutoff); - if (!kvp.Value.Any()) + foreach (var kvp in this.notificationHistory) { - keysToRemove.Add(kvp.Key); + kvp.Value.RemoveAll(t => t < cutoff); + if (kvp.Value.Count == 0) + { + keysToRemove.Add(kvp.Key); + } + } + + foreach (var key in keysToRemove) + { + this.notificationHistory.TryRemove(key, out _); } } - foreach (var key in keysToRemove) + var staleTimestampKeys = this.lastNotificationTimes + .Where(kvp => kvp.Value < cutoff) + .Select(kvp => kvp.Key) + .ToList(); + foreach (var key in staleTimestampKeys) { - this.notificationHistory.TryRemove(key, out _); + this.lastNotificationTimes.TryRemove(key, out _); } - this.logger.LogDebug("Cleaned up notification history, removed {Count} empty entries", keysToRemove.Count); + this.logger.LogDebug( + "Cleaned up notification history, removed {Count} empty entries and {TimestampCount} stale timestamps", + keysToRemove.Count, + staleTimestampKeys.Count); } catch (Exception ex) { @@ -597,16 +639,19 @@ private async Task CleanupCallbackAsync() protected virtual void Dispose(bool disposing) { - if (!this.disposed) + if (this.disposed) { - if (disposing) - { - this.processingTimer?.Dispose(); - this.cleanupTimer?.Dispose(); - this.processingLock?.Dispose(); - this.logger.LogInformation("SmartNotificationService disposed"); - } - this.disposed = true; + return; + } + + this.disposed = true; + + if (disposing) + { + this.processingTimer?.Dispose(); + this.cleanupTimer?.Dispose(); + this.processingLock?.Dispose(); + this.logger.LogInformation("SmartNotificationService disposed"); } } diff --git a/Tests/ThreadPilot.Core.Tests/KeyboardShortcutDefaultsTests.cs b/Tests/ThreadPilot.Core.Tests/KeyboardShortcutDefaultsTests.cs new file mode 100644 index 0000000..cb4f67e --- /dev/null +++ b/Tests/ThreadPilot.Core.Tests/KeyboardShortcutDefaultsTests.cs @@ -0,0 +1,95 @@ +namespace ThreadPilot.Core.Tests +{ + using System.Windows.Input; + using Microsoft.Extensions.Logging; + using Moq; + using ThreadPilot.Models; + using ThreadPilot.Services; + + public sealed class KeyboardShortcutDefaultsTests + { + [Fact] + public async Task LoadShortcutsFromSettingsAsync_WithEmptyList_AttemptsTheDefaultShortcuts() + { + var logger = new RecordingLogger(); + using var service = CreateService(logger, new ApplicationSettingsModel()); + + await service.LoadShortcutsFromSettingsAsync(); + + var attempted = logger.Messages.Count(message => message.Contains("Skipped registering shortcut", StringComparison.Ordinal)); + Assert.Equal(service.GetDefaultShortcuts().Count, attempted); + } + + [Fact] + public async Task LoadShortcutsFromSettingsAsync_WithConfiguredShortcuts_DoesNotFallBackToDefaults() + { + var logger = new RecordingLogger(); + var settings = new ApplicationSettingsModel + { + KeyboardShortcuts = + [ + new KeyboardShortcut + { + ActionName = ShortcutActions.ShowMainWindow, + Key = Key.F8, + Modifiers = ModifierKeys.Control, + IsEnabled = true, + IsGlobal = true, + }, + ], + }; + using var service = CreateService(logger, settings); + + await service.LoadShortcutsFromSettingsAsync(); + + var attempted = logger.Messages.Count(message => message.Contains("Skipped registering shortcut", StringComparison.Ordinal)); + Assert.Equal(1, attempted); + } + + [Fact] + public void GetDefaultShortcuts_AreAllEnabledAndGlobal() + { + var logger = new RecordingLogger(); + using var service = CreateService(logger, new ApplicationSettingsModel()); + + var defaults = service.GetDefaultShortcuts(); + + Assert.NotEmpty(defaults); + Assert.All(defaults.Values, shortcut => + { + Assert.True(shortcut.IsEnabled); + Assert.True(shortcut.IsGlobal); + Assert.False(string.IsNullOrWhiteSpace(shortcut.ActionName)); + }); + } + + private static KeyboardShortcutService CreateService( + ILogger logger, + ApplicationSettingsModel settings) + { + var settingsService = new Mock(MockBehavior.Strict); + settingsService.SetupGet(service => service.Settings).Returns(settings); + return new KeyboardShortcutService(logger, settingsService.Object); + } + + private sealed class RecordingLogger : ILogger + { + public List Messages { get; } = new(); + + public IDisposable? BeginScope(TState state) + where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + { + this.Messages.Add(formatter(state, exception)); + } + } + } +} diff --git a/Tests/ThreadPilot.Core.Tests/PersistentProcessRuleJsonStoreTests.cs b/Tests/ThreadPilot.Core.Tests/PersistentProcessRuleJsonStoreTests.cs index b1520e7..a80706c 100644 --- a/Tests/ThreadPilot.Core.Tests/PersistentProcessRuleJsonStoreTests.cs +++ b/Tests/ThreadPilot.Core.Tests/PersistentProcessRuleJsonStoreTests.cs @@ -224,6 +224,54 @@ public async Task LoadAsync_WithCorruptJson_ReturnsEmptyList() } } + [Fact] + public async Task LoadAsync_AfterFailedRead_RetriesInsteadOfCachingAnEmptySet() + { + var filePath = CreateTemporaryFilePath(); + Directory.CreateDirectory(Path.GetDirectoryName(filePath)!); + await File.WriteAllTextAsync(filePath, "{ not json"); + var store = new PersistentProcessRuleJsonStore(() => filePath); + + try + { + Assert.Empty(await store.LoadAsync()); + + await new PersistentProcessRuleJsonStore(() => filePath) + .SaveAsync([CreateRule("recovered", "Recovered.exe", ProcessPriorityClass.High)]); + + var reloaded = await store.LoadAsync(); + + Assert.Equal("recovered", Assert.Single(reloaded).Id); + } + finally + { + DeleteFile(filePath); + } + } + + [Fact] + public async Task LoadAsync_WithUnreadableFile_PreservesACopyForRecovery() + { + var filePath = CreateTemporaryFilePath(); + Directory.CreateDirectory(Path.GetDirectoryName(filePath)!); + const string OriginalContent = "{ not json but the user's only copy"; + await File.WriteAllTextAsync(filePath, OriginalContent); + var store = new PersistentProcessRuleJsonStore(() => filePath); + + try + { + Assert.Empty(await store.LoadAsync()); + + var backupPath = filePath + ".unreadable"; + Assert.True(File.Exists(backupPath)); + Assert.Equal(OriginalContent, await File.ReadAllTextAsync(backupPath)); + } + finally + { + DeleteFile(filePath); + } + } + private static PersistentProcessRule CreateRule( string id, string processName, diff --git a/Tests/ThreadPilot.Core.Tests/SmartNotificationQuietHoursTests.cs b/Tests/ThreadPilot.Core.Tests/SmartNotificationQuietHoursTests.cs new file mode 100644 index 0000000..f93cef9 --- /dev/null +++ b/Tests/ThreadPilot.Core.Tests/SmartNotificationQuietHoursTests.cs @@ -0,0 +1,46 @@ +namespace ThreadPilot.Core.Tests +{ + using ThreadPilot.Services; + + public sealed class SmartNotificationQuietHoursTests + { + [Theory] + [InlineData(23, 0, 22, 8, true)] + [InlineData(2, 0, 22, 8, true)] + [InlineData(7, 59, 22, 8, true)] + [InlineData(22, 0, 22, 8, true)] + [InlineData(8, 0, 22, 8, true)] + [InlineData(12, 0, 22, 8, false)] + [InlineData(21, 59, 22, 8, false)] + [InlineData(9, 0, 22, 8, false)] + [InlineData(12, 0, 9, 17, true)] + [InlineData(9, 0, 9, 17, true)] + [InlineData(17, 0, 9, 17, true)] + [InlineData(8, 59, 9, 17, false)] + [InlineData(17, 1, 9, 17, false)] + [InlineData(23, 0, 9, 17, false)] + public void IsWithinQuietHours_HandlesOvernightAndSameDayWindows( + int hour, + int minute, + int startHour, + int endHour, + bool expected) + { + var actual = SmartNotificationService.IsWithinQuietHours( + new TimeSpan(hour, minute, 0), + TimeSpan.FromHours(startHour), + TimeSpan.FromHours(endHour)); + + Assert.Equal(expected, actual); + } + + [Fact] + public void IsWithinQuietHours_ZeroLengthWindow_IsNeverActive() + { + Assert.False(SmartNotificationService.IsWithinQuietHours( + TimeSpan.FromHours(10), + TimeSpan.FromHours(10), + TimeSpan.FromHours(10))); + } + } +} diff --git a/ViewModels/SettingsViewModel.cs b/ViewModels/SettingsViewModel.cs index 33198b4..cc27bff 100644 --- a/ViewModels/SettingsViewModel.cs +++ b/ViewModels/SettingsViewModel.cs @@ -621,17 +621,27 @@ public bool CanClose() private void OnSettingsServiceSettingsChanged(object? sender, ApplicationSettingsChangedEventArgs e) { // Marshal to UI thread to avoid cross-thread property change issues - System.Windows.Application.Current.Dispatcher.InvokeAsync(() => + System.Windows.Application.Current?.Dispatcher.InvokeAsync(() => { this.isSyncingFromService = true; try { - this.Settings.CopyFrom(e.NewSettings); + var persistedSettings = (ApplicationSettingsModel)e.NewSettings.Clone(); if (!string.IsNullOrWhiteSpace(this.cachedDefaultPowerPlanGuid)) { - this.Settings.DefaultPowerPlanId = this.cachedDefaultPowerPlanGuid; - this.Settings.DefaultPowerPlanName = this.cachedDefaultPowerPlanName; + persistedSettings.DefaultPowerPlanId = this.cachedDefaultPowerPlanGuid; + persistedSettings.DefaultPowerPlanName = this.cachedDefaultPowerPlanName; } + + if (this.HasUnsavedChanges) + { + this.savedSettingsSnapshot = persistedSettings; + this.UpdatePendingChangesState(); + this.Logger.LogDebug("Settings changed externally while edits were pending; kept the pending edits and re-based the saved snapshot"); + return; + } + + this.Settings.CopyFrom(persistedSettings); this.SetSavedSettingsSnapshot(this.Settings); this.ApplyLanguagePreference(this.Settings.Language, logUserAction: false); this.StatusMessage = this.GetLocalizedString("Settings_StatusSynchronized", "Settings synchronized"); @@ -643,6 +653,13 @@ private void OnSettingsServiceSettingsChanged(object? sender, ApplicationSetting }); } + protected override void OnDispose() + { + this.Settings.PropertyChanged -= this.OnSettingsPropertyChanged; + this.settingsService.SettingsChanged -= this.OnSettingsServiceSettingsChanged; + base.OnDispose(); + } + private async Task RefreshPowerPlansAsync() { try From 7d478021ec6f5f0f8593877074dfc7c1389a07aa Mon Sep 17 00:00:00 2001 From: Kool <79905997+eakkawut@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:44:08 +0700 Subject: [PATCH 2/7] fix: dispose services and view models on shutdown The service container was never disposed. Disposing it is what flushes buffered structured logs, stops the WMI watchers and releases native performance counters, so the last few seconds of diagnostics before every exit were lost. ProcessMonitorService.Dispose set the disposed flag before calling StopMonitoringAsync, which short-circuits on that flag, so Dispose was a guaranteed no-op and left the WMI watchers and the polling timer running. Idempotency is now tracked separately from the disposed flag, and the semaphore call sites are guarded against ObjectDisposedException. PowerPlanViewModel now releases its ten-second refresh timer. LogViewerViewModel and ProcessPowerPlanAssociationViewModel detach from the singleton services they subscribed to, so a closed window no longer keeps handlers alive for the rest of the process. The two async void handlers in SettingsWindow are wrapped, so a failure there reports instead of tearing down the process. --- App.xaml.cs | 20 ++++++++ Services/ProcessMonitorService.cs | 51 ++++++++++++++++--- .../ProcessMonitorServiceSettingsTests.cs | 38 ++++++++++++++ ViewModels/LogViewerViewModel.cs | 15 +++++- ViewModels/PowerPlanViewModel.cs | 23 ++++++++- .../ProcessPowerPlanAssociationViewModel.cs | 8 +++ Views/SettingsWindow.xaml.cs | 23 +++++++-- 7 files changed, 164 insertions(+), 14 deletions(-) diff --git a/App.xaml.cs b/App.xaml.cs index 1513528..95c04b6 100644 --- a/App.xaml.cs +++ b/App.xaml.cs @@ -338,6 +338,8 @@ protected override void OnExit(ExitEventArgs e) this.DispatcherUnhandledException -= this.OnDispatcherUnhandledException; TaskScheduler.UnobservedTaskException -= this.OnUnobservedTaskException; + this.DisposeServiceProvider(); + if (this.singleInstanceMutex != null) { try @@ -355,6 +357,24 @@ protected override void OnExit(ExitEventArgs e) base.OnExit(e); } + private void DisposeServiceProvider() + { + if (this.ServiceProvider is not IDisposable disposableProvider) + { + return; + } + + var logger = this.ServiceProvider.GetService>(); + try + { + disposableProvider.Dispose(); + } + catch (Exception ex) + { + logger?.LogWarning(ex, "Failed to dispose the application service provider during shutdown"); + } + } + #if DEBUG [System.Runtime.InteropServices.DllImport("kernel32.dll")] private static extern bool AllocConsole(); diff --git a/Services/ProcessMonitorService.cs b/Services/ProcessMonitorService.cs index 51740cc..b1253c7 100644 --- a/Services/ProcessMonitorService.cs +++ b/Services/ProcessMonitorService.cs @@ -30,6 +30,7 @@ public class ProcessMonitorService : IProcessMonitorService private bool isWmiAvailable; private bool isFallbackPollingActive; private int disposedFlag; + private int disposeRequestedFlag; // Configuration - will be updated from settings private int fallbackPollingIntervalMs = 5000; // Default 5 seconds @@ -131,8 +132,15 @@ public async Task StopMonitoringAsync() } var semaphoreHeld = false; - await this.wmiStartSemaphore.WaitAsync().ConfigureAwait(false); - semaphoreHeld = true; + try + { + await this.wmiStartSemaphore.WaitAsync().ConfigureAwait(false); + semaphoreHeld = true; + } + catch (ObjectDisposedException) + { + return; + } try { @@ -167,7 +175,13 @@ public async Task StopMonitoringAsync() { if (semaphoreHeld) { - this.wmiStartSemaphore.Release(); + try + { + this.wmiStartSemaphore.Release(); + } + catch (ObjectDisposedException) + { + } } } } @@ -227,7 +241,15 @@ private async Task TryStartWmiMonitoringAsync() return false; } - await this.wmiStartSemaphore.WaitAsync().ConfigureAwait(false); + try + { + await this.wmiStartSemaphore.WaitAsync().ConfigureAwait(false); + } + catch (ObjectDisposedException) + { + return false; + } + try { if (this.IsDisposed || !this.isMonitoring || !this.enableWmiMonitoring) @@ -286,7 +308,13 @@ await Task.Run(() => } finally { - this.wmiStartSemaphore.Release(); + try + { + this.wmiStartSemaphore.Release(); + } + catch (ObjectDisposedException) + { + } } } @@ -684,7 +712,7 @@ private static string NormalizeProcessName(string processName) public void Dispose() { - if (Interlocked.Exchange(ref this.disposedFlag, 1) == 1) + if (Interlocked.Exchange(ref this.disposeRequestedFlag, 1) == 1) { return; } @@ -698,7 +726,16 @@ public void Dispose() this.OnMonitoringStatusChanged($"Error during process monitor disposal: {ex.Message}", ex); } - this.wmiStartSemaphore.Dispose(); + Interlocked.Exchange(ref this.disposedFlag, 1); + + try + { + this.wmiStartSemaphore.Dispose(); + } + catch (Exception ex) + { + this.OnMonitoringStatusChanged($"Error releasing process monitor resources: {ex.Message}", ex); + } } } } diff --git a/Tests/ThreadPilot.Core.Tests/ProcessMonitorServiceSettingsTests.cs b/Tests/ThreadPilot.Core.Tests/ProcessMonitorServiceSettingsTests.cs index 08fef0e..2e9acf5 100644 --- a/Tests/ThreadPilot.Core.Tests/ProcessMonitorServiceSettingsTests.cs +++ b/Tests/ThreadPilot.Core.Tests/ProcessMonitorServiceSettingsTests.cs @@ -53,6 +53,44 @@ public async Task StartMonitoringAsync_UsesFallbackPollingIntervalFromApplicatio Assert.Contains("Fallback polling started (interval: 12345ms)", messages); } + [Fact] + public async Task Dispose_ActuallyStopsMonitoring() + { + var monitor = CreateMonitor(new ApplicationSettingsModel + { + EnableWmiMonitoring = false, + EnableFallbackPolling = true, + FallbackPollingIntervalMs = 60000, + }); + + await monitor.StartMonitoringAsync(); + Assert.True(monitor.IsMonitoring); + Assert.True(monitor.IsFallbackPollingActive); + + monitor.Dispose(); + + Assert.False(monitor.IsMonitoring); + Assert.False(monitor.IsFallbackPollingActive); + } + + [Fact] + public async Task Dispose_IsIdempotent() + { + var monitor = CreateMonitor(new ApplicationSettingsModel + { + EnableWmiMonitoring = false, + EnableFallbackPolling = true, + FallbackPollingIntervalMs = 60000, + }); + + await monitor.StartMonitoringAsync(); + + monitor.Dispose(); + monitor.Dispose(); + + Assert.False(monitor.IsMonitoring); + } + private static ProcessMonitorService CreateMonitor(ApplicationSettingsModel settings) { var processService = new Mock(MockBehavior.Strict); diff --git a/ViewModels/LogViewerViewModel.cs b/ViewModels/LogViewerViewModel.cs index 1e56a04..21cea6a 100644 --- a/ViewModels/LogViewerViewModel.cs +++ b/ViewModels/LogViewerViewModel.cs @@ -12,13 +12,14 @@ namespace ThreadPilot.ViewModels { - public partial class LogViewerViewModel : ObservableObject + public partial class LogViewerViewModel : ObservableObject, IDisposable { private readonly IActivityAuditService activityAuditService; private readonly IEnhancedLoggingService loggingService; private readonly IApplicationSettingsService settingsService; private readonly ILogger logger; private bool isActive; + private bool disposed; [ObservableProperty] private ObservableCollection logEntries = new(); @@ -355,6 +356,18 @@ private void StartAutoRefresh() // For now, we'll keep it simple without the timer } + public void Dispose() + { + if (this.disposed) + { + return; + } + + this.disposed = true; + this.isActive = false; + this.activityAuditService.EntryAdded -= this.OnActivityEntryAdded; + } + private void OnActivityEntryAdded(object? sender, ActivityAuditEntry entry) { if (!this.isActive || !this.ShouldDisplay(entry)) diff --git a/ViewModels/PowerPlanViewModel.cs b/ViewModels/PowerPlanViewModel.cs index 2e832ed..d727e82 100644 --- a/ViewModels/PowerPlanViewModel.cs +++ b/ViewModels/PowerPlanViewModel.cs @@ -64,8 +64,13 @@ private void SetupRefreshTimer() try { - // Marshal timer callback to UI thread to prevent cross-thread access exceptions - await System.Windows.Application.Current.Dispatcher.InvokeAsync(async () => + var dispatcher = System.Windows.Application.Current?.Dispatcher; + if (dispatcher == null) + { + return; + } + + await dispatcher.InvokeAsync(async () => { if (!this.isAutoRefreshPaused) { @@ -84,6 +89,20 @@ await System.Windows.Application.Current.Dispatcher.InvokeAsync(async () => }; } + protected override void OnDispose() + { + this.isAutoRefreshPaused = true; + + if (this.refreshTimer != null) + { + this.refreshTimer.Stop(); + this.refreshTimer.Dispose(); + this.refreshTimer = null; + } + + base.OnDispose(); + } + public void PauseAutoRefresh() { this.isAutoRefreshPaused = true; diff --git a/ViewModels/ProcessPowerPlanAssociationViewModel.cs b/ViewModels/ProcessPowerPlanAssociationViewModel.cs index b55c3cc..80a025b 100644 --- a/ViewModels/ProcessPowerPlanAssociationViewModel.cs +++ b/ViewModels/ProcessPowerPlanAssociationViewModel.cs @@ -126,6 +126,14 @@ public ProcessPowerPlanAssociationViewModel( this.monitorManagerService.ProcessPowerPlanChanged += this.OnProcessPowerPlanChanged; } + protected override void OnDispose() + { + this.associationService.ConfigurationChanged -= this.OnConfigurationChanged; + this.monitorManagerService.ServiceStatusChanged -= this.OnServiceStatusChanged; + this.monitorManagerService.ProcessPowerPlanChanged -= this.OnProcessPowerPlanChanged; + base.OnDispose(); + } + public override async Task InitializeAsync() { if (this.isInitialized) diff --git a/Views/SettingsWindow.xaml.cs b/Views/SettingsWindow.xaml.cs index a3de744..4431826 100644 --- a/Views/SettingsWindow.xaml.cs +++ b/Views/SettingsWindow.xaml.cs @@ -33,16 +33,31 @@ protected override void OnClosing(CancelEventArgs e) private async void UnsavedSettingsSave_Click(object sender, RoutedEventArgs e) { - var saved = await this.viewModel.SaveIfDirtyAsync(); - if (saved) + try { - this.CloseAfterUnsavedPrompt(); + var saved = await this.viewModel.SaveIfDirtyAsync(); + if (saved) + { + this.CloseAfterUnsavedPrompt(); + } + } + catch (Exception ex) + { + System.Diagnostics.Debug.WriteLine($"Failed to save settings from the unsaved-changes prompt: {ex.Message}"); } } private async void UnsavedSettingsDiscard_Click(object sender, RoutedEventArgs e) { - await this.viewModel.DiscardPendingChangesAsync(); + try + { + await this.viewModel.DiscardPendingChangesAsync(); + } + catch (Exception ex) + { + System.Diagnostics.Debug.WriteLine($"Failed to discard pending settings: {ex.Message}"); + } + this.CloseAfterUnsavedPrompt(); } From 1488cdb60317d02f510f0284e6b4b372da36a391 Mon Sep 17 00:00:00 2001 From: Kool <79905997+eakkawut@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:44:09 +0700 Subject: [PATCH 3/7] fix: keep window close and tray updates on the right thread MainWindow.OnClosing cancels the close and then runs the shutdown chain. A fault anywhere in that chain left the window permanently unclosable with no visible error, because the close had already been cancelled. It now runs through TaskSafety with a fallback Shutdown(). The tray context-menu rebuild mutates Windows Forms ToolStrip state from a powercfg continuation, that is, a thread-pool thread. UpdateContextMenuAsync now takes a dispatcher and marshals every tray call, and monitoring-status updates raised from the WMI watcher and polling-timer threads are dispatched too. The tray power-plan entry could also go stale. It refreshes from the existing IPowerPlanService.PowerPlanChanged notification rather than by re-enabling the periodic tray timer: that timer is gated off by ShowAdvancedDiagnostics, and the gating is what produced the idle-CPU reduction recorded for v1.4.4. OnClosed also disposes the view models the window owns, which is what the disposal overrides in the previous commit exist for. ProcessViewModel and MasksViewModel are singletons and are left to the container. --- MainWindow.Behaviors.partial.cs | 53 +++++++++++++-- MainWindow.xaml.cs | 1 + Services/SystemTrayStatusUpdater.cs | 55 ++++++++++------ .../SystemTrayStatusUpdaterTests.cs | 65 ++++++++++++++++++- 4 files changed, 148 insertions(+), 26 deletions(-) diff --git a/MainWindow.Behaviors.partial.cs b/MainWindow.Behaviors.partial.cs index 46d1856..c62f50c 100644 --- a/MainWindow.Behaviors.partial.cs +++ b/MainWindow.Behaviors.partial.cs @@ -603,6 +603,10 @@ private async Task InitializeSystemTrayAsync() // Initialize system tray context menu with current data await this.UpdateSystemTrayContextMenuAsync(); + this.trayPowerPlanService ??= this.serviceProvider.GetRequiredService(); + this.trayPowerPlanService.PowerPlanChanged -= this.OnPowerPlanChangedForTray; + this.trayPowerPlanService.PowerPlanChanged += this.OnPowerPlanChangedForTray; + // Start periodic system tray updates this.StartSystemTrayUpdateTimer(); } @@ -1092,14 +1096,24 @@ private async Task UpdateSystemTrayContextMenuAsync() { try { - await this.systemTrayStatusUpdater.UpdateContextMenuAsync(this.systemTrayService); + await this.systemTrayStatusUpdater.UpdateContextMenuAsync( + this.systemTrayService, + action => this.Dispatcher.InvokeAsync(action).Task); } catch (Exception ex) { - System.Diagnostics.Debug.WriteLine($"Failed to update system tray context menu: {ex.Message}"); + this.LogDebug($"Failed to update system tray context menu: {ex.Message}"); } } + private void OnPowerPlanChangedForTray(object? sender, PowerPlanChangedEventArgs e) + { + TaskSafety.FireAndForget(this.UpdateSystemTrayContextMenuAsync(), ex => + { + this.LogDebug($"Failed to refresh tray menu after power plan change: {ex.Message}"); + }); + } + private void StartSystemTrayUpdateTimer() { try @@ -1304,8 +1318,15 @@ private string Localize(string key, string fallback) => private void OnMonitoringStatusChanged(object? sender, MonitoringStatusEventArgs e) { - // Update tray icon and status - this.systemTrayService.UpdateMonitoringStatus(e.IsMonitoring, e.IsWmiAvailable); + if (this.Dispatcher.CheckAccess()) + { + this.systemTrayService.UpdateMonitoringStatus(e.IsMonitoring, e.IsWmiAvailable); + } + else + { + this.Dispatcher.InvokeAsync(() => + this.systemTrayService.UpdateMonitoringStatus(e.IsMonitoring, e.IsWmiAvailable)); + } // Show notification if there's an error if (e.Error != null && this.settingsService.Settings.EnableErrorNotifications) @@ -2045,7 +2066,16 @@ protected override void OnClosing(System.ComponentModel.CancelEventArgs e) } e.Cancel = true; - _ = this.HandleWindowCloseAsync(); + + TaskSafety.FireAndForget(this.HandleWindowCloseAsync(), ex => + { + this.LogDebug($"Window close handling failed: {ex.Message}"); + this.Dispatcher.InvokeAsync(() => + { + this.isPerformingShutdown = true; + System.Windows.Application.Current?.Shutdown(); + }); + }); } protected override void OnClosed(EventArgs e) @@ -2060,6 +2090,12 @@ protected override void OnClosed(EventArgs e) this.processMonitorManagerService.ServiceStatusChanged -= this.OnProcessMonitorManagerStatusChanged; this.keyboardShortcutService.ShortcutActivated -= this.OnShortcutActivated; + if (this.trayPowerPlanService != null) + { + this.trayPowerPlanService.PowerPlanChanged -= this.OnPowerPlanChangedForTray; + this.trayPowerPlanService = null; + } + this.UnsubscribeSystemTrayEvents(); this.systemTrayUpdateTimer?.Stop(); @@ -2067,7 +2103,14 @@ protected override void OnClosed(EventArgs e) this.initializationTimeoutTimer?.Stop(); this.initializationTimeoutTimer?.Dispose(); + this.performanceViewModel?.Dispose(); + this.settingsViewModel.Dispose(); + this.powerPlanViewModel.Dispose(); + this.associationViewModel.Dispose(); + this.logViewerViewModel.Dispose(); + this.systemTweaksViewModel.Dispose(); + this.mainWindowViewModel.Dispose(); this.selfResourceManagementService.RestoreForegroundMode(); this.navigationBehavior.Dispose(); diff --git a/MainWindow.xaml.cs b/MainWindow.xaml.cs index a37a279..4d7520d 100644 --- a/MainWindow.xaml.cs +++ b/MainWindow.xaml.cs @@ -70,6 +70,7 @@ public partial class MainWindow : Wpf.Ui.Controls.FluentWindow private TaskCompletionSource? unsavedSettingsDialogCompletionSource; private bool isSilentStartupMode; private bool showStartupMinimizedSuggestionOnReady; + private IPowerPlanService? trayPowerPlanService; public MainWindow( ProcessViewModel processViewModel, diff --git a/Services/SystemTrayStatusUpdater.cs b/Services/SystemTrayStatusUpdater.cs index 4fa050b..9d5df44 100644 --- a/Services/SystemTrayStatusUpdater.cs +++ b/Services/SystemTrayStatusUpdater.cs @@ -5,13 +5,14 @@ namespace ThreadPilot.Services using System.IO; using System.Linq; using System.Threading.Tasks; + using Microsoft.Extensions.Logging; using ThreadPilot.Models; public interface ISystemTrayStatusUpdater { bool ShouldRunPerformanceStatusUpdates { get; } - Task UpdateContextMenuAsync(ISystemTrayService systemTrayService); + Task UpdateContextMenuAsync(ISystemTrayService systemTrayService, Func dispatchAsync); Task UpdateStatusAsync(ISystemTrayService systemTrayService, Func dispatchAsync); } @@ -21,34 +22,34 @@ public sealed class SystemTrayStatusUpdater : ISystemTrayStatusUpdater private readonly IPowerPlanService powerPlanService; private readonly Lazy performanceService; private readonly ILocalizationService? localizationService; + private readonly ILogger? logger; public SystemTrayStatusUpdater( IPowerPlanService powerPlanService, Lazy performanceService, - ILocalizationService? localizationService = null) + ILocalizationService? localizationService = null, + ILogger? logger = null) { this.powerPlanService = powerPlanService ?? throw new ArgumentNullException(nameof(powerPlanService)); this.performanceService = performanceService ?? throw new ArgumentNullException(nameof(performanceService)); this.localizationService = localizationService; + this.logger = logger; } public bool ShouldRunPerformanceStatusUpdates => AppNavigationOptions.ShowAdvancedDiagnostics; - public async Task UpdateContextMenuAsync(ISystemTrayService systemTrayService) + public async Task UpdateContextMenuAsync(ISystemTrayService systemTrayService, Func dispatchAsync) { ArgumentNullException.ThrowIfNull(systemTrayService); + ArgumentNullException.ThrowIfNull(dispatchAsync); - var activePowerPlan = await this.UpdatePowerPlanMenuAsync(systemTrayService).ConfigureAwait(false); - this.UpdateProfileMenu(systemTrayService); + var activePowerPlan = await this.UpdatePowerPlanMenuAsync(systemTrayService, dispatchAsync).ConfigureAwait(false); + await this.UpdateProfileMenuAsync(systemTrayService, dispatchAsync).ConfigureAwait(false); await this.UpdateStatusCoreAsync( systemTrayService, activePowerPlan, - action => - { - action(); - return Task.CompletedTask; - }).ConfigureAwait(false); + dispatchAsync).ConfigureAwait(false); } public async Task UpdateStatusAsync(ISystemTrayService systemTrayService, Func dispatchAsync) @@ -62,34 +63,48 @@ public async Task UpdateStatusAsync(ISystemTrayService systemTrayService, await this.UpdateStatusCoreAsync(systemTrayService, activePowerPlan, dispatchAsync).ConfigureAwait(false); return true; } - catch + catch (Exception ex) { + this.logger?.LogDebug(ex, "Failed to update the system tray status"); return false; } } - private async Task UpdatePowerPlanMenuAsync(ISystemTrayService systemTrayService) + private async Task UpdatePowerPlanMenuAsync( + ISystemTrayService systemTrayService, + Func dispatchAsync) { var powerPlans = await this.powerPlanService.GetPowerPlansAsync().ConfigureAwait(false); var activePowerPlan = powerPlans.FirstOrDefault(plan => plan.IsActive); - systemTrayService.UpdatePowerPlans(powerPlans, activePowerPlan); + + await dispatchAsync(() => systemTrayService.UpdatePowerPlans(powerPlans, activePowerPlan)).ConfigureAwait(false); return activePowerPlan; } - private void UpdateProfileMenu(ISystemTrayService systemTrayService) + private async Task UpdateProfileMenuAsync( + ISystemTrayService systemTrayService, + Func dispatchAsync) { var profilesDirectory = StoragePaths.ProfilesDirectory; var profileNames = new List(); - if (Directory.Exists(profilesDirectory)) + try + { + if (Directory.Exists(profilesDirectory)) + { + profileNames = Directory.GetFiles(profilesDirectory, "*.json") + .Select(Path.GetFileNameWithoutExtension) + .Where(name => !string.IsNullOrWhiteSpace(name)) + .ToList()!; + } + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { - profileNames = Directory.GetFiles(profilesDirectory, "*.json") - .Select(Path.GetFileNameWithoutExtension) - .Where(name => !string.IsNullOrWhiteSpace(name)) - .ToList()!; + this.logger?.LogDebug(ex, "Could not enumerate saved profiles for the tray menu"); + profileNames = new List(); } - systemTrayService.UpdateProfiles(profileNames); + await dispatchAsync(() => systemTrayService.UpdateProfiles(profileNames)).ConfigureAwait(false); } private async Task UpdateStatusCoreAsync( diff --git a/Tests/ThreadPilot.Core.Tests/SystemTrayStatusUpdaterTests.cs b/Tests/ThreadPilot.Core.Tests/SystemTrayStatusUpdaterTests.cs index 97b0051..2a765b8 100644 --- a/Tests/ThreadPilot.Core.Tests/SystemTrayStatusUpdaterTests.cs +++ b/Tests/ThreadPilot.Core.Tests/SystemTrayStatusUpdaterTests.cs @@ -13,7 +13,7 @@ public async Task UpdateContextMenuAsync_DiagnosticsHidden_DoesNotResolvePerform var harness = new Harness(); var updater = harness.CreateUpdater(performanceFactory: () => throw new InvalidOperationException("Performance service should not be resolved.")); - await updater.UpdateContextMenuAsync(harness.Tray.Object); + await updater.UpdateContextMenuAsync(harness.Tray.Object, Harness.PassThroughDispatcher); harness.Tray.Verify(x => x.UpdatePowerPlans(It.IsAny>(), It.IsAny()), Times.Once); harness.Tray.Verify(x => x.UpdateProfiles(It.IsAny>()), Times.Once); @@ -21,6 +21,63 @@ public async Task UpdateContextMenuAsync_DiagnosticsHidden_DoesNotResolvePerform harness.Tray.Verify(x => x.UpdateSystemStatus(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } + [Fact] + public async Task UpdateContextMenuAsync_MarshalsEveryTrayMutationThroughTheDispatcher() + { + var harness = new Harness(); + var updater = harness.CreateUpdater(performanceFactory: () => throw new InvalidOperationException("Performance service should not be resolved.")); + var dispatchedCallCount = 0; + var undispatchedTrayCalls = 0; + var insideDispatcher = false; + + harness.Tray + .Setup(x => x.UpdatePowerPlans(It.IsAny>(), It.IsAny())) + .Callback(() => + { + if (!insideDispatcher) + { + undispatchedTrayCalls++; + } + }); + harness.Tray + .Setup(x => x.UpdateProfiles(It.IsAny>())) + .Callback(() => + { + if (!insideDispatcher) + { + undispatchedTrayCalls++; + } + }); + harness.Tray + .Setup(x => x.UpdateSystemStatus(It.IsAny())) + .Callback(() => + { + if (!insideDispatcher) + { + undispatchedTrayCalls++; + } + }); + + await updater.UpdateContextMenuAsync(harness.Tray.Object, action => + { + dispatchedCallCount++; + insideDispatcher = true; + try + { + action(); + } + finally + { + insideDispatcher = false; + } + + return Task.CompletedTask; + }); + + Assert.Equal(0, undispatchedTrayCalls); + Assert.Equal(3, dispatchedCallCount); + } + [Fact] public async Task UpdateStatusAsync_DiagnosticsHidden_DoesNotRequestLightweightMetrics() { @@ -41,6 +98,12 @@ public async Task UpdateStatusAsync_DiagnosticsHidden_DoesNotRequestLightweightM private sealed class Harness { + public static Task PassThroughDispatcher(Action action) + { + action(); + return Task.CompletedTask; + } + public Mock Tray { get; } = new(MockBehavior.Strict); public Mock PowerPlan { get; } = new(MockBehavior.Strict); From 42b8f8a411105bc70e449b72e0e348b6ffa222ce Mon Sep 17 00:00:00 2001 From: Kool <79905997+eakkawut@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:44:09 +0700 Subject: [PATCH 4/7] fix: harden performance monitoring and CPU Set probing PerformanceMonitoringService had four problems: historicalData was mutated without a lock, a second StartMonitoringAsync could install a duplicate timer, the core-counter list could be replaced while a tick was iterating it, and disposal released the counters before marking state so an in-flight tick could resurrect them. It now locks the history, guards the start with Interlocked, snapshots the counter list, and marks state before releasing anything. The first tick is delayed one second instead of firing immediately. WMI result collections and objects are disposed. Per-core counters only ever covered the first 64 logical processors, because the legacy Processor category cannot express processor groups. Above that limit the group-aware Processor Information category is used, and a missing individual instance no longer clears the whole per-core view. A failed CPU Set topology probe marked the mapping initialised, so one transient failure disabled CPU Sets for the entire process lifetime. The initialised flag is now only set on success, with a thirty-second backoff between retries. The failure path still reports CpuSetsUnavailable and callers still fall back exactly as before. EnhancedLoggingService.IsDebugLoggingEnabled deep-cloned the settings model on every structured log call, because the settings accessor clones. The flag is cached and kept current from SettingsChanged instead. --- Platforms/Windows/ProcessCpuSetHandler.cs | 20 ++- Services/EnhancedLoggingService.cs | 12 +- Services/PerformanceMonitoringService.cs | 181 +++++++++++++++++----- 3 files changed, 164 insertions(+), 49 deletions(-) diff --git a/Platforms/Windows/ProcessCpuSetHandler.cs b/Platforms/Windows/ProcessCpuSetHandler.cs index 4cf6e53..ff79ffc 100644 --- a/Platforms/Windows/ProcessCpuSetHandler.cs +++ b/Platforms/Windows/ProcessCpuSetHandler.cs @@ -14,6 +14,9 @@ public class ProcessCpuSetHandler : IProcessCpuSetHandler private static CpuSetMapping staticCpuSetMapping = CpuSetMapping.Empty; private static readonly object staticInitLock = new object(); private static bool staticInitialized = false; + private static long lastCpuSetMappingFailureTicks = -1; + + private static readonly TimeSpan CpuSetMappingRetryInterval = TimeSpan.FromSeconds(30); private readonly Queue cpuTimeMovingAverageBuffer = new(); private readonly string executableName; @@ -77,19 +80,26 @@ private static CpuSetMapping EnsureStaticInitialization(IProcessCpuSetNativeApi return staticCpuSetMapping; } + if (lastCpuSetMappingFailureTicks >= 0 && + Environment.TickCount64 - lastCpuSetMappingFailureTicks < CpuSetMappingRetryInterval.TotalMilliseconds) + { + return CpuSetMapping.Empty; + } + try { staticCpuSetMapping = GetCpuSetMapping(nativeApi); + lastCpuSetMappingFailureTicks = -1; + + staticInitialized = true; + return staticCpuSetMapping; } catch (Exception) { - // If we can't get CPU Set mapping, CPU Sets won't be available - // The handler will still work but ApplyCpuSetMask will return false staticCpuSetMapping = CpuSetMapping.Empty; + lastCpuSetMappingFailureTicks = Environment.TickCount64; + return CpuSetMapping.Empty; } - - staticInitialized = true; - return staticCpuSetMapping; } } diff --git a/Services/EnhancedLoggingService.cs b/Services/EnhancedLoggingService.cs index 93e33e5..4158c1f 100644 --- a/Services/EnhancedLoggingService.cs +++ b/Services/EnhancedLoggingService.cs @@ -22,6 +22,7 @@ public class EnhancedLoggingService : IEnhancedLoggingService, IDisposable private int flushScheduled; private bool isInitialized; private bool disposed; + private volatile bool isDebugLoggingEnabled; // PERFORMANCE IMPROVEMENT: Correlation tracking for better debugging internal readonly AsyncLocal CorrelationId = new(); @@ -31,7 +32,7 @@ public class EnhancedLoggingService : IEnhancedLoggingService, IDisposable public string LogDirectoryPath => this.logDirectory; - public bool IsDebugLoggingEnabled => this.settingsService.Settings.EnableDebugLogging; + public bool IsDebugLoggingEnabled => this.isDebugLoggingEnabled; public event EventHandler? CriticalErrorOccurred; @@ -44,9 +45,17 @@ public EnhancedLoggingService(ILogger logger, IApplicati this.logDirectory = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "ThreadPilot", "Logs"); this.currentLogFilePath = this.GetCurrentLogFilePath(); + this.isDebugLoggingEnabled = settingsService.Settings.EnableDebugLogging; + this.settingsService.SettingsChanged += this.OnSettingsChanged; + this.flushTimer = new System.Threading.Timer(this.FlushLogs, null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); } + private void OnSettingsChanged(object? sender, ApplicationSettingsChangedEventArgs e) + { + this.isDebugLoggingEnabled = e.NewSettings.EnableDebugLogging; + } + public async Task InitializeAsync() { if (this.isInitialized) @@ -517,6 +526,7 @@ public void Dispose() return; } + this.settingsService.SettingsChanged -= this.OnSettingsChanged; this.flushTimer?.Dispose(); this.FlushLogsAsync().Wait(TimeSpan.FromSeconds(5)); this.fileLock?.Dispose(); diff --git a/Services/PerformanceMonitoringService.cs b/Services/PerformanceMonitoringService.cs index 7f7d2ef..044a98d 100644 --- a/Services/PerformanceMonitoringService.cs +++ b/Services/PerformanceMonitoringService.cs @@ -32,7 +32,9 @@ public class PerformanceMonitoringService : IPerformanceMonitoringService, IDisp private int cachedProcessCount; private DateTime processCountCacheUtc = DateTime.MinValue; private readonly object runtimeTelemetryLock = new(); + private readonly object historicalDataLock = new(); private int isMonitoringTickInProgress; + private int monitoringStartedFlag; private bool runtimeTelemetryInitialized; private int previousGen0Collections; private int previousGen1Collections; @@ -40,13 +42,17 @@ public class PerformanceMonitoringService : IPerformanceMonitoringService, IDisp private long previousTotalAllocatedBytes; private double maxObservedGcPauseMs; private DateTime lastGcPauseAlertUtc = DateTime.MinValue; - private bool isMonitoring; - private bool disposed; + private volatile bool isMonitoring; + private volatile bool disposed; private static readonly TimeSpan GcPauseAlertCooldown = TimeSpan.FromMinutes(1); private static readonly TimeSpan WmiQueryTimeout = TimeSpan.FromSeconds(5); + + private static readonly TimeSpan FirstMonitoringTickDelay = TimeSpan.FromSeconds(1); private const int HistoricalDataCapacity = 1000; private const double Gen2PauseAlertThresholdMs = 100; + private const int LegacyProcessorCategoryInstanceLimit = 64; + private const string GroupAwareProcessorCategory = "Processor Information"; public event EventHandler? MetricsUpdated; @@ -99,12 +105,15 @@ public async Task GetSystemMetricsAsync(bool lightweig metrics.TopMemoryProcess = topMemoryProcesses.FirstOrDefault(); // Store in historical data - if (this.historicalData.Count >= HistoricalDataCapacity) + lock (this.historicalDataLock) { - this.historicalData.Dequeue(); - } + if (this.historicalData.Count >= HistoricalDataCapacity) + { + this.historicalData.Dequeue(); + } - this.historicalData.Enqueue(metrics); + this.historicalData.Enqueue(metrics); + } } return metrics; @@ -125,9 +134,15 @@ public async Task> GetCpuCoreUsageAsync() this.EnsureCpuCoreCountersInitialized(); var topology = await this.cpuTopologyService.DetectTopologyAsync().ConfigureAwait(false); - for (int i = 0; i < this.cpuCoreCounters.Count; i++) + PerformanceCounter[] counters; + lock (this.counterInitializationLock) + { + counters = this.cpuCoreCounters.ToArray(); + } + + for (int i = 0; i < counters.Length; i++) { - var counter = this.cpuCoreCounters[i]; + var counter = counters[i]; var usage = counter.NextValue(); var coreUsage = new CpuCoreUsage @@ -161,9 +176,15 @@ public async Task GetMemoryUsageAsync() // Get physical memory info var scope = CreateCimv2ScopeWithTimeout(); using var searcher = new ManagementObjectSearcher(scope, new ObjectQuery("SELECT TotalPhysicalMemory FROM Win32_ComputerSystem")); - foreach (var obj in searcher.Get()) + using (var results = searcher.Get()) { - memoryInfo.TotalPhysicalMemory = Convert.ToInt64(obj["TotalPhysicalMemory"]); + foreach (var obj in results) + { + using (obj) + { + memoryInfo.TotalPhysicalMemory = Convert.ToInt64(obj["TotalPhysicalMemory"]); + } + } } // Get available memory @@ -175,10 +196,16 @@ public async Task GetMemoryUsageAsync() // Get virtual memory info using var memSearcher = new ManagementObjectSearcher(scope, new ObjectQuery("SELECT TotalVirtualMemorySize, FreeVirtualMemory FROM Win32_OperatingSystem")); - foreach (var obj in memSearcher.Get()) + using (var memResults = memSearcher.Get()) { - memoryInfo.TotalVirtualMemory = Convert.ToInt64(obj["TotalVirtualMemorySize"]) * 1024; // Convert KB to bytes - memoryInfo.AvailableVirtualMemory = Convert.ToInt64(obj["FreeVirtualMemory"]) * 1024; + foreach (var obj in memResults) + { + using (obj) + { + memoryInfo.TotalVirtualMemory = Convert.ToInt64(obj["TotalVirtualMemorySize"]) * 1024; // Convert KB to bytes + memoryInfo.AvailableVirtualMemory = Convert.ToInt64(obj["FreeVirtualMemory"]) * 1024; + } + } } memoryInfo.UsedVirtualMemory = memoryInfo.TotalVirtualMemory - memoryInfo.AvailableVirtualMemory; @@ -251,11 +278,16 @@ public async Task> GetTopMemoryProcessesAsync(int c } } - public async Task StartMonitoringAsync() + public Task StartMonitoringAsync() { - if (this.isMonitoring) + if (this.disposed) { - return; + return Task.CompletedTask; + } + + if (Interlocked.CompareExchange(ref this.monitoringStartedFlag, 1, 0) == 1) + { + return Task.CompletedTask; } this.logger.LogInformation("Starting performance monitoring"); @@ -273,6 +305,11 @@ public async Task StartMonitoringAsync() try { + if (this.disposed || !this.isMonitoring) + { + return; + } + var metrics = await this.GetSystemMetricsAsync().ConfigureAwait(false); await this.EmitGcDiagnosticsIfNeededAsync(metrics).ConfigureAwait(false); this.MetricsUpdated?.Invoke(this, new PerformanceMetricsUpdatedEventArgs(metrics)); @@ -285,12 +322,14 @@ public async Task StartMonitoringAsync() { Interlocked.Exchange(ref this.isMonitoringTickInProgress, 0); } - }, null, TimeSpan.Zero, TimeSpan.FromSeconds(2)); + }, null, FirstMonitoringTickDelay, TimeSpan.FromSeconds(2)); + + return Task.CompletedTask; } public Task StopMonitoringAsync() { - if (!this.isMonitoring) + if (Interlocked.CompareExchange(ref this.monitoringStartedFlag, 0, 1) == 0) { return Task.CompletedTask; } @@ -307,13 +346,21 @@ public Task StopMonitoringAsync() public Task> GetHistoricalDataAsync(TimeSpan duration) { var cutoffTime = DateTime.UtcNow - duration; - var data = this.historicalData.Where(m => m.Timestamp >= cutoffTime).ToList(); - return Task.FromResult(data); + + lock (this.historicalDataLock) + { + var data = this.historicalData.Where(m => m.Timestamp >= cutoffTime).ToList(); + return Task.FromResult(data); + } } public Task ClearHistoricalDataAsync() { - this.historicalData.Clear(); + lock (this.historicalDataLock) + { + this.historicalData.Clear(); + } + this.logger.LogInformation("Historical performance data cleared"); return Task.CompletedTask; } @@ -325,15 +372,47 @@ private void InitializeCpuCoreCounters() try { var coreCount = Environment.ProcessorCount; + + var useGroupAwareCategory = coreCount > LegacyProcessorCategoryInstanceLimit && + PerformanceCounterCategory.Exists(GroupAwareProcessorCategory); + for (int i = 0; i < coreCount; i++) { - tempCounters.Add(this.CreatePrimedCounter("Processor", "% Processor Time", i.ToString())); + var instanceName = useGroupAwareCategory + ? $"{i / LegacyProcessorCategoryInstanceLimit},{i % LegacyProcessorCategoryInstanceLimit}" + : i.ToString(); + var categoryName = useGroupAwareCategory + ? GroupAwareProcessorCategory + : "Processor"; + + try + { + tempCounters.Add(this.CreatePrimedCounter(categoryName, "% Processor Time", instanceName)); + } + catch (Exception ex) when (ex is InvalidOperationException or UnauthorizedAccessException) + { + this.logger.LogWarning( + ex, + "Skipping CPU core counter for instance '{Instance}' in category '{Category}'", + instanceName, + categoryName); + } + } + + if (tempCounters.Count == 0) + { + this.logger.LogWarning("No CPU core performance counters could be initialized"); + return; } this.cpuCoreCounters.Clear(); this.cpuCoreCounters.AddRange(tempCounters); - this.logger.LogInformation("Initialized {CoreCount} CPU core performance counters", coreCount); + this.logger.LogInformation( + "Initialized {CounterCount} of {CoreCount} CPU core performance counters (category: {Category})", + tempCounters.Count, + coreCount, + useGroupAwareCategory ? GroupAwareProcessorCategory : "Processor"); } catch (Exception ex) { @@ -356,14 +435,14 @@ private void InitializeCpuCoreCounters() private void EnsureSystemCountersInitialized() { - if (this.totalCpuCounter != null && this.memoryCounter != null) + if (this.disposed || (this.totalCpuCounter != null && this.memoryCounter != null)) { return; } lock (this.counterInitializationLock) { - if (this.totalCpuCounter != null && this.memoryCounter != null) + if (this.disposed || (this.totalCpuCounter != null && this.memoryCounter != null)) { return; } @@ -390,14 +469,14 @@ private void EnsureSystemCountersInitialized() private void EnsureCpuCoreCountersInitialized() { - if (this.cpuCoreCounters.Count > 0) + if (this.disposed || this.cpuCoreCounters.Count > 0) { return; } lock (this.counterInitializationLock) { - if (this.cpuCoreCounters.Count > 0) + if (this.disposed || this.cpuCoreCounters.Count > 0) { return; } @@ -474,17 +553,22 @@ private async Task GetTotalPhysicalMemoryAsync() { var scope = CreateCimv2ScopeWithTimeout(); using var searcher = new ManagementObjectSearcher(scope, new ObjectQuery("SELECT TotalPhysicalMemory FROM Win32_ComputerSystem")); - foreach (var obj in searcher.Get()) - { - var totalMemory = Convert.ToInt64(obj["TotalPhysicalMemory"]); - lock (this.totalMemoryCacheLock) + using var results = searcher.Get(); + foreach (var obj in results) + { + using (obj) { - this.cachedTotalPhysicalMemory = totalMemory; - this.totalPhysicalMemoryCacheUtc = DateTime.UtcNow; - } + var totalMemory = Convert.ToInt64(obj["TotalPhysicalMemory"]); - return totalMemory; + lock (this.totalMemoryCacheLock) + { + this.cachedTotalPhysicalMemory = totalMemory; + this.totalPhysicalMemoryCacheUtc = DateTime.UtcNow; + } + + return totalMemory; + } } return 0; @@ -511,7 +595,8 @@ private async Task GetActiveProcessCountAsync() { var scope = CreateCimv2ScopeWithTimeout(); using var searcher = new ManagementObjectSearcher(scope, new ObjectQuery("SELECT Count(*) AS Count FROM Win32_Process")); - var result = searcher.Get().Cast().FirstOrDefault(); + using var results = searcher.Get(); + using var result = results.Cast().FirstOrDefault(); var countValue = result?["Count"]; var count = countValue != null ? Convert.ToInt32(countValue) : 0; @@ -704,18 +789,28 @@ public void Dispose() return; } + this.disposed = true; + this.isMonitoring = false; + Interlocked.Exchange(ref this.monitoringStartedFlag, 0); + this.monitoringTimer?.Dispose(); + this.monitoringTimer = null; Interlocked.Exchange(ref this.isMonitoringTickInProgress, 0); - this.totalCpuCounter?.Dispose(); - this.memoryCounter?.Dispose(); - foreach (var counter in this.cpuCoreCounters) + lock (this.counterInitializationLock) { - counter?.Dispose(); - } + this.totalCpuCounter?.Dispose(); + this.totalCpuCounter = null; + this.memoryCounter?.Dispose(); + this.memoryCounter = null; - this.cpuCoreCounters.Clear(); - this.disposed = true; + foreach (var counter in this.cpuCoreCounters) + { + counter?.Dispose(); + } + + this.cpuCoreCounters.Clear(); + } } } } From 300fe4afe69645c8c9f2cb0b9d5cd2c090af81b6 Mon Sep 17 00:00:00 2001 From: Kool <79905997+eakkawut@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:44:10 +0700 Subject: [PATCH 5/7] fix: order autostart changes so a failure cannot leave no autostart EnableAutostartAsync removed the legacy HKCU Run entry before checking whether it could create the scheduled task. Without elevation it deleted the mechanism that was working and then bailed out, leaving the user with no autostart while the UI still reported it enabled. Elevation is now checked first, and the legacy entry is retired only once the scheduled task is confirmed created. DisableAutostartAsync removes the task before the legacy entry, and UpdateAutostartAsync calls enable directly, relying on schtasks /F to overwrite, instead of a disable-then-enable sequence that had no rollback. The requireAdministrator manifest, the startup elevation bootstrap and the scheduled-task definitions are unchanged. --- Services/AutostartService.cs | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/Services/AutostartService.cs b/Services/AutostartService.cs index a6ceb85..db205ce 100644 --- a/Services/AutostartService.cs +++ b/Services/AutostartService.cs @@ -14,8 +14,8 @@ public partial class AutostartService : IAutostartService private readonly ILogger logger; private readonly IElevationService elevationService; private readonly IElevatedTaskService elevatedTaskService; - private bool isAutostartEnabled; - private string? autostartPath; + private volatile bool isAutostartEnabled; + private volatile string? autostartPath; public event EventHandler? AutostartStatusChanged; @@ -50,9 +50,6 @@ public async Task EnableAutostartAsync(bool startMinimized = true) var arguments = this.GetAutostartArguments(startMinimized); var fullCommand = $"\"{executablePath}\" {arguments}"; - // Clean up legacy registry-based startup to keep a single elevated startup mechanism. - this.TryRemoveLegacyRegistryAutostart(); - if (!this.elevationService.IsRunningAsAdministrator()) { LogAutostartRequiresElevation(this.logger); @@ -68,6 +65,11 @@ public async Task EnableAutostartAsync(bool startMinimized = true) } var scheduledTaskCreated = await this.elevatedTaskService.EnsureAutostartTaskAsync(executablePath, arguments); + if (scheduledTaskCreated) + { + this.TryRemoveLegacyRegistryAutostart(); + } + if (!scheduledTaskCreated) { LogAutostartTaskRegistrationFailed(this.logger); @@ -113,8 +115,6 @@ public async Task DisableAutostartAsync() return false; } - this.TryRemoveLegacyRegistryAutostart(); - var scheduledTaskRemoved = await this.elevatedTaskService.RemoveAutostartTaskAsync(); if (!scheduledTaskRemoved) { @@ -122,6 +122,8 @@ public async Task DisableAutostartAsync() return false; } + this.TryRemoveLegacyRegistryAutostart(); + LogAutostartDisabled(this.logger); this.isAutostartEnabled = false; @@ -169,13 +171,6 @@ public async Task CheckAutostartStatusAsync() public async Task UpdateAutostartAsync(bool startMinimized = true) { - if (!this.isAutostartEnabled) - { - return await this.EnableAutostartAsync(startMinimized); - } - - // Re-enable with new parameters - await this.DisableAutostartAsync(); return await this.EnableAutostartAsync(startMinimized); } From d647aad62ad1d13a33cd75c7e515f82e2d870686 Mon Sep 17 00:00:00 2001 From: Kool <79905997+eakkawut@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:44:10 +0700 Subject: [PATCH 6/7] docs: record reliability hardening in the changelog Adds an Unreleased section covering the data-loss, lifecycle and threading fixes, with explicit Safety notes confirming the elevation model, the protected-process denylist, the priority guardrails and the affinity apply pipeline are unchanged. --- docs/CHANGELOG.md | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index f8ace56..ba0b080 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,6 +2,40 @@ All notable changes to this project are documented in this file. +## Unreleased - Reliability, lifecycle and threading hardening + +### Fixed + +- Saved process rules are no longer lost when the rules file cannot be read. A transient lock from antivirus, backup or a sync client used to pin an empty rule set for the rest of the session, so the next save overwrote every saved rule. A file that cannot be parsed is now preserved alongside the original for recovery. +- Pending edits in Settings are no longer discarded when something writes settings in the background, such as the startup update check recording its last-check time. +- Do Not Disturb now suppresses notifications during the configured quiet hours. The overnight comparison was inverted, so the default 22:00-08:00 window never suppressed anything while a daytime window suppressed everything. Turning Do Not Disturb on explicitly now takes effect regardless of the schedule, and the timed window raises its change notification when it expires. +- Default keyboard shortcuts are applied on a fresh installation. The fallback only ran when the stored shortcut list was null, which never happens, so no global hotkeys were registered. +- Closing the main window can no longer leave the window permanently unclosable when saving settings fails during shutdown. +- The last few seconds of diagnostics before exit are no longer lost. Application shutdown now releases the service container, which is what flushes buffered log entries and stops the WMI watchers and native performance counters. +- Stopping process monitoring during shutdown now actually stops it; disposal previously short-circuited itself and left the WMI watchers and the polling timer running. +- The tray Power Plans submenu now shows the active plan after a plan change instead of keeping the checkmark on whichever plan was active at startup. +- Per-core CPU readings work on systems with more than 64 logical processors by using the group-aware performance counter category, and a single unavailable core instance no longer clears the whole per-core view. +- Autostart no longer removes the existing startup entry before confirming the replacement was created, and updating autostart settings no longer disables it first. +- Notification delivery failures now retry as intended instead of being dropped. + +### Changed + +- Tray menu, tray tooltip and monitoring-status updates are marshalled to the UI thread. These are Windows Forms controls that were previously mutated from WMI and timer threads. +- Throttling, notification history, performance history and CPU counter state are guarded against concurrent access. +- View models owned by the main window are released on close, including the power plan refresh timer and subscriptions to long-lived services. +- Debug-logging state is cached instead of deep-cloning the settings model on every structured log call. +- A failed CPU Set topology probe is retried after a short delay instead of disabling CPU Sets for the rest of the session. + +### Safety + +- No change to the elevation model, the administrator-required manifest, the protected-process denylist, the Realtime priority block or the High priority warning. +- No change to how affinity, priority or memory priority are applied; the affinity apply pipeline and its fallback order are untouched. +- Power plan behaviour is unchanged. The tray now listens to the existing power-plan-changed notification rather than polling, so no additional `powercfg` calls are introduced. + +### Notes + +- Windows Management Instrumentation recovery still depends on fallback polling being enabled, which remains the default. + ## v1.5.2 - Process monitoring configuration ### Fixed From b7d31eabdbe77bfac26013893682b627755afefa Mon Sep 17 00:00:00 2001 From: Kool <79905997+eakkawut@users.noreply.github.com> Date: Thu, 6 Aug 2026 13:44:10 +0700 Subject: [PATCH 7/7] chore: ignore the local tmp working directory Keeps scratch notes and draft documents under /tmp out of the working tree status, alongside the other local-only artifact patterns. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7c9d08e..afbcbdb 100644 --- a/.gitignore +++ b/.gitignore @@ -193,3 +193,4 @@ gitleaks*.tar.gz *.orig *.rej *.xaml.bak +/tmp