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..e2d9813 100644 --- a/Services/PerformanceMonitoringService.cs +++ b/Services/PerformanceMonitoringService.cs @@ -21,7 +21,7 @@ public class PerformanceMonitoringService : IPerformanceMonitoringService, IDisp private readonly object counterInitializationLock = new(); private PerformanceCounter? totalCpuCounter; private PerformanceCounter? memoryCounter; - private readonly List cpuCoreCounters; + private readonly List<(int LogicalProcessorIndex, PerformanceCounter Counter)> cpuCoreCounters; private System.Threading.Timer? monitoringTimer; private readonly object totalMemoryCacheLock = new(); private readonly TimeSpan totalPhysicalMemoryCacheDuration = TimeSpan.FromMinutes(5); @@ -32,7 +32,10 @@ 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 int monitoringGeneration; private bool runtimeTelemetryInitialized; private int previousGen0Collections; private int previousGen1Collections; @@ -40,13 +43,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; @@ -63,7 +70,7 @@ public PerformanceMonitoringService( this.settingsService = settingsService; this.enhancedLoggingService = enhancedLoggingService; this.historicalData = new Queue(HistoricalDataCapacity); - this.cpuCoreCounters = new List(); + this.cpuCoreCounters = new List<(int LogicalProcessorIndex, PerformanceCounter Counter)>(); } public async Task GetSystemMetricsAsync(bool lightweight = false) @@ -99,12 +106,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,23 +135,15 @@ public async Task> GetCpuCoreUsageAsync() this.EnsureCpuCoreCountersInitialized(); var topology = await this.cpuTopologyService.DetectTopologyAsync().ConfigureAwait(false); - for (int i = 0; i < this.cpuCoreCounters.Count; i++) + (int LogicalProcessorIndex, PerformanceCounter Counter)[] counters; + lock (this.counterInitializationLock) { - var counter = this.cpuCoreCounters[i]; - var usage = counter.NextValue(); - - var coreUsage = new CpuCoreUsage - { - CoreId = i, - CoreName = $"Core {i}", - Usage = usage, - CoreType = DetermineCoreType(i, topology), - IsHyperThreaded = IsHyperThreadedCore(i, topology), - PhysicalCoreId = GetPhysicalCoreId(i, topology), - }; - - coreUsages.Add(coreUsage); + counters = this.cpuCoreCounters.ToArray(); } + + coreUsages.AddRange(BuildCpuCoreUsages( + counters.Select(counter => (counter.LogicalProcessorIndex, (double)counter.Counter.NextValue())), + topology)); } catch (Exception ex) { @@ -151,6 +153,19 @@ public async Task> GetCpuCoreUsageAsync() return coreUsages; } + internal static List BuildCpuCoreUsages( + IEnumerable<(int LogicalProcessorIndex, double Usage)> samples, + CpuTopologyModel? topology) => + samples.Select(sample => new CpuCoreUsage + { + CoreId = sample.LogicalProcessorIndex, + CoreName = $"Core {sample.LogicalProcessorIndex}", + Usage = sample.Usage, + CoreType = DetermineCoreType(sample.LogicalProcessorIndex, topology), + IsHyperThreaded = IsHyperThreadedCore(sample.LogicalProcessorIndex, topology), + PhysicalCoreId = GetPhysicalCoreId(sample.LogicalProcessorIndex, topology), + }).ToList(); + public async Task GetMemoryUsageAsync() { try @@ -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,21 +278,31 @@ 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"); this.isMonitoring = true; - Interlocked.Exchange(ref this.isMonitoringTickInProgress, 0); + var generation = Interlocked.Increment(ref this.monitoringGeneration); // PERFORMANCE OPTIMIZATION: Increased interval from 1s to 2s for better performance this.monitoringTimer = new System.Threading.Timer( async _ => { + if (generation != Volatile.Read(ref this.monitoringGeneration)) + { + return; + } + if (Interlocked.Exchange(ref this.isMonitoringTickInProgress, 1) == 1) { return; @@ -273,9 +310,17 @@ public async Task StartMonitoringAsync() try { + if (this.disposed || !this.isMonitoring || generation != Volatile.Read(ref this.monitoringGeneration)) + { + return; + } + var metrics = await this.GetSystemMetricsAsync().ConfigureAwait(false); await this.EmitGcDiagnosticsIfNeededAsync(metrics).ConfigureAwait(false); - this.MetricsUpdated?.Invoke(this, new PerformanceMetricsUpdatedEventArgs(metrics)); + if (generation == Volatile.Read(ref this.monitoringGeneration)) + { + this.MetricsUpdated?.Invoke(this, new PerformanceMetricsUpdatedEventArgs(metrics)); + } } catch (Exception ex) { @@ -285,19 +330,21 @@ 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; } this.logger.LogInformation("Stopping performance monitoring"); this.isMonitoring = false; - Interlocked.Exchange(ref this.isMonitoringTickInProgress, 0); + Interlocked.Increment(ref this.monitoringGeneration); this.monitoringTimer?.Dispose(); this.monitoringTimer = null; @@ -307,33 +354,73 @@ 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; } private void InitializeCpuCoreCounters() { - var tempCounters = new List(); + var tempCounters = new List<(int LogicalProcessorIndex, PerformanceCounter Counter)>(); 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((i, 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) { @@ -342,7 +429,7 @@ private void InitializeCpuCoreCounters() { try { - counter.Dispose(); + counter.Counter.Dispose(); } catch { @@ -356,14 +443,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 +477,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 +561,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 +603,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 +797,28 @@ public void Dispose() return; } + this.disposed = true; + this.isMonitoring = false; + Interlocked.Exchange(ref this.monitoringStartedFlag, 0); + Interlocked.Increment(ref this.monitoringGeneration); + this.monitoringTimer?.Dispose(); - Interlocked.Exchange(ref this.isMonitoringTickInProgress, 0); - this.totalCpuCounter?.Dispose(); - this.memoryCounter?.Dispose(); + this.monitoringTimer = null; - 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.Counter.Dispose(); + } + + this.cpuCoreCounters.Clear(); + } } } } diff --git a/Services/PersistentProcessRuleJsonStore.cs b/Services/PersistentProcessRuleJsonStore.cs index 0811312..6cc51a2 100644 --- a/Services/PersistentProcessRuleJsonStore.cs +++ b/Services/PersistentProcessRuleJsonStore.cs @@ -16,9 +16,12 @@ public sealed class PersistentProcessRuleJsonStore : IPersistentProcessRuleStore }; private readonly Func filePathProvider; + private readonly Action copyFile; private readonly ILogger? logger; private readonly SemaphoreSlim cacheLock = new(1, 1); private volatile IReadOnlyList? cachedRules; + private bool loadFailed; + private bool preservationRequired; public PersistentProcessRuleJsonStore(ILogger? logger = null) : this(() => StoragePaths.PersistentRulesFilePath, logger) @@ -27,10 +30,12 @@ public PersistentProcessRuleJsonStore(ILogger? l internal PersistentProcessRuleJsonStore( Func filePathProvider, - ILogger? logger = null) + ILogger? logger = null, + Action? copyFile = null) { this.filePathProvider = filePathProvider ?? throw new ArgumentNullException(nameof(filePathProvider)); this.logger = logger; + this.copyFile = copyFile ?? ((source, destination) => File.Copy(source, destination, overwrite: false)); } public async Task> LoadAsync() @@ -53,6 +58,8 @@ public async Task> LoadAsync() if (!File.Exists(filePath)) { this.logger?.LogDebug("Persistent process rules file does not exist at {FilePath}", filePath); + this.loadFailed = false; + this.preservationRequired = false; return this.cachedRules = []; } @@ -61,12 +68,16 @@ 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; + this.preservationRequired = 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.preservationRequired = !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,11 +95,30 @@ 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.preservationRequired && File.Exists(filePath)) + { + if (!this.TryPreserveUnreadableFile(filePath, null)) + { + throw new IOException($"Cannot save persistent process rules because the existing file could not be preserved: {filePath}"); + } + + this.preservationRequired = false; + } + + 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); await AtomicFileWriter.WriteAllTextAsync(filePath, json).ConfigureAwait(false); this.cachedRules = rules.ToArray(); + this.loadFailed = false; this.logger?.LogDebug("Saved {RuleCount} persistent process rules to {FilePath}", rules.Count, filePath); } catch (Exception ex) when (ex is JsonException or IOException or UnauthorizedAccessException) @@ -102,5 +132,25 @@ public async Task SaveAsync(IReadOnlyList rules) this.cacheLock.Release(); } } + + private bool TryPreserveUnreadableFile(string filePath, Exception? readException) + { + var backupPath = $"{filePath}.unreadable.{DateTime.UtcNow:yyyyMMddHHmmssfff}.{Guid.NewGuid():N}"; + + try + { + this.copyFile(filePath, backupPath); + this.logger?.LogWarning( + readException, + "Preserved unreadable persistent process rules file as {BackupPath}", + backupPath); + return true; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + this.logger?.LogDebug(ex, "Could not preserve unreadable persistent process rules file {FilePath}", filePath); + return false; + } + } } } 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/PerformanceMonitoringServiceTests.cs b/Tests/ThreadPilot.Core.Tests/PerformanceMonitoringServiceTests.cs new file mode 100644 index 0000000..fbe376b --- /dev/null +++ b/Tests/ThreadPilot.Core.Tests/PerformanceMonitoringServiceTests.cs @@ -0,0 +1,45 @@ +namespace ThreadPilot.Core.Tests +{ + using System.Reflection; + using Microsoft.Extensions.Logging.Abstractions; + using Moq; + using ThreadPilot.Services; + + public sealed class PerformanceMonitoringServiceTests + { + [Fact] + public void BuildCpuCoreUsages_PreservesLogicalProcessorIdsAcrossMissingCounters() + { + var usages = PerformanceMonitoringService.BuildCpuCoreUsages( + [(0, 10d), (3, 30d)], + topology: null); + + Assert.Equal([0, 3], usages.Select(usage => usage.CoreId)); + Assert.Equal(["Core 0", "Core 3"], usages.Select(usage => usage.CoreName)); + } + + [Fact] + public async Task StopStart_DoesNotClearAnInFlightTickGuard() + { + using var service = CreateService(); + var tickField = typeof(PerformanceMonitoringService).GetField( + "isMonitoringTickInProgress", + BindingFlags.Instance | BindingFlags.NonPublic)!; + await service.StartMonitoringAsync(); + tickField.SetValue(service, 1); + + await service.StopMonitoringAsync(); + await service.StartMonitoringAsync(); + + Assert.Equal(1, tickField.GetValue(service)); + } + + private static PerformanceMonitoringService CreateService() => + new( + NullLogger.Instance, + new Mock().Object, + new Mock().Object, + new Mock().Object, + new Mock().Object); + } +} diff --git a/Tests/ThreadPilot.Core.Tests/PersistentProcessRuleJsonStoreTests.cs b/Tests/ThreadPilot.Core.Tests/PersistentProcessRuleJsonStoreTests.cs index b1520e7..95bf867 100644 --- a/Tests/ThreadPilot.Core.Tests/PersistentProcessRuleJsonStoreTests.cs +++ b/Tests/ThreadPilot.Core.Tests/PersistentProcessRuleJsonStoreTests.cs @@ -224,6 +224,139 @@ 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 = Assert.Single(Directory.GetFiles(Path.GetDirectoryName(filePath)!, "rules.json.unreadable*")); + Assert.Equal(OriginalContent, await File.ReadAllTextAsync(backupPath)); + } + finally + { + DeleteFile(filePath); + } + } + + [Fact] + public async Task SaveAsync_WhenUnreadableFileCannotBePreserved_DoesNotOverwriteOriginal() + { + var filePath = CreateTemporaryFilePath(); + Directory.CreateDirectory(Path.GetDirectoryName(filePath)!); + const string OriginalContent = "{ locked user rules"; + await File.WriteAllTextAsync(filePath, OriginalContent); + var store = new PersistentProcessRuleJsonStore( + () => filePath, + copyFile: (_, _) => throw new IOException("Recovery destination unavailable")); + + try + { + Assert.Empty(await store.LoadAsync()); + + await Assert.ThrowsAsync(() => + store.SaveAsync([CreateRule("replacement", "Replacement.exe", ProcessPriorityClass.High)])); + Assert.Equal(OriginalContent, await File.ReadAllTextAsync(filePath)); + } + finally + { + DeleteFile(filePath); + } + } + + [Fact] + public async Task LoadAsync_WithExistingRecoveryFile_CreatesANewRecoveryCopy() + { + var filePath = CreateTemporaryFilePath(); + Directory.CreateDirectory(Path.GetDirectoryName(filePath)!); + const string CurrentContent = "{ current user rules"; + await File.WriteAllTextAsync(filePath, CurrentContent); + await File.WriteAllTextAsync(filePath + ".unreadable", "stale recovery"); + var store = new PersistentProcessRuleJsonStore(() => filePath); + + try + { + Assert.Empty(await store.LoadAsync()); + + var recoveryFiles = Directory.GetFiles(Path.GetDirectoryName(filePath)!, "rules.json.unreadable*"); + Assert.Equal(2, recoveryFiles.Length); + Assert.Contains(recoveryFiles, path => File.ReadAllText(path) == CurrentContent); + } + finally + { + DeleteFile(filePath); + } + } + + [Fact] + public async Task SaveAsync_RetriesRecoveryAfterTransientCopyFailure() + { + var filePath = CreateTemporaryFilePath(); + Directory.CreateDirectory(Path.GetDirectoryName(filePath)!); + const string OriginalContent = "{ temporarily locked user rules"; + await File.WriteAllTextAsync(filePath, OriginalContent); + var copyAttempts = 0; + var store = new PersistentProcessRuleJsonStore( + () => filePath, + copyFile: (source, destination) => + { + if (Interlocked.Increment(ref copyAttempts) == 1) + { + throw new IOException("Transient copy failure"); + } + + File.Copy(source, destination); + }); + + try + { + Assert.Empty(await store.LoadAsync()); + + await store.SaveAsync([CreateRule("replacement", "Replacement.exe", ProcessPriorityClass.High)]); + + Assert.Equal(2, copyAttempts); + Assert.Contains( + Directory.GetFiles(Path.GetDirectoryName(filePath)!, "rules.json.unreadable*"), + path => File.ReadAllText(path) == OriginalContent); + } + 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/ProcessViewXamlBindingTests.cs b/Tests/ThreadPilot.Core.Tests/ProcessViewXamlBindingTests.cs index 540ae83..e45cf43 100644 --- a/Tests/ThreadPilot.Core.Tests/ProcessViewXamlBindingTests.cs +++ b/Tests/ThreadPilot.Core.Tests/ProcessViewXamlBindingTests.cs @@ -183,18 +183,39 @@ public void ProcessGridContextMenu_ContainsExpectedActionsAndSubmenus() } [Fact] - public void ProcessToolbar_ExposesLockProcessListToggle() - { + public void ProcessToolbar_ExposesLockProcessListToggle() + { var document = XDocument.Load(ProcessViewPath, LoadOptions.PreserveWhitespace); var serialized = document.ToString(SaveOptions.DisableFormatting); Assert.Contains("ProcessView_LockList", serialized, StringComparison.Ordinal); Assert.Contains("IsChecked=\"{Binding IsProcessListLocked}\"", serialized, StringComparison.Ordinal); - Assert.Contains("ProcessView_LockListTooltip", serialized, StringComparison.Ordinal); - } - - [Fact] - public void MasksView_SelectedCpuTilesUseSubtleMaskSelectionResources() + Assert.Contains("ProcessView_LockListTooltip", serialized, StringComparison.Ordinal); + } + + [Fact] + public void MonitoringDisabledOverlay_CoversProcessControlsWithSinglePrimaryMessage() + { + var document = XDocument.Load(ProcessViewPath, LoadOptions.PreserveWhitespace); + var overlay = Assert.Single( + document.Descendants(), + element => element.Attributes().Any(attribute => + attribute.Name.LocalName == "Name" && + attribute.Value == "MonitoringDisabledOverlay")); + var serialized = overlay.ToString(SaveOptions.DisableFormatting); + + Assert.Equal("Border", overlay.Name.LocalName); + Assert.Contains("Grid.RowSpan=\"3\"", serialized, StringComparison.Ordinal); + Assert.Contains("Panel.ZIndex=\"10\"", serialized, StringComparison.Ordinal); + Assert.Contains("Background=\"{DynamicResource SurfaceMutedBrush}\"", serialized, StringComparison.Ordinal); + Assert.Contains("Foreground=\"{DynamicResource TextPrimaryBrush}\"", serialized, StringComparison.Ordinal); + Assert.Contains("InverseBoolToVisibilityConverter", serialized, StringComparison.Ordinal); + Assert.Single(overlay.Descendants(), element => element.Name.LocalName == "TextBlock"); + Assert.DoesNotContain("ProcessView_MonitoringDisabledDescription", serialized, StringComparison.Ordinal); + } + + [Fact] + public void MasksView_SelectedCpuTilesUseSubtleMaskSelectionResources() { var masksViewPath = Path.Combine( GetRepositoryRoot(), diff --git a/Tests/ThreadPilot.Core.Tests/SettingsViewModelThemeTests.cs b/Tests/ThreadPilot.Core.Tests/SettingsViewModelThemeTests.cs index 81760ab..eb5a91a 100644 --- a/Tests/ThreadPilot.Core.Tests/SettingsViewModelThemeTests.cs +++ b/Tests/ThreadPilot.Core.Tests/SettingsViewModelThemeTests.cs @@ -152,6 +152,28 @@ public async Task SaveSettingsCommand_PersistsSelectedLanguage() Assert.False(viewModel.HasUnsavedChanges); } + [Fact] + public async Task SaveSettingsCommand_MergesPendingEditWithExternalUpdate() + { + var harness = new Harness(); + ApplicationSettingsModel? savedSettings = null; + harness.SettingsService + .Setup(service => service.UpdateSettingsAsync(It.IsAny())) + .Callback(settings => savedSettings = (ApplicationSettingsModel)settings.Clone()) + .Returns(Task.CompletedTask); + var viewModel = harness.CreateViewModel(); + viewModel.Settings.Language = "it-IT"; + var externalUpdate = (ApplicationSettingsModel)harness.PersistedSettings.Clone(); + externalUpdate.LastUpdateCheckUtc = DateTimeOffset.UtcNow; + + viewModel.ApplyPersistedSettings(externalUpdate); + await ((IAsyncRelayCommand)viewModel.SaveSettingsCommand).ExecuteAsync(null); + + Assert.NotNull(savedSettings); + Assert.Equal("it-IT", savedSettings.Language); + Assert.Equal(externalUpdate.LastUpdateCheckUtc, savedSettings.LastUpdateCheckUtc); + } + [Fact] public async Task NavigationPrompt_SavePersistsPendingSettingsBeforeNavigating() { @@ -246,6 +268,8 @@ private sealed class Harness public ActivityAuditService Audit { get; } = new(NullLogger.Instance); + public ApplicationSettingsModel PersistedSettings => this.settings; + public Harness(bool initialDarkTheme = false) { this.settings = new ApplicationSettingsModel 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..4608906 100644 --- a/ViewModels/SettingsViewModel.cs +++ b/ViewModels/SettingsViewModel.cs @@ -8,6 +8,7 @@ namespace ThreadPilot.ViewModels using System.Reflection; using System.Text; using System.Text.Json; + using System.Text.Json.Nodes; using System.Threading.Tasks; using System.Windows; using System.Windows.Input; @@ -621,26 +622,67 @@ 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.ApplyPersistedSettings(e.NewSettings)); + } + + internal void ApplyPersistedSettings(ApplicationSettingsModel newSettings) + { + this.isSyncingFromService = true; + try { - this.isSyncingFromService = true; - try + var persistedSettings = (ApplicationSettingsModel)newSettings.Clone(); + if (!string.IsNullOrWhiteSpace(this.cachedDefaultPowerPlanGuid)) { - this.Settings.CopyFrom(e.NewSettings); - if (!string.IsNullOrWhiteSpace(this.cachedDefaultPowerPlanGuid)) - { - this.Settings.DefaultPowerPlanId = this.cachedDefaultPowerPlanGuid; - this.Settings.DefaultPowerPlanName = this.cachedDefaultPowerPlanName; - } - this.SetSavedSettingsSnapshot(this.Settings); - this.ApplyLanguagePreference(this.Settings.Language, logUserAction: false); - this.StatusMessage = this.GetLocalizedString("Settings_StatusSynchronized", "Settings synchronized"); + persistedSettings.DefaultPowerPlanId = this.cachedDefaultPowerPlanGuid; + persistedSettings.DefaultPowerPlanName = this.cachedDefaultPowerPlanName; } - finally + + if (this.HasUnsavedChanges) { - this.isSyncingFromService = false; + var mergedSettings = MergeSettings(this.savedSettingsSnapshot, this.Settings, persistedSettings); + this.Settings.CopyFrom(mergedSettings); + 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"); + } + finally + { + this.isSyncingFromService = false; + } + } + + internal static ApplicationSettingsModel MergeSettings( + ApplicationSettingsModel previousPersisted, + ApplicationSettingsModel locallyEdited, + ApplicationSettingsModel newlyPersisted) + { + var previousJson = JsonSerializer.SerializeToNode(previousPersisted)!.AsObject(); + var localJson = JsonSerializer.SerializeToNode(locallyEdited)!.AsObject(); + var mergedJson = JsonSerializer.SerializeToNode(newlyPersisted)!.AsObject(); + + foreach (var property in localJson) + { + if (!JsonNode.DeepEquals(property.Value, previousJson[property.Key])) + { + mergedJson[property.Key] = property.Value?.DeepClone(); + } + } + + return mergedJson.Deserialize()!; + } + + protected override void OnDispose() + { + this.Settings.PropertyChanged -= this.OnSettingsPropertyChanged; + this.settingsService.SettingsChanged -= this.OnSettingsServiceSettingsChanged; + base.OnDispose(); } private async Task RefreshPowerPlansAsync() diff --git a/Views/ProcessView.xaml b/Views/ProcessView.xaml index f180e6e..bff1eaa 100644 --- a/Views/ProcessView.xaml +++ b/Views/ProcessView.xaml @@ -400,25 +400,6 @@ - - - - - - - @@ -594,6 +575,24 @@ + + + +