Skip to content

Commit 06b714a

Browse files
Rem0oRem0o
authored
MemoryGroup: Remove retry + proper identifier (LibreHardwareMonitor#1797)
* Remove retry + proper identifier * Update LibreHardwareMonitorLib.csproj * Revert "Update LibreHardwareMonitorLib.csproj" This reverts commit 185aa2d. * 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.
1 parent 8dc7d17 commit 06b714a

File tree

2 files changed

+144
-76
lines changed

2 files changed

+144
-76
lines changed

‎LibreHardwareMonitorLib/Hardware/Memory/MemoryGroup.cs‎

Lines changed: 140 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,13 @@
44
// Partial Copyright (C) Michael Möller <mmoeller@openhardwaremonitor.org> and Contributors.
55
// All Rights Reserved.
66

7+
using System;
78
using System.Collections.Generic;
8-
using System.Timers;
9+
using System.Diagnostics;
10+
using System.Linq;
11+
using System.Text;
12+
using System.Threading;
13+
using System.Threading.Tasks;
914
using RAMSPDToolkit.I2CSMBus;
1015
using RAMSPDToolkit.SPD;
1116
using RAMSPDToolkit.SPD.Enums;
@@ -15,133 +20,196 @@
1520

1621
namespace LibreHardwareMonitor.Hardware.Memory;
1722

18-
internal class MemoryGroup : IGroup
23+
internal class MemoryGroup : IGroup, IHardwareChanged
1924
{
20-
//Retry 12x
21-
private const int RetryCount = 12;
22-
23-
//Retry every 2.5 seconds
24-
private const double RetryTime = 2500;
2525
private static readonly object _lock = new();
26+
private List<Hardware> _hardware = [];
2627

27-
private readonly List<Hardware> _hardware = [];
28-
private int _elapsedCounter;
29-
30-
private Timer _timer;
28+
private CancellationTokenSource _cancellationTokenSource;
29+
private Exception _lastException;
30+
private bool _opened = false;
3131

32-
static MemoryGroup()
32+
public MemoryGroup(ISettings settings)
3333
{
34-
if (Ring0.IsOpen)
34+
if (Ring0.IsOpen && (DriverManager.Driver is null || !DriverManager.Driver.IsOpen))
3535
{
3636
// Assign implementation of IDriver.
3737
DriverManager.Driver = new RAMSPDToolkitDriver(Ring0.KernelDriver);
3838
SMBusManager.UseWMI = false;
3939
}
40-
else
41-
{
42-
// Still need to set Driver if Ring0 is absent.
43-
DriverManager.Driver = new RAMSPDToolkitDriver(null);
44-
SMBusManager.UseWMI = false;
45-
}
46-
}
4740

48-
public MemoryGroup(ISettings settings)
49-
{
5041
_hardware.Add(new VirtualMemory(settings));
5142
_hardware.Add(new TotalMemory(settings));
5243

53-
//No RAM detected
54-
if (!DetectThermalSensors(out List<SPDAccessor> accessors))
44+
if (DriverManager.Driver == null)
5545
{
56-
//Retry a couple of times
57-
//SMBus might not be detected right after boot
58-
_timer = new Timer(RetryTime);
59-
60-
_timer.Elapsed += (_, _) =>
61-
{
62-
if (_elapsedCounter++ >= RetryCount || DetectThermalSensors(out accessors))
63-
{
64-
_timer.Stop();
65-
_timer = null;
66-
67-
if (accessors != null)
68-
AddDimms(accessors, settings);
69-
}
70-
};
71-
72-
_timer.Start();
46+
return;
7347
}
74-
else
48+
49+
if (!TryAddDimms(settings))
7550
{
76-
AddDimms(accessors, settings);
51+
StartRetryTask(settings);
7752
}
53+
54+
_opened = true;
7855
}
7956

57+
public event HardwareEventHandler HardwareAdded;
58+
public event HardwareEventHandler HardwareRemoved;
59+
8060
public IReadOnlyList<IHardware> Hardware => _hardware;
8161

8262
public string GetReport()
8363
{
84-
return null;
64+
StringBuilder report = new();
65+
report.AppendLine("Memory Report:");
66+
if (_lastException != null)
67+
{
68+
report.AppendLine($"Error while detecting memory: {_lastException.Message}");
69+
}
70+
71+
foreach (Hardware hardware in _hardware)
72+
{
73+
report.AppendLine($"{hardware.Name} ({hardware.Identifier}):");
74+
report.AppendLine();
75+
foreach (ISensor sensor in hardware.Sensors)
76+
{
77+
report.AppendLine($"{sensor.Name}: {sensor.Value?.ToString() ?? "No value"}");
78+
}
79+
}
80+
81+
return report.ToString();
8582
}
8683

8784
public void Close()
8885
{
89-
foreach (Hardware ram in _hardware)
90-
ram.Close();
86+
lock (_lock)
87+
{
88+
_opened = false;
89+
foreach (Hardware ram in _hardware)
90+
ram.Close();
91+
92+
_hardware.Clear();
93+
94+
_cancellationTokenSource?.Cancel();
95+
_cancellationTokenSource?.Dispose();
96+
_cancellationTokenSource = null;
97+
}
9198
}
9299

93-
private static bool DetectThermalSensors(out List<SPDAccessor> accessors)
100+
private bool TryAddDimms(ISettings settings)
94101
{
95-
lock (_lock)
102+
try
96103
{
97-
var list = new List<SPDAccessor>();
104+
lock (_lock)
105+
{
106+
if (!_opened)
107+
{
108+
return true;
109+
}
98110

99-
bool ramDetected = false;
111+
if (DetectThermalSensors(out List<SPDAccessor> accessors))
112+
{
113+
AddDimms(accessors, settings);
114+
return true;
115+
}
116+
}
117+
}
118+
catch (Exception ex)
119+
{
120+
_lastException = ex;
121+
Debug.Assert(false, "Exception while detecting RAM: " + ex.Message);
122+
}
123+
124+
return false;
125+
}
126+
127+
private void StartRetryTask(ISettings settings)
128+
{
129+
_cancellationTokenSource = new CancellationTokenSource();
100130

101-
SMBusManager.DetectSMBuses();
131+
Task.Run(async () =>
132+
{
133+
int retryRemaining = 5;
102134

103-
//Go through detected SMBuses
104-
foreach (var smbus in SMBusManager.RegisteredSMBuses)
135+
while (!_cancellationTokenSource.IsCancellationRequested && --retryRemaining > 0)
105136
{
106-
//Go through possible RAM slots
107-
for (byte i = SPDConstants.SPD_BEGIN; i <= SPDConstants.SPD_END; ++i)
108-
{
109-
//Detect type of RAM, if available
110-
var detector = new SPDDetector(smbus, i);
137+
await Task.Delay(TimeSpan.FromSeconds(2.5), _cancellationTokenSource.Token).ConfigureAwait(false);
111138

112-
//RAM available and detected
113-
if (detector.Accessor != null)
139+
if (TryAddDimms(settings))
140+
{
141+
lock (_lock)
114142
{
115-
//We are only interested in modules with thermal sensor
116-
if (detector.Accessor is IThermalSensor { HasThermalSensor: true })
117-
list.Add(detector.Accessor);
143+
if (!_opened)
144+
{
145+
return;
146+
}
147+
148+
foreach (Hardware hardware in _hardware.OfType<DimmMemory>())
149+
{
150+
HardwareAdded?.Invoke(hardware);
151+
}
118152

119-
ramDetected = true;
153+
_cancellationTokenSource.Dispose();
154+
_cancellationTokenSource = null;
155+
156+
break;
120157
}
158+
121159
}
122160
}
161+
}, _cancellationTokenSource.Token);
162+
}
123163

124-
accessors = list.Count > 0 ? list : [];
125-
return ramDetected;
164+
private static bool DetectThermalSensors(out List<SPDAccessor> accessors)
165+
{
166+
accessors = [];
167+
168+
bool ramDetected = false;
169+
170+
SMBusManager.DetectSMBuses();
171+
172+
//Go through detected SMBuses
173+
foreach (SMBusInterface smbus in SMBusManager.RegisteredSMBuses)
174+
{
175+
//Go through possible RAM slots
176+
for (byte i = SPDConstants.SPD_BEGIN; i <= SPDConstants.SPD_END; ++i)
177+
{
178+
//Detect type of RAM, if available
179+
SPDDetector detector = new(smbus, i);
180+
181+
//RAM available and detected
182+
if (detector.Accessor != null)
183+
{
184+
//We are only interested in modules with thermal sensor
185+
if (detector.Accessor is IThermalSensor { HasThermalSensor: true })
186+
accessors.Add(detector.Accessor);
187+
188+
ramDetected = true;
189+
}
190+
}
126191
}
192+
193+
return ramDetected;
127194
}
128195

129196
private void AddDimms(List<SPDAccessor> accessors, ISettings settings)
130197
{
131-
foreach (var ram in accessors)
198+
List<Hardware> newHardwareList = [.. _hardware];
199+
200+
foreach (SPDAccessor ram in accessors)
132201
{
133202
//Default value
134203
string name = $"DIMM #{ram.Index}";
135204

136205
//Check if we can switch to the correct page
137206
if (ram.ChangePage(PageData.ModulePartNumber))
138-
{
139207
name = $"{ram.GetModuleManufacturerString()} - {ram.ModulePartNumber()} (#{ram.Index})";
140-
}
141208

142-
var memory = new DimmMemory(ram, name, new Identifier("ram"), settings);
143-
144-
_hardware.Add(memory);
209+
DimmMemory memory = new(ram, name, new Identifier($"memory/dimm/{ram.Index}"), settings);
210+
newHardwareList.Add(memory);
145211
}
212+
213+
_hardware = newHardwareList;
146214
}
147215
}

‎LibreHardwareMonitorLib/Hardware/RAMSPDToolkitDriver.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ internal class RAMSPDToolkitDriver : IDriver
1818
{
1919
private KernelDriver _kernelDriver;
2020

21-
private const byte PCI_MAX_NUMBER_OF_BUS = 255;
22-
private const byte PCI_NUMBER_OF_DEVICE = 32;
23-
private const byte PCI_NUMBER_OF_FUNCTION = 8;
21+
private const byte PCI_MAX_NUMBER_OF_BUS = 255;
22+
private const byte PCI_NUMBER_OF_DEVICE = 32;
23+
private const byte PCI_NUMBER_OF_FUNCTION = 8;
2424

25-
public bool IsOpen => _kernelDriver != null;
25+
public bool IsOpen => _kernelDriver?.IsOpen ?? false;
2626

2727
public RAMSPDToolkitDriver(KernelDriver kernelDriver)
2828
{

0 commit comments

Comments
 (0)