From 06b714ae1b1a09fdbe866a5bd718bbdbb8fa5f55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Mercier?= Date: Mon, 21 Jul 2025 15:28:46 -0400 Subject: [PATCH] MemoryGroup: Remove retry + proper identifier (#1797) * Remove retry + proper identifier * Update LibreHardwareMonitorLib.csproj * Revert "Update LibreHardwareMonitorLib.csproj" This reverts commit 185aa2dbd3d338da79aff8bfa9419f016d107e77. * Add safer retry logic for MemoryGroup * Move kernel driver assignation in instance ctor instead of static (one time) ctor, since Ring0 can get closed and reopened multiple times in the lifetime of an application. * Add Task based retry logic with try catch + exception reporting + hardware changed event * Update MemoryGroup.cs * Update RAMSPDToolkitDriver.cs * Update RAMSPDToolkitDriver.cs * Mutate _hardware field to avoid enumeration exceptions * Add Report + set configureAwait on the Delay. --- .../Hardware/Memory/MemoryGroup.cs | 222 ++++++++++++------ .../Hardware/RAMSPDToolkitDriver.cs | 8 +- 2 files changed, 149 insertions(+), 81 deletions(-) diff --git a/LibreHardwareMonitorLib/Hardware/Memory/MemoryGroup.cs b/LibreHardwareMonitorLib/Hardware/Memory/MemoryGroup.cs index b62b0d0..378d7ba 100644 --- a/LibreHardwareMonitorLib/Hardware/Memory/MemoryGroup.cs +++ b/LibreHardwareMonitorLib/Hardware/Memory/MemoryGroup.cs @@ -4,8 +4,13 @@ // Partial Copyright (C) Michael Möller and Contributors. // All Rights Reserved. +using System; using System.Collections.Generic; -using System.Timers; +using System.Diagnostics; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; using RAMSPDToolkit.I2CSMBus; using RAMSPDToolkit.SPD; using RAMSPDToolkit.SPD.Enums; @@ -15,133 +20,196 @@ using RAMSPDToolkit.Windows.Driver; namespace LibreHardwareMonitor.Hardware.Memory; -internal class MemoryGroup : IGroup +internal class MemoryGroup : IGroup, IHardwareChanged { - //Retry 12x - private const int RetryCount = 12; - - //Retry every 2.5 seconds - private const double RetryTime = 2500; private static readonly object _lock = new(); + private List _hardware = []; - private readonly List _hardware = []; - private int _elapsedCounter; + private CancellationTokenSource _cancellationTokenSource; + private Exception _lastException; + private bool _opened = false; - private Timer _timer; - - static MemoryGroup() + public MemoryGroup(ISettings settings) { - if (Ring0.IsOpen) + if (Ring0.IsOpen && (DriverManager.Driver is null || !DriverManager.Driver.IsOpen)) { // Assign implementation of IDriver. DriverManager.Driver = new RAMSPDToolkitDriver(Ring0.KernelDriver); SMBusManager.UseWMI = false; } - else - { - // Still need to set Driver if Ring0 is absent. - DriverManager.Driver = new RAMSPDToolkitDriver(null); - SMBusManager.UseWMI = false; - } - } - public MemoryGroup(ISettings settings) - { _hardware.Add(new VirtualMemory(settings)); _hardware.Add(new TotalMemory(settings)); - //No RAM detected - if (!DetectThermalSensors(out List accessors)) + if (DriverManager.Driver == null) { - //Retry a couple of times - //SMBus might not be detected right after boot - _timer = new Timer(RetryTime); - - _timer.Elapsed += (_, _) => - { - if (_elapsedCounter++ >= RetryCount || DetectThermalSensors(out accessors)) - { - _timer.Stop(); - _timer = null; - - if (accessors != null) - AddDimms(accessors, settings); - } - }; - - _timer.Start(); + return; } - else + + if (!TryAddDimms(settings)) { - AddDimms(accessors, settings); + StartRetryTask(settings); } + + _opened = true; } + public event HardwareEventHandler HardwareAdded; + public event HardwareEventHandler HardwareRemoved; + public IReadOnlyList Hardware => _hardware; public string GetReport() { - return null; + StringBuilder report = new(); + report.AppendLine("Memory Report:"); + if (_lastException != null) + { + report.AppendLine($"Error while detecting memory: {_lastException.Message}"); + } + + foreach (Hardware hardware in _hardware) + { + report.AppendLine($"{hardware.Name} ({hardware.Identifier}):"); + report.AppendLine(); + foreach (ISensor sensor in hardware.Sensors) + { + report.AppendLine($"{sensor.Name}: {sensor.Value?.ToString() ?? "No value"}"); + } + } + + return report.ToString(); } public void Close() { - foreach (Hardware ram in _hardware) - ram.Close(); + lock (_lock) + { + _opened = false; + foreach (Hardware ram in _hardware) + ram.Close(); + + _hardware.Clear(); + + _cancellationTokenSource?.Cancel(); + _cancellationTokenSource?.Dispose(); + _cancellationTokenSource = null; + } + } + + private bool TryAddDimms(ISettings settings) + { + try + { + lock (_lock) + { + if (!_opened) + { + return true; + } + + if (DetectThermalSensors(out List accessors)) + { + AddDimms(accessors, settings); + return true; + } + } + } + catch (Exception ex) + { + _lastException = ex; + Debug.Assert(false, "Exception while detecting RAM: " + ex.Message); + } + + return false; + } + + private void StartRetryTask(ISettings settings) + { + _cancellationTokenSource = new CancellationTokenSource(); + + Task.Run(async () => + { + int retryRemaining = 5; + + while (!_cancellationTokenSource.IsCancellationRequested && --retryRemaining > 0) + { + await Task.Delay(TimeSpan.FromSeconds(2.5), _cancellationTokenSource.Token).ConfigureAwait(false); + + if (TryAddDimms(settings)) + { + lock (_lock) + { + if (!_opened) + { + return; + } + + foreach (Hardware hardware in _hardware.OfType()) + { + HardwareAdded?.Invoke(hardware); + } + + _cancellationTokenSource.Dispose(); + _cancellationTokenSource = null; + + break; + } + + } + } + }, _cancellationTokenSource.Token); } private static bool DetectThermalSensors(out List accessors) { - lock (_lock) + accessors = []; + + bool ramDetected = false; + + SMBusManager.DetectSMBuses(); + + //Go through detected SMBuses + foreach (SMBusInterface smbus in SMBusManager.RegisteredSMBuses) { - var list = new List(); - - bool ramDetected = false; - - SMBusManager.DetectSMBuses(); - - //Go through detected SMBuses - foreach (var smbus in SMBusManager.RegisteredSMBuses) + //Go through possible RAM slots + for (byte i = SPDConstants.SPD_BEGIN; i <= SPDConstants.SPD_END; ++i) { - //Go through possible RAM slots - for (byte i = SPDConstants.SPD_BEGIN; i <= SPDConstants.SPD_END; ++i) + //Detect type of RAM, if available + SPDDetector detector = new(smbus, i); + + //RAM available and detected + if (detector.Accessor != null) { - //Detect type of RAM, if available - var detector = new SPDDetector(smbus, i); + //We are only interested in modules with thermal sensor + if (detector.Accessor is IThermalSensor { HasThermalSensor: true }) + accessors.Add(detector.Accessor); - //RAM available and detected - if (detector.Accessor != null) - { - //We are only interested in modules with thermal sensor - if (detector.Accessor is IThermalSensor { HasThermalSensor: true }) - list.Add(detector.Accessor); - - ramDetected = true; - } + ramDetected = true; } } - - accessors = list.Count > 0 ? list : []; - return ramDetected; } + + return ramDetected; } private void AddDimms(List accessors, ISettings settings) { - foreach (var ram in accessors) + List newHardwareList = [.. _hardware]; + + foreach (SPDAccessor ram in accessors) { //Default value string name = $"DIMM #{ram.Index}"; //Check if we can switch to the correct page if (ram.ChangePage(PageData.ModulePartNumber)) - { name = $"{ram.GetModuleManufacturerString()} - {ram.ModulePartNumber()} (#{ram.Index})"; - } - var memory = new DimmMemory(ram, name, new Identifier("ram"), settings); - - _hardware.Add(memory); + DimmMemory memory = new(ram, name, new Identifier($"memory/dimm/{ram.Index}"), settings); + newHardwareList.Add(memory); } + + _hardware = newHardwareList; } } diff --git a/LibreHardwareMonitorLib/Hardware/RAMSPDToolkitDriver.cs b/LibreHardwareMonitorLib/Hardware/RAMSPDToolkitDriver.cs index a85846d..20583ff 100644 --- a/LibreHardwareMonitorLib/Hardware/RAMSPDToolkitDriver.cs +++ b/LibreHardwareMonitorLib/Hardware/RAMSPDToolkitDriver.cs @@ -18,11 +18,11 @@ namespace LibreHardwareMonitor.Hardware { private KernelDriver _kernelDriver; - private const byte PCI_MAX_NUMBER_OF_BUS = 255; - private const byte PCI_NUMBER_OF_DEVICE = 32; - private const byte PCI_NUMBER_OF_FUNCTION = 8; + private const byte PCI_MAX_NUMBER_OF_BUS = 255; + private const byte PCI_NUMBER_OF_DEVICE = 32; + private const byte PCI_NUMBER_OF_FUNCTION = 8; - public bool IsOpen => _kernelDriver != null; + public bool IsOpen => _kernelDriver?.IsOpen ?? false; public RAMSPDToolkitDriver(KernelDriver kernelDriver) {