From 8ed454f37b90b693b9f2c19d04a8822cdf19760f Mon Sep 17 00:00:00 2001 From: laurentiu021 Date: Mon, 8 Jun 2026 15:41:37 +0300 Subject: [PATCH] fix: ping CTS leak, Event Log UI-thread block, AMD/Intel GPU name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 3 (batch 2) — thread/resource safety: - PingMonitorService.Stop dropped the CTS reference (set _cts=null) without disposing it whenever the 1.5s loop wait timed out, leaking the CTS. It now captures the CTS locally and disposes it once the loop finishes — immediately if already complete, else via a ContinueWith. - EventLogService.ReadAsync ran the blocking EventLogReader.ReadEvent() COM call on the caller's UI thread (await Task.Yield only released it momentarily), so large logs froze the UI. Each read now runs on the thread pool via Task.Run. - DashboardViewModel.UpdateGpuUsage only queried NVIDIA via NvAPI, so AMD/Intel users saw no GPU at all. Added a Win32_VideoController (WMI) fallback for the adapter name (usage % stays NVIDIA-only — vendor-specific APIs). StartupViewModel's cross-thread mutation (also flagged in the audit) was already fixed in #585, verified. Test: PingMonitorServiceLifecycleTests covers start/stop/dispose/double-start. --- CHANGELOG.md | 9 +++ .../PingMonitorServiceLifecycleTests.cs | 59 +++++++++++++++++++ .../SysManager/Services/EventLogService.cs | 13 ++-- .../SysManager/Services/PingMonitorService.cs | 26 +++++--- SysManager/SysManager/SysManager.csproj | 6 +- .../ViewModels/DashboardViewModel.cs | 40 ++++++++++++- 6 files changed, 135 insertions(+), 18 deletions(-) create mode 100644 SysManager/SysManager.Tests/PingMonitorServiceLifecycleTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3bf8b8e..ef1f16e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,15 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +## [1.20.4] - 2026-06-08 + +### Fixed +- **Ping monitor no longer leaks its CancellationTokenSource.** When `Stop()` was called while the ping loop was still winding down (the 1.5s wait timed out), the CTS reference was dropped and never disposed. It is now disposed once the loop actually finishes — immediately if already complete, otherwise via a continuation. +- **System Logs no longer block the UI thread while reading the Event Log.** `EventLogService.ReadAsync` ran the blocking `EventLogReader.ReadEvent()` COM call on the caller's (UI) thread, freezing the app while large logs were enumerated. Each read now runs on the thread pool via `Task.Run`. + +### Changed +- **Dashboard GPU name now works for AMD/Intel, not just NVIDIA.** When no NVIDIA GPU is present, the Dashboard falls back to `Win32_VideoController` (WMI) to show the adapter name. Live usage % remains NVIDIA-only (it requires vendor-specific APIs). + ## [1.20.3] - 2026-06-08 ### Fixed diff --git a/SysManager/SysManager.Tests/PingMonitorServiceLifecycleTests.cs b/SysManager/SysManager.Tests/PingMonitorServiceLifecycleTests.cs new file mode 100644 index 0000000..2778449 --- /dev/null +++ b/SysManager/SysManager.Tests/PingMonitorServiceLifecycleTests.cs @@ -0,0 +1,59 @@ +// SysManager · PingMonitorServiceLifecycleTests +// Author: laurentiu021 · https://github.com/laurentiu021/SystemManager +// License: MIT + +using SysManager.Services; + +namespace SysManager.Tests; + +/// +/// Lifecycle/regression tests for . The Stop() +/// path was rewritten so the CancellationTokenSource is always disposed (even +/// when the loop is still winding down) instead of being dropped and leaked. +/// These verify the start/stop/dispose cycle is safe and idempotent. +/// +public class PingMonitorServiceLifecycleTests +{ + [Fact] + public void StartStop_DoesNotThrow() + { + using var svc = new PingMonitorService(); + svc.Start(); + svc.Stop(); + } + + [Fact] + public void Stop_WithoutStart_IsNoOp() + { + using var svc = new PingMonitorService(); + svc.Stop(); // never started — must not throw + } + + [Fact] + public void RepeatedStartStop_DoesNotThrow() + { + using var svc = new PingMonitorService(); + for (var i = 0; i < 5; i++) + { + svc.Start(); + svc.Stop(); + } + } + + [Fact] + public void Dispose_AfterStart_DoesNotThrow() + { + var svc = new PingMonitorService(); + svc.Start(); + svc.Dispose(); // Dispose() routes through Stop() + } + + [Fact] + public void DoubleStart_IsIdempotent() + { + using var svc = new PingMonitorService(); + svc.Start(); + svc.Start(); // second Start() must be ignored while running, not leak a CTS + svc.Stop(); + } +} diff --git a/SysManager/SysManager/Services/EventLogService.cs b/SysManager/SysManager/Services/EventLogService.cs index d5d644d..33f5a93 100644 --- a/SysManager/SysManager/Services/EventLogService.cs +++ b/SysManager/SysManager/Services/EventLogService.cs @@ -43,11 +43,17 @@ private async IAsyncEnumerable ReadInternal( int emitted = 0; using (reader) { + var localReader = reader; while (!ct.IsCancellationRequested && emitted < opt.MaxResults) { - EventRecord? rec = null; - try { rec = reader.ReadEvent(); } + // ReadEvent() is a blocking COM/IO call. Run it on a thread-pool + // thread so enumerating large logs never blocks the UI thread the + // caller awaits on. (await Task.Yield() alone did not move the work + // off the UI thread — it only released it momentarily per 200 rows.) + EventRecord? rec; + try { rec = await Task.Run(() => localReader.ReadEvent(), ct).ConfigureAwait(false); } catch (EventLogException) { continue; } + catch (OperationCanceledException) { yield break; } if (rec is null) yield break; FriendlyEventEntry? entry = null; @@ -61,9 +67,6 @@ private async IAsyncEnumerable ReadInternal( emitted++; yield return entry; - - // Yield occasionally to keep the UI responsive on huge logs. - if (emitted % 200 == 0) await Task.Yield(); } } } diff --git a/SysManager/SysManager/Services/PingMonitorService.cs b/SysManager/SysManager/Services/PingMonitorService.cs index 9439490..7df3194 100644 --- a/SysManager/SysManager/Services/PingMonitorService.cs +++ b/SysManager/SysManager/Services/PingMonitorService.cs @@ -52,17 +52,25 @@ public void Stop() { lock (_stateLock) { - _cts?.Cancel(); - try { _loop?.Wait(1500); } - catch (AggregateException) { /* task cancellation or faulted — expected during stop */ } - catch (ObjectDisposedException) { /* task already cleaned up */ } - // Only dispose CTS if the loop actually completed; otherwise the - // background task still holds a reference to the token and would - // throw ObjectDisposedException on next cancellation check. - if (_loop is { IsCompleted: true }) - _cts?.Dispose(); + var cts = _cts; + var loop = _loop; _cts = null; _loop = null; + if (cts is null) return; + + cts.Cancel(); + try { loop?.Wait(1500); } + catch (AggregateException) { /* task cancellation or faulted — expected during stop */ } + catch (ObjectDisposedException) { /* task already cleaned up */ } + + // Dispose the CTS once the loop has actually finished. If Wait timed out + // (the loop is still winding down), defer disposal to a continuation so the + // CTS is never leaked — the previous code dropped the reference and never + // disposed it in that case. + if (loop is null || loop.IsCompleted) + cts.Dispose(); + else + loop.ContinueWith(_ => cts.Dispose(), TaskScheduler.Default); } } diff --git a/SysManager/SysManager/SysManager.csproj b/SysManager/SysManager/SysManager.csproj index 055b0cb..bd9affb 100644 --- a/SysManager/SysManager/SysManager.csproj +++ b/SysManager/SysManager/SysManager.csproj @@ -10,9 +10,9 @@ SysManager true NU1603;NU1701 - 1.20.3 - 1.20.3.0 - 1.20.3.0 + 1.20.4 + 1.20.4.0 + 1.20.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/DashboardViewModel.cs b/SysManager/SysManager/ViewModels/DashboardViewModel.cs index 0457520..8a7b015 100644 --- a/SysManager/SysManager/ViewModels/DashboardViewModel.cs +++ b/SysManager/SysManager/ViewModels/DashboardViewModel.cs @@ -171,11 +171,49 @@ private void UpdateGpuUsage() GpuName = gpu.FullName; GpuVram = $"{memUsed:F1} / {memTotal:F1} GB VRAM"; }); + return; } } catch (Exception ex) { - Log.Debug("GPU polling unavailable: {Error}", ex.Message); + Log.Debug("NVIDIA GPU polling unavailable: {Error}", ex.Message); + } + + // No NVIDIA GPU (NvAPI only covers NVIDIA). Fall back to WMI so AMD/Intel + // GPUs at least show the adapter name. Live usage % is NVIDIA-only because + // it requires vendor-specific APIs. + UpdateGpuNameFromWmi(); + } + + private void UpdateGpuNameFromWmi() + { + try + { + using var searcher = new System.Management.ManagementObjectSearcher( + "SELECT Name FROM Win32_VideoController"); + using var collection = searcher.Get(); + foreach (System.Management.ManagementObject mo in collection) + { + using (mo) + { + var name = mo["Name"]?.ToString()?.Trim(); + if (string.IsNullOrEmpty(name)) continue; + System.Windows.Application.Current?.Dispatcher.BeginInvoke(() => + { + GpuName = name; + GpuVram = ""; + }); + return; // first adapter is enough + } + } + } + catch (System.Management.ManagementException ex) + { + Log.Debug("WMI GPU name lookup unavailable: {Error}", ex.Message); + } + catch (System.Runtime.InteropServices.COMException ex) + { + Log.Debug("WMI GPU name lookup failed: {Error}", ex.Message); } }