From 268f15a19ed93bc9e0e25b9a7dd27ed9589840ab Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Tue, 2 Jun 2026 11:06:52 -0500 Subject: [PATCH] Extract TreeRebuildCoalescer from HardwareMonitorService Moves the hardware-change debounce/coalescing worker (the dirty/queued flags and re-queue logic) into a dedicated class with an injectable delay, so the concurrency behavior can be unit-tested deterministically instead of living as an untestable fire-and-forget block in the service. Behavior is unchanged. Adds 3 tests (single rebuild after delay, burst coalesced to one, no rebuild when closed) driven by a controllable delay gate. 199 tests pass (was 196). Co-Authored-By: Claude Opus 4.8 --- .../Services/TreeRebuildCoalescerTests.cs | 60 ++++++++++++++++ .../Services/HardwareMonitorService.cs | 42 +---------- .../Services/TreeRebuildCoalescer.cs | 72 +++++++++++++++++++ 3 files changed, 135 insertions(+), 39 deletions(-) create mode 100644 LibreHardwareMonitor.Windows.WinUI.Tests/Services/TreeRebuildCoalescerTests.cs create mode 100644 LibreHardwareMonitor.Windows.WinUI/Services/TreeRebuildCoalescer.cs diff --git a/LibreHardwareMonitor.Windows.WinUI.Tests/Services/TreeRebuildCoalescerTests.cs b/LibreHardwareMonitor.Windows.WinUI.Tests/Services/TreeRebuildCoalescerTests.cs new file mode 100644 index 0000000..9df74b3 --- /dev/null +++ b/LibreHardwareMonitor.Windows.WinUI.Tests/Services/TreeRebuildCoalescerTests.cs @@ -0,0 +1,60 @@ +// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. +// If a copy of the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. +// Copyright (C) LibreHardwareMonitor and Contributors. + +using System.Threading; +using System.Threading.Tasks; +using LibreHardwareMonitor.Windows.WinUI.Services; +using Xunit; + +namespace LibreHardwareMonitor.Windows.WinUI.Tests.Services; + +public class TreeRebuildCoalescerTests +{ + [Fact] + public async Task Request_RebuildsOnceAfterDelay() + { + var gate = new TaskCompletionSource(); + int rebuilds = 0; + var coalescer = new TreeRebuildCoalescer(() => Interlocked.Increment(ref rebuilds), () => true, () => gate.Task); + + coalescer.Request(); + Assert.Equal(0, rebuilds); // worker is blocked on the (not-yet-completed) delay + + gate.SetResult(); + await coalescer.RebuildLoopTask; + + Assert.Equal(1, rebuilds); + } + + [Fact] + public async Task Request_CoalescesBurstIntoSingleRebuild() + { + var gate = new TaskCompletionSource(); + int rebuilds = 0; + var coalescer = new TreeRebuildCoalescer(() => Interlocked.Increment(ref rebuilds), () => true, () => gate.Task); + + coalescer.Request(); + coalescer.Request(); + coalescer.Request(); + + gate.SetResult(); + await coalescer.RebuildLoopTask; + + Assert.Equal(1, rebuilds); + } + + [Fact] + public async Task Request_DoesNotRebuildWhenCannotRebuild() + { + var gate = new TaskCompletionSource(); + int rebuilds = 0; + var coalescer = new TreeRebuildCoalescer(() => Interlocked.Increment(ref rebuilds), () => false, () => gate.Task); + + coalescer.Request(); + gate.SetResult(); + await coalescer.RebuildLoopTask; + + Assert.Equal(0, rebuilds); + } +} diff --git a/LibreHardwareMonitor.Windows.WinUI/Services/HardwareMonitorService.cs b/LibreHardwareMonitor.Windows.WinUI/Services/HardwareMonitorService.cs index 6e51f67..ecc246a 100644 --- a/LibreHardwareMonitor.Windows.WinUI/Services/HardwareMonitorService.cs +++ b/LibreHardwareMonitor.Windows.WinUI/Services/HardwareMonitorService.cs @@ -3,7 +3,6 @@ // Copyright (C) LibreHardwareMonitor and Contributors. using System; -using System.Diagnostics; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -27,13 +26,13 @@ public sealed class HardwareMonitorService : IDisposable private readonly object _updateLock = new(); private readonly UpdateVisitor _updateVisitor = new(); + private readonly TreeRebuildCoalescer _treeRebuildCoalescer; private bool _isOpen; - private int _treeRebuildQueued; - private volatile bool _treeRebuildDirty; public HardwareMonitorService(AppSettings settings) { Settings = settings; + _treeRebuildCoalescer = new TreeRebuildCoalescer(() => RebuildTree(), () => _isOpen); ApplyWinUiHardwareDefaults(); Computer = new Computer(settings); Computer.HardwareAdded += HardwareChanged; @@ -269,42 +268,7 @@ public sealed class HardwareMonitorService : IDisposable storageDevice.ForceWakeup = Settings.GetValue("forceDriveWakeupItem", false); if (_isOpen) - QueueTreeRebuild(); + _treeRebuildCoalescer.Request(); } - private void QueueTreeRebuild() - { - _treeRebuildDirty = true; - if (Interlocked.Exchange(ref _treeRebuildQueued, 1) == 1) - return; - - _ = Task.Run(async () => - { - try - { - await Task.Delay(100).ConfigureAwait(false); - - // 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) - { - Debug.WriteLine($"Hardware tree rebuild failed: {ex}"); - } - 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(); - } - }); - } } diff --git a/LibreHardwareMonitor.Windows.WinUI/Services/TreeRebuildCoalescer.cs b/LibreHardwareMonitor.Windows.WinUI/Services/TreeRebuildCoalescer.cs new file mode 100644 index 0000000..a5e40ee --- /dev/null +++ b/LibreHardwareMonitor.Windows.WinUI/Services/TreeRebuildCoalescer.cs @@ -0,0 +1,72 @@ +// This Source Code Form is subject to the terms of the Mozilla Public License, v. 2.0. +// If a copy of the MPL was not distributed with this file, You can obtain one at http://mozilla.org/MPL/2.0/. +// Copyright (C) LibreHardwareMonitor and Contributors. + +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; + +namespace LibreHardwareMonitor.Windows.WinUI.Services; + +/// +/// Coalesces a burst of hardware-change notifications into as few tree rebuilds as possible without dropping the last +/// one. After the first request a single worker waits a short debounce delay, then rebuilds while more changes keep +/// arriving; a change that lands between the worker finishing and the queued flag clearing re-arms the worker. +/// Extracted from so this concurrency logic can be unit-tested with a controllable +/// delay. +/// +internal sealed class TreeRebuildCoalescer +{ + private readonly Action _rebuild; + private readonly Func _canRebuild; + private readonly Func _delay; + private int _queued; + private volatile bool _dirty; + + public TreeRebuildCoalescer(Action rebuild, Func canRebuild, Func? delay = null) + { + _rebuild = rebuild; + _canRebuild = canRebuild; + _delay = delay ?? (() => Task.Delay(100)); + } + + /// The most recently started worker. Exposed only so tests can deterministically await a rebuild cycle. + internal Task RebuildLoopTask { get; private set; } = Task.CompletedTask; + + public void Request() + { + _dirty = true; + if (Interlocked.Exchange(ref _queued, 1) == 1) + return; + + RebuildLoopTask = Task.Run(async () => + { + try + { + await _delay().ConfigureAwait(false); + + // Rebuild until no change has arrived since the last rebuild began, so a change that lands while a + // rebuild is in flight is not coalesced away and lost. + while (_dirty) + { + _dirty = false; + if (_canRebuild()) + _rebuild(); + } + } + catch (Exception ex) + { + Debug.WriteLine($"Hardware tree rebuild failed: {ex}"); + } + finally + { + Interlocked.Exchange(ref _queued, 0); + + // A change may have slipped in between the loop's exit and clearing the flag; re-queue so it is honored. + if (_dirty) + Request(); + } + }); + } +}