Fixes applied (14 of 15 findings — see note on R5)

Concurrency crashes
  - H1 HardwareMonitorService.RebuildTree now builds the tree under _updateLock, so it can't enumerate a hardware's _active HashSet while the update loop mutates it.
  - A1 AppSettings now guards every dictionary read/write (and snapshots in Save) with a lock — safe under concurrent access from the parallel discovery threads.
  - R2 Plumbed that same lock (HardwareMonitorService.SensorReadLock) into RemoteWebServer and wrapped the Prometheus sensor.Values enumeration with it.
  - L1 Computer — refactored Add into AddCore, which performs the cancellation/enabled re-check and the _groups insertion atomically under _lock. A deferred task can
  no longer add (and leak) a group after Close() drained the list; if it loses the race it closes the group instead.
  - M1 UpdateTimer_Tick now bails before/after the await when _isShuttingDown is set in MainWindow_Closed, so an in-flight tick won't touch the disposed
  view-model/Computer.

  Broken behavior
  - T1 Tray callback now decodes NOTIFYICON_VERSION_4 correctly (message = LOWORD(lParam), icon id = HIWORD(lParam)) — right-click menu and double-click work again.
  - M2 A transient update exception no longer calls _timer.Stop(); the loop keeps running.
  - H2 Newly discovered (deferred) storage devices get the current ForceDriveWakeup setting applied in HardwareChanged.
  - V2 Sensor items carry a parent reference; toggling IsVisible recomputes the parent group's visibility, so no empty group headers. (Strengthened the existing test
  that had skipped this assertion.)
  - H3 Tree-rebuild coalescing now uses a dirty flag with a re-check, so a change arriving during a rebuild isn't lost.
  - R4 Web routing matches endpoints exactly on the query-stripped path (Url.AbsolutePath), so static assets like metrics.html aren't hijacked.
  - L8 IntelCpu.Update skips the bus/core-clock math while TimeStampCounterFrequency is still 0 (deferred-TSC window), so clocks keep their prior value instead of
  reporting 0 MHz.
  - V1 Existing plot series keep their assigned color; only an explicit user pen color updates them (no per-tick color shifting).
  - M3 Runtime errors write to a dedicated runtime.log (once), instead of overwriting the shared startup.log.
  - T2 CreateSensorIcon returns IntPtr.Zero on DIB failure instead of the shared main-icon handle (which callers DestroyIcon).

  I also set _isOpen = false in HardwareMonitorService.Dispose so the rebuild guard actually holds during shutdown (the latent after-close-rebuild issue adjacent to
  H3
This commit is contained in:
2026-05-30 22:56:25 -05:00
parent 5f88f71e61
commit f9edc1dc64
10 changed files with 216 additions and 84 deletions
@@ -29,6 +29,7 @@ public sealed class HardwareMonitorService : IDisposable
private readonly UpdateVisitor _updateVisitor = new();
private bool _isOpen;
private int _treeRebuildQueued;
private volatile bool _treeRebuildDirty;
public HardwareMonitorService(AppSettings settings)
{
@@ -44,6 +45,12 @@ public sealed class HardwareMonitorService : IDisposable
public Computer Computer { get; }
/// <summary>
/// Lock guarding access to live sensor collections (values and active-sensor sets). Held by <see cref="UpdateAsync" />
/// while sensors are updated; other readers (tree rebuild, the remote web server) take it to read consistently.
/// </summary>
public object SensorReadLock => _updateLock;
public AppSettings Settings { get; }
public SensorTreeItemViewModel Root { get; private set; } = SensorTreeItemViewModel.CreateRoot(Environment.MachineName);
@@ -177,8 +184,14 @@ public sealed class HardwareMonitorService : IDisposable
public void RebuildTree(bool raiseTreeRebuilt = true)
{
SensorTreeItemViewModel root = SensorTreeItemViewModel.CreateRoot(Environment.MachineName);
foreach (IHardware hardware in Computer.Hardware.OrderBy(hardware => hardware.HardwareType).ThenBy(hardware => hardware.Name, StringComparer.OrdinalIgnoreCase))
root.Children.Add(SensorTreeItemViewModel.FromHardware(hardware, Settings));
// Building the tree reads each hardware's live sensor set (a HashSet). Hold the update lock so it cannot run
// concurrently with UpdateAsync()'s Computer.Accept, which mutates that set via ActivateSensor on another thread.
lock (_updateLock)
{
foreach (IHardware hardware in Computer.Hardware.OrderBy(hardware => hardware.HardwareType).ThenBy(hardware => hardware.Name, StringComparer.OrdinalIgnoreCase))
root.Children.Add(SensorTreeItemViewModel.FromHardware(hardware, Settings));
}
Root = root;
if (raiseTreeRebuilt)
@@ -187,6 +200,9 @@ public sealed class HardwareMonitorService : IDisposable
public void Dispose()
{
// Clear before closing so any tree-rebuild task still pending its delay bails instead of rebuilding from a
// half-closed Computer.
_isOpen = false;
Computer.HardwareAdded -= HardwareChanged;
Computer.HardwareRemoved -= HardwareChanged;
Computer.Close();
@@ -247,12 +263,18 @@ public sealed class HardwareMonitorService : IDisposable
private void HardwareChanged(IHardware hardware)
{
// Storage discovery is deferred, so drives usually appear after ForceDriveWakeup was applied at startup (against
// an empty hardware list). Apply the current setting to each newly discovered drive so it actually takes effect.
if (hardware is StorageDevice storageDevice)
storageDevice.ForceWakeup = Settings.GetValue("forceDriveWakeupItem", false);
if (_isOpen)
QueueTreeRebuild();
}
private void QueueTreeRebuild()
{
_treeRebuildDirty = true;
if (Interlocked.Exchange(ref _treeRebuildQueued, 1) == 1)
return;
@@ -262,8 +284,14 @@ public sealed class HardwareMonitorService : IDisposable
{
await Task.Delay(100).ConfigureAwait(false);
if (_isOpen)
RebuildTree();
// Rebuild until no change has arrived since the last rebuild began, so a HardwareAdded/Removed that lands
// while a rebuild is in flight is not coalesced away and lost.
while (_treeRebuildDirty)
{
_treeRebuildDirty = false;
if (_isOpen)
RebuildTree();
}
}
catch (Exception ex)
{
@@ -272,6 +300,10 @@ public sealed class HardwareMonitorService : IDisposable
finally
{
Interlocked.Exchange(ref _treeRebuildQueued, 0);
// A change may have slipped in between the loop's exit and clearing the flag; re-queue so it is honored.
if (_treeRebuildDirty)
QueueTreeRebuild();
}
});
}