From 0bc192cedb8205d3d34c7e0be01a46507f76f2b1 Mon Sep 17 00:00:00 2001 From: laurentiu021 Date: Thu, 6 Aug 2026 16:26:09 +0300 Subject: [PATCH] fix: serialize the Resource History reloads and handle an unreadable history file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Bandwidth Monitor's concurrent-reload race was fixed in 1.57.3, but ResourceHistoryViewModel carried the identical defect and was missed: three entry points (the constructor's InitializeAsync, the fire-and-forget in OnSelectedRangeChanged, and the Refresh command) each call ReplaceWith on five buffers LiveCharts observes, with no gate. Its CollectionDeepObserver updates a HashSet from the change notification, so a second thread arriving mid-notification corrupts it. Same SemaphoreSlim gate as the sibling, disposed with the VM. Both history services also caught only IOException around their file access. UnauthorizedAccessException is a sibling of IOException, not a subclass, so a permission error escaped — and because these run in a background sampler the user never invoked, that surfaced as a failure with no user action behind it. On the bandwidth side it was worse: the caller sets _historyOwnsChart before awaiting and only hands it back on OperationCanceledException, so an escaping access error would have frozen the live chart permanently. ResourceHistoryService's paths were static readonly over SpecialFolder.LocalApplicationData, which resolves through the Win32 known-folder API and ignores the LOCALAPPDATA environment variable — so the service could not be pointed anywhere else by a test or even a child process, and its load, prune and retention paths had no coverage at all. It now takes the same optional configDir seam as BandwidthHistoryService and the other seven persistence services, which is what let the regression tests below exist. Verification: the access-denied case was proven red before the fix and green after, using a deny-read ACL (a directory in the file's place does NOT reproduce it, since File.Exists returns false and the load guard short-circuits) — 10 pass / 1 fail before, 11 / 0 after, failing with the expected UnauthorizedAccessException. The reload gate cannot be reproduced headlessly on either VM: LiveCharts only attaches its deep observer to a rendered chart, so the corruption needs a live UI. It is justified by CI's proven failure on the structurally identical sibling, and the test asserts the reachable invariant instead of claiming a repro. DI resolution and the singleton contract were re-verified, and all four projects rebuild with 0 warnings. --- ARCHITECTURE.md | 4 +- CHANGELOG.md | 6 + .../ResourceHistoryServiceTests.cs | 185 ++++++++++++++++++ .../ResourceHistoryViewModelTests.cs | 115 +++++++++++ .../Services/BandwidthHistoryService.cs | 7 + .../Services/ResourceHistoryService.cs | 61 ++++-- SysManager/SysManager/SysManager.csproj | 6 +- .../ViewModels/ResourceHistoryViewModel.cs | 20 +- 8 files changed, 378 insertions(+), 26 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 65ca5337..4da803b2 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -421,7 +421,9 @@ Key services: every 10s as append-only NDJSON in `%LocalAppData%\SysManager\resource-history.ndjson`, with 7/14/30-day retention (periodic prune). Reuses `SystemInfoService` + NvAPIWrapper + `TemperatureService`; serialize/parse/prune/downsample/CSV are pure, unit-tested static - helpers. Strictly local — no system writes, nothing leaves the machine. + helpers, and the directory is injectable — like `BandwidthHistoryService` — so tests cover the + load and retention paths without touching the user's own history. Strictly local — no system + writes, nothing leaves the machine. - Bandwidth Monitor sources — `IBandwidthMonitorService` is the seam with two implementations: `ConnectionBandwidthSource` (default, no admin) sums `NetworkInterface` byte counters for total throughput and reads the extended TCP/UDP tables via iphlpapi P/Invoke (`GetExtendedTcpTable`/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 280158ff..bd229939 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project are documented here. The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.57.4] - 2026-08-06 + +### Fixed +- **Switching the Resource History chart between ranges no longer risks an error.** Picking a different period while another one was still loading could have both of them rebuild the five graphs at the same time, which could fail instead of drawing. Only one rebuild runs at a time now. This is the same problem that was fixed on the Bandwidth Monitor in 1.57.3 — Resource History had it too, and it was missed then. +- **An unreadable history file no longer crashes the recorder.** Both the Resource History and Bandwidth Monitor recorders handled a disk error while reading or writing their history, but not a permission error — and a locked-down or read-only file produces the second, not the first. Because the recording runs quietly in the background, that surfaced as the app failing over something the user never started. Both now log it and carry on with an empty chart, exactly as they already did for a disk error. + ## [1.57.3] - 2026-08-06 ### Fixed diff --git a/SysManager/SysManager.Tests/ResourceHistoryServiceTests.cs b/SysManager/SysManager.Tests/ResourceHistoryServiceTests.cs index 7e2435e0..e0fb2356 100644 --- a/SysManager/SysManager.Tests/ResourceHistoryServiceTests.cs +++ b/SysManager/SysManager.Tests/ResourceHistoryServiceTests.cs @@ -2,6 +2,12 @@ // Author: laurentiu021 · https://github.com/laurentiu021/SystemManager // License: MIT +using System; +using System.IO; +using System.Linq; +using System.Security.AccessControl; +using System.Security.Principal; +using System.Threading.Tasks; using SysManager.Models; using SysManager.Services; @@ -212,3 +218,182 @@ public void ToCsv_UsesInvariantDecimalSeparator() public void RetentionOptions_AreSevenFourteenThirty() => Assert.Equal([7, 14, 30], ResourceHistoryService.RetentionOptions); } + +/// +/// Disk-backed tests for , using the injected temp directory +/// so the developer's own history in %LOCALAPPDATA% is never read or written. +/// These could not exist before: the service pinned its paths to +/// in static readonly fields, and +/// that resolves through the Win32 known-folder API — it ignores the LOCALAPPDATA environment +/// variable, so not even a child process could redirect it. Every test above had to stay on the pure +/// helpers as a result, leaving the load/prune/retention paths uncovered. +/// +public class ResourceHistoryServiceDiskTests : IDisposable +{ + private readonly string _dir; + + public ResourceHistoryServiceDiskTests() + { + _dir = Path.Combine(Path.GetTempPath(), "SysManagerResourceHistoryTests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_dir); + } + + public void Dispose() + { + try { if (Directory.Exists(_dir)) Directory.Delete(_dir, recursive: true); } + catch (IOException) { /* a leftover temp dir must never fail a test run */ } + GC.SuppressFinalize(this); + } + + // skipHardwareInit: none of these tests read a sensor, and probing real hardware in a unit + // test would make it environment-dependent. + private ResourceHistoryService NewService() => new( + new SystemInfoService(), + new TemperatureService(new DiskHealthService(), skipHardwareInit: true), + _dir); + + private string DataPath => Path.Combine(_dir, "resource-history.ndjson"); + + private void Seed(params ResourceSample[] samples) + => File.WriteAllLines(DataPath, samples.Select(ResourceHistoryService.Serialize)); + + private static ResourceSample At(DateTime t, double cpu = 10) + => new(t, cpu, 20, null, null, null); + + // ── The seam itself ───────────────────────────────────────────────────── + + [Fact] + public async Task LoadAsync_WithNoFile_ReturnsEmpty() + { + using var service = NewService(); + Assert.Empty(await service.LoadAsync(TimeSpan.FromDays(7))); + } + + [Fact] + public async Task LoadAsync_ReadsOnlyItsOwnDirectory() + { + var now = DateTime.Now; + Seed(At(now.AddMinutes(-1))); + + using var mine = NewService(); + Assert.Single(await mine.LoadAsync(TimeSpan.FromHours(1))); + + // A different directory must be independently empty — proof the path is genuinely injected + // and not silently falling back to the shared profile location. + var other = Path.Combine(_dir, "other"); + Directory.CreateDirectory(other); + using var elsewhere = new ResourceHistoryService( + new SystemInfoService(), + new TemperatureService(new DiskHealthService(), skipHardwareInit: true), + other); + Assert.Empty(await elsewhere.LoadAsync(TimeSpan.FromDays(30))); + } + + // ── Load: range filtering and ordering ────────────────────────────────── + + [Fact] + public async Task LoadAsync_ExcludesSamplesOlderThanTheRange() + { + var now = DateTime.Now; + Seed( + At(now.AddDays(-3)), // outside a 1-hour range + At(now.AddMinutes(-30)), // inside + At(now.AddMinutes(-5))); // inside + + using var service = NewService(); + var loaded = await service.LoadAsync(TimeSpan.FromHours(1)); + + Assert.Equal(2, loaded.Count); + } + + [Fact] + public async Task LoadAsync_ReturnsOldestFirst() + { + // The file is append-only and time-ordered, and LoadAsync walks it backwards then reverses. + // Chart code depends on this order, so it is asserted rather than assumed. + var now = DateTime.Now; + Seed(At(now.AddMinutes(-30), cpu: 1), At(now.AddMinutes(-20), cpu: 2), At(now.AddMinutes(-10), cpu: 3)); + + using var service = NewService(); + var loaded = await service.LoadAsync(TimeSpan.FromHours(1)); + + Assert.Equal([1d, 2d, 3d], loaded.Select(s => s.CpuPercent)); + } + + [Fact] + public async Task LoadAsync_SkipsMalformedLinesWithoutFailing() + { + var now = DateTime.Now; + File.WriteAllLines(DataPath, + [ + "not json at all", + ResourceHistoryService.Serialize(At(now.AddMinutes(-10))), + "{ truncated", + ]); + + using var service = NewService(); + var loaded = await service.LoadAsync(TimeSpan.FromHours(1)); + + Assert.Single(loaded); + } + + [Fact] + public async Task LoadAsync_WhenTheFileCannotBeRead_ReturnsEmptyRatherThanThrowing() + { + // UnauthorizedAccessException is a SIBLING of IOException, not a subclass, so it escaped the + // service's original single `catch (IOException)`. That matters because the history file is + // read by a background sampler the user never invoked: an unhandled throw there is a crash + // with no action that caused it. A deny-read ACL is what actually produces it — a directory + // in the file's place does not, because File.Exists returns false and the guard short-circuits. + Seed(At(DateTime.Now.AddMinutes(-1))); + + var identity = WindowsIdentity.GetCurrent().User; + if (identity is null) return; // no SID to deny — nothing to assert on this host + + var info = new FileInfo(DataPath); + var acl = info.GetAccessControl(); + var deny = new FileSystemAccessRule(identity, FileSystemRights.Read, AccessControlType.Deny); + acl.AddAccessRule(deny); + info.SetAccessControl(acl); + try + { + using var service = NewService(); + Assert.Empty(await service.LoadAsync(TimeSpan.FromDays(7))); + } + finally + { + // Remove the deny rule, or Dispose cannot delete the temp directory. + acl.RemoveAccessRule(deny); + info.SetAccessControl(acl); + } + } + + // ── Retention persistence ─────────────────────────────────────────────── + + [Fact] + public void RetentionDays_DefaultsToSeven() + { + using var service = NewService(); + Assert.Equal(7, service.RetentionDays); + } + + [Fact] + public void RetentionDays_PersistsAcrossInstances() + { + using (var first = NewService()) + first.RetentionDays = 30; + + // A second instance, as after an app restart — the point of persisting at all. + using var second = NewService(); + Assert.Equal(30, second.RetentionDays); + Assert.True(File.Exists(Path.Combine(_dir, "resource-history-config.json"))); + } + + [Fact] + public void RetentionDays_RejectsAValueOutsideTheOfferedOptions() + { + using var service = NewService(); + service.RetentionDays = 999; + Assert.Equal(7, service.RetentionDays); + } +} diff --git a/SysManager/SysManager.Tests/ResourceHistoryViewModelTests.cs b/SysManager/SysManager.Tests/ResourceHistoryViewModelTests.cs index d88116b9..dcfbef31 100644 --- a/SysManager/SysManager.Tests/ResourceHistoryViewModelTests.cs +++ b/SysManager/SysManager.Tests/ResourceHistoryViewModelTests.cs @@ -2,7 +2,12 @@ // Author: laurentiu021 · https://github.com/laurentiu021/SystemManager // License: MIT +using System; +using System.IO; +using System.Linq; +using System.Threading.Tasks; using SysManager.Models; +using SysManager.Services; using SysManager.ViewModels; namespace SysManager.Tests; @@ -42,3 +47,113 @@ public void BuildSummary_WithTemps_IncludesPeakTemp() Assert.Contains("CPU temp peak 72°C", summary); } } + +/// +/// Tests that drive the real reload path, which needs a pointed +/// at a temp directory — only possible since that service gained the injectable configDir seam. +/// +public class ResourceHistoryViewModelReloadTests : IDisposable +{ + private readonly string _dir; + + public ResourceHistoryViewModelReloadTests() + { + _dir = Path.Combine(Path.GetTempPath(), "SysManagerResourceHistoryVmTests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(_dir); + } + + public void Dispose() + { + try { if (Directory.Exists(_dir)) Directory.Delete(_dir, recursive: true); } + catch (IOException) { /* a leftover temp dir must never fail a test run */ } + GC.SuppressFinalize(this); + } + + private ResourceHistoryService SeededService(params ResourceSample[] samples) + { + File.WriteAllLines( + Path.Combine(_dir, "resource-history.ndjson"), + samples.Select(ResourceHistoryService.Serialize)); + return new ResourceHistoryService( + new SystemInfoService(), + // skipHardwareInit: the reload path reads no sensor; probing real hardware would make + // this test depend on the machine it runs on. + new TemperatureService(new DiskHealthService(), skipHardwareInit: true), + _dir); + } + + [Fact] + public async Task ConcurrentReloadsDoNotCorruptTheChartSeries() + { + // Regression pin for the same defect CI proved on BandwidthMonitorViewModel, which this VM + // still carried: THREE entry points can run ReloadAsync at once — the constructor's + // InitializeAsync, the fire-and-forget in OnSelectedRangeChanged, and the Refresh command — + // and each calls ReplaceWith on five buffers LiveCharts observes. Its CollectionDeepObserver + // updates a HashSet from the change notification, so a second thread arriving mid-notification + // corrupts it ("Operations that change non-concurrent collections must have exclusive access"). + // + // WHAT THIS TEST CAN AND CANNOT PROVE: the corruption needs a RENDERED chart for LiveCharts to + // attach its observer, which never happens in a headless test — so this asserts the reachable + // invariant (the series stay coherent, nothing escapes) rather than reproducing the observer + // corruption. The gate is justified by the proven CI failure on the identical sibling. + var now = DateTime.Now; + using var service = SeededService( + new ResourceSample(now.AddMinutes(-30), 10, 20, 30, 40, 50), + new ResourceSample(now.AddMinutes(-20), 15, 25, 35, 45, 55), + new ResourceSample(now.AddMinutes(-10), 20, 30, 40, 50, 60)); + using var vm = new ResourceHistoryViewModel(service); + await vm.InitializationComplete; + + // Rapid range assignment starts a reload from the changed-handler each time, bypassing the + // command — that is what genuinely overlaps. + for (int i = 0; i < 20; i++) + vm.SelectedRange = vm.RangeOptions[i % vm.RangeOptions.Count]; + + // Fire the command mid-flight: the second, independent entry point. + var refresh = vm.ReloadCommand.ExecuteAsync(null); + for (int i = 0; i < 20; i++) + vm.SelectedRange = vm.RangeOptions[(i + 1) % vm.RangeOptions.Count]; + await refresh; + + // All three usage series are rebuilt from the same downsampled points, so their lengths must + // agree. A torn ReplaceWith is exactly what makes them diverge. + var lengths = vm.UsageSeries + .Select(s => ((System.Collections.IEnumerable)s.Values!).Cast().Count()) + .Distinct() + .ToList(); + Assert.Single(lengths); + } + + [Fact] + public async Task ARedundantRefreshOnTheSameRangeStillShowsThatRange() + { + // The gate drops nothing user-visible: a second reload for the already-selected range still + // ends on that range, with its samples counted, and releases the progress bar. + var now = DateTime.Now; + using var service = SeededService( + new ResourceSample(now.AddMinutes(-20), 10, 20, null, null, null), + new ResourceSample(now.AddMinutes(-10), 20, 30, null, null, null)); + using var vm = new ResourceHistoryViewModel(service); + await vm.InitializationComplete; + + vm.SelectedRange = vm.RangeOptions.First(r => r.Range == TimeSpan.FromHours(1)); + await vm.ReloadCommand.ExecuteAsync(null); + await vm.ReloadCommand.ExecuteAsync(null); // a redundant Refresh on the same range + + Assert.Equal(2, vm.SampleCount); + Assert.True(vm.HasData); + Assert.False(vm.IsBusy); // the gate released, so the progress bar cleared + } + + [Fact] + public async Task ReloadWithNoHistory_ReportsTheEmptyStateRatherThanABlankChart() + { + using var service = SeededService(); // no samples at all + using var vm = new ResourceHistoryViewModel(service); + await vm.InitializationComplete; + + Assert.False(vm.HasData); + Assert.Equal(0, vm.SampleCount); + Assert.Contains("No history yet", vm.StatusMessage); + } +} diff --git a/SysManager/SysManager/Services/BandwidthHistoryService.cs b/SysManager/SysManager/Services/BandwidthHistoryService.cs index 4b43ec2d..51459c86 100644 --- a/SysManager/SysManager/Services/BandwidthHistoryService.cs +++ b/SysManager/SysManager/Services/BandwidthHistoryService.cs @@ -51,6 +51,11 @@ public async Task AppendAsync(BandwidthSample sample, CancellationToken ct = def await File.AppendAllTextAsync(_dataPath, Serialize(sample) + "\n", ct).ConfigureAwait(false); } catch (IOException ex) { Log.Debug("Bandwidth history append failed: {Error}", ex.Message); } + // Same reason as ResourceHistoryService: UnauthorizedAccessException is a SIBLING of + // IOException, so it escapes the catch above. Here an escaping throw is worse — the + // caller sets _historyOwnsChart before awaiting, and only OperationCanceledException + // hands it back, so an unhandled access error would freeze the live chart for good. + catch (UnauthorizedAccessException ex) { Log.Debug("Bandwidth history append denied: {Error}", ex.Message); } finally { _fileLock.Release(); } } @@ -77,6 +82,7 @@ public async Task> LoadAsync(TimeSpan range, Canc return samples; } catch (IOException ex) { Log.Debug("Bandwidth history load failed: {Error}", ex.Message); return []; } + catch (UnauthorizedAccessException ex) { Log.Debug("Bandwidth history load denied: {Error}", ex.Message); return []; } finally { _fileLock.Release(); } } @@ -97,6 +103,7 @@ public async Task PruneAsync(CancellationToken ct = default) } catch (OperationCanceledException) { /* shutdown */ } catch (IOException ex) { Log.Debug("Bandwidth history prune failed: {Error}", ex.Message); } + catch (UnauthorizedAccessException ex) { Log.Debug("Bandwidth history prune denied: {Error}", ex.Message); } finally { _fileLock.Release(); } } diff --git a/SysManager/SysManager/Services/ResourceHistoryService.cs b/SysManager/SysManager/Services/ResourceHistoryService.cs index 02d0b9ca..fffb87da 100644 --- a/SysManager/SysManager/Services/ResourceHistoryService.cs +++ b/SysManager/SysManager/Services/ResourceHistoryService.cs @@ -31,16 +31,14 @@ public sealed class ResourceHistoryService : IDisposable // long-lived session doesn't let the file grow past the retention window on disk. private const int PruneEverySamples = 360; // 360 × 10s = 1 hour - private static readonly string DataDir = Path.Join( - Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "SysManager"); - private static readonly string DataPath = Path.Join(DataDir, "resource-history.ndjson"); - private static readonly string ConfigPath = Path.Join(DataDir, "resource-history-config.json"); - private static readonly JsonSerializerOptions SampleJson = new() { WriteIndented = false }; private readonly SystemInfoService _sys; private readonly TemperatureService _temps; private readonly SemaphoreSlim _fileLock = new(1, 1); + private readonly string _dataDir; + private readonly string _dataPath; + private readonly string _configPath; private CancellationTokenSource? _cts; private Task? _loopTask; @@ -54,10 +52,23 @@ public sealed class ResourceHistoryService : IDisposable private int _retentionDays = 7; - public ResourceHistoryService(SystemInfoService sys, TemperatureService temps) + /// + /// Creates the service. is overridable so tests exercise the real + /// append/load/prune paths against a temp directory instead of the user's own history file — + /// same seam as and . + /// The paths were previously static readonly, which made this service impossible to + /// test: resolves through the Win32 + /// known-folder API and ignores the LOCALAPPDATA environment variable, so not even a child + /// process could redirect it away from the real profile. + /// + public ResourceHistoryService(SystemInfoService sys, TemperatureService temps, string? configDir = null) { _sys = sys; _temps = temps; + _dataDir = configDir ?? Path.Join( + Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), "SysManager"); + _dataPath = Path.Join(_dataDir, "resource-history.ndjson"); + _configPath = Path.Join(_dataDir, "resource-history-config.json"); _retentionDays = LoadRetention(); } @@ -184,10 +195,15 @@ private async Task AppendAsync(ResourceSample sample, CancellationToken ct) await _fileLock.WaitAsync(ct).ConfigureAwait(false); try { - Directory.CreateDirectory(DataDir); - await File.AppendAllTextAsync(DataPath, Serialize(sample) + "\n", ct).ConfigureAwait(false); + Directory.CreateDirectory(_dataDir); + await File.AppendAllTextAsync(_dataPath, Serialize(sample) + "\n", ct).ConfigureAwait(false); } catch (IOException ex) { Log.Debug("Resource history append failed: {Error}", ex.Message); } + // UnauthorizedAccessException is a SIBLING of IOException, not a subclass, so it escapes the + // catch above. It is what File APIs raise for a read-only or ACL-denied file, which is + // reachable if the profile folder is locked down — and an unhandled throw here would + // surface as a crash from a background sampler the user never invoked. + catch (UnauthorizedAccessException ex) { Log.Debug("Resource history append denied: {Error}", ex.Message); } finally { if (!_disposed) _fileLock.Release(); } } @@ -200,8 +216,8 @@ public async Task> LoadAsync(TimeSpan range, Cance await _fileLock.WaitAsync(ct).ConfigureAwait(false); try { - if (!File.Exists(DataPath)) return []; - var lines = await File.ReadAllLinesAsync(DataPath, ct).ConfigureAwait(false); + if (!File.Exists(_dataPath)) return []; + var lines = await File.ReadAllLinesAsync(_dataPath, ct).ConfigureAwait(false); var cutoff = DateTime.Now - range; // The file is append-only and time-ordered, so the requested range is a suffix: // walk from the END and stop at the first line older than the cutoff. This bounds @@ -218,6 +234,7 @@ public async Task> LoadAsync(TimeSpan range, Cance return samples; } catch (IOException ex) { Log.Debug("Resource history load failed: {Error}", ex.Message); return []; } + catch (UnauthorizedAccessException ex) { Log.Debug("Resource history load denied: {Error}", ex.Message); return []; } finally { if (!_disposed) _fileLock.Release(); } } @@ -228,20 +245,21 @@ public async Task PruneAsync(CancellationToken ct = default) catch (OperationCanceledException) { return; } try { - if (!File.Exists(DataPath)) return; - var lines = await File.ReadAllLinesAsync(DataPath, ct).ConfigureAwait(false); + if (!File.Exists(_dataPath)) return; + var lines = await File.ReadAllLinesAsync(_dataPath, ct).ConfigureAwait(false); var kept = Prune(lines, DateTime.Now, TimeSpan.FromDays(_retentionDays)); // Only rewrite when something actually changed, to avoid needless disk churn. if (kept.Count == lines.Length) return; // Atomic rewrite: write to a temp file in the same directory, then swap it in // with a single File.Move. A crash mid-write can then only leave a stray .tmp - // (never read — the sampler reads DataPath), never a truncated history file. - var tmp = DataPath + ".tmp"; + // (never read — the sampler reads _dataPath), never a truncated history file. + var tmp = _dataPath + ".tmp"; await File.WriteAllLinesAsync(tmp, kept, ct).ConfigureAwait(false); - File.Move(tmp, DataPath, overwrite: true); + File.Move(tmp, _dataPath, overwrite: true); } catch (OperationCanceledException) { /* shutdown */ } catch (IOException ex) { Log.Debug("Resource history prune failed: {Error}", ex.Message); } + catch (UnauthorizedAccessException ex) { Log.Debug("Resource history prune denied: {Error}", ex.Message); } finally { if (!_disposed) _fileLock.Release(); } } @@ -339,12 +357,12 @@ public static string ToCsv(IEnumerable samples) private sealed record RetentionConfig(int RetentionDays); - private static int LoadRetention() + private int LoadRetention() { try { - if (!File.Exists(ConfigPath)) return 7; - var cfg = JsonSerializer.Deserialize(File.ReadAllText(ConfigPath)); + if (!File.Exists(_configPath)) return 7; + var cfg = JsonSerializer.Deserialize(File.ReadAllText(_configPath)); return cfg is not null && RetentionOptions.Contains(cfg.RetentionDays) ? cfg.RetentionDays : 7; } catch (Exception ex) when (ex is JsonException or IOException or UnauthorizedAccessException) @@ -354,14 +372,15 @@ private static int LoadRetention() } } - private static void SaveRetention(int days) + private void SaveRetention(int days) { try { - Directory.CreateDirectory(DataDir); - File.WriteAllText(ConfigPath, JsonSerializer.Serialize(new RetentionConfig(days))); + Directory.CreateDirectory(_dataDir); + File.WriteAllText(_configPath, JsonSerializer.Serialize(new RetentionConfig(days))); } catch (IOException ex) { Log.Debug("Resource history config save failed: {Error}", ex.Message); } + catch (UnauthorizedAccessException ex) { Log.Debug("Resource history config save denied: {Error}", ex.Message); } } public void Dispose() diff --git a/SysManager/SysManager/SysManager.csproj b/SysManager/SysManager/SysManager.csproj index f261c364..83ffdf3a 100644 --- a/SysManager/SysManager/SysManager.csproj +++ b/SysManager/SysManager/SysManager.csproj @@ -10,9 +10,9 @@ SysManager true NU1603;NU1701 - 1.57.3 - 1.57.3.0 - 1.57.3.0 + 1.57.4 + 1.57.4.0 + 1.57.4.0 SysManager SysManager — Windows system monitoring toolkit by laurentiu021. Network, updates, health, logs, safe deep cleanup. https://github.com/laurentiu021/SystemManager diff --git a/SysManager/SysManager/ViewModels/ResourceHistoryViewModel.cs b/SysManager/SysManager/ViewModels/ResourceHistoryViewModel.cs index 6ecf183d..5e0c38d4 100644 --- a/SysManager/SysManager/ViewModels/ResourceHistoryViewModel.cs +++ b/SysManager/SysManager/ViewModels/ResourceHistoryViewModel.cs @@ -73,6 +73,15 @@ public sealed partial class ResourceHistoryViewModel : ViewModelBase private readonly BulkObservableCollection _cpuTempBuffer = new(); private readonly BulkObservableCollection _gpuTempBuffer = new(); + // Serializes ReloadAsync. Same guard, same reason as BandwidthMonitorViewModel's: three + // independent entry points can overlap (the constructor's InitializeAsync, the + // fire-and-forget from OnSelectedRangeChanged, and the Refresh button's command), and each + // one calls ReplaceWith on buffers LiveCharts is observing. Its observer keeps a HashSet it + // updates from the change notification, so a second thread arriving mid-notification + // corrupts it — "Operations that change non-concurrent collections must have exclusive + // access", which is how CI caught the identical race on the bandwidth chart. + private readonly SemaphoreSlim _reloadGate = new(1, 1); + public ResourceHistoryViewModel(ResourceHistoryService service) { _service = service; @@ -110,6 +119,10 @@ partial void OnRetentionDaysChanged(int value) [RelayCommand] private async Task ReloadAsync() { + // A gate rather than a lock: this is an async path so it must not block the UI thread, and + // dropping nothing is important here — unlike a range switch, each caller may be asking for + // a different range, so every request runs, just strictly one at a time. + await _reloadGate.WaitAsync().ConfigureAwait(true); IsBusy = true; try { @@ -132,7 +145,11 @@ private async Task ReloadAsync() ? $"Showing {_loaded.Count} sample(s) over {SelectedRange.Label.ToLowerInvariant()}." : "No history yet — samples are recorded every 10 seconds while the app runs."; } - finally { IsBusy = false; } + finally + { + IsBusy = false; + _reloadGate.Release(); + } } /// Pure: averages/peaks for the summary strip. Testable without WPF. @@ -252,6 +269,7 @@ protected override void Dispose(bool disposing) (LegendBackgroundPaint as IDisposable)?.Dispose(); (TooltipTextPaint as IDisposable)?.Dispose(); (TooltipBackgroundPaint as IDisposable)?.Dispose(); + _reloadGate.Dispose(); } base.Dispose(disposing); }