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 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/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/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/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); } 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/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/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(); + } } } } 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/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/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/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/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/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/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/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); 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/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 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(); } 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