Skip to content

fix: WMI disposal, Ookla timeout, docs accuracy (CQ-001, CQ-003) #370

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
laurentiu021 merged 1 commit into from
May 15, 2026
Merged

fix: WMI disposal, Ookla timeout, docs accuracy (CQ-001, CQ-003) #370

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,20 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

## [0.48.10] - 2026-05-15

### Fixed
- **DiskHealthService (CQ-001)** — ManagementObjectCollection and ManagementObject
instances now properly disposed via `using` statements, preventing COM resource
leaks during SMART/reliability queries.
- **SpeedTestService (CQ-003)** — Ookla CLI process now has a 5-minute independent
timeout via linked CancellationTokenSource, preventing indefinite hangs.

### Changed
- **CONTRIBUTING.md** — corrected .NET SDK reference from 8 to 9; added
SysManager.IntegrationTests to project layout.
- **SECURITY.md** — updated supported versions table to reflect 0.48.x as latest.

## [0.48.9] - 2026-05-14

### Fixed
Expand Down
5 changes: 3 additions & 2 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ follows, and what to expect when you open a pull request.
**Prerequisites**

- Windows 10 or later (WPF + Windows APIs won't build elsewhere).
- [.NET 8 SDK](https://dotnet.microsoft.com/download/dotnet/8.0).
- [.NET 9 SDK](https://dotnet.microsoft.com/download/dotnet/9.0).
- Git 2.40+.
- A Windows account with admin rights (needed to test elevated features;
the app itself runs fine unelevated for everything else).
Expand Down Expand Up @@ -75,7 +75,8 @@ SysManager/
│ ├── Views/ # XAML + minimal code-behind
│ ├── Helpers/ # small utilities, converters
│ └── Resources/ # icons, generated assets
├── SysManager.Tests/ # xUnit unit + integration tests
├── SysManager.Tests/ # xUnit unit tests
├── SysManager.IntegrationTests/ # integration tests (local only, not CI)
└── SysManager.UITests/ # FlaUI UI-automation tests
```

Expand Down
7 changes: 3 additions & 4 deletions SECURITY.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@ older build, the first step is usually to update.

| Version | Supported |
| ------- | ------------------ |
| 0.48.x | :white_check_mark: |
| 0.47.x | :white_check_mark: |
| 0.46.x | :white_check_mark: |
| 0.45.x | :white_check_mark: |
| 0.44.x | :x: |
| < 0.44 | :x: |
| 0.46.x | :x: |
| < 0.46 | :x: |
Comment on lines +15 to +18

@coderabbitai coderabbitai Bot May 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align the support table with the written support policy.

Lines 15-18 now show two supported minors, but Line 10 still says fixes apply to the latest minor only. Please make them consistent to avoid ambiguity in security support commitments.

Suggested doc fix 
-Security fixes are applied to the latest minor release only. If you're on an
-older build, the first step is usually to update.
+Security fixes are applied to the latest two minor release lines. If you're on an
+older build, the first step is usually to update.
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SECURITY.md` around lines 15 - 18, The support table in SECURITY.md lists
0.48.x and 0.47.x as supported but the written policy line stating "fixes apply
to the latest minor only" is inconsistent; either update that sentence to state
that fixes apply to the latest two minors or change the table to mark only the
latest minor as supported. Locate the sentence containing "fixes apply to the
latest minor only" and either amend it to "latest two minor releases" (to match
rows 0.48.x and 0.47.x) or adjust the table rows (0.48.x, 0.47.x, 0.46.x) so
only the intended supported minor remains checked—ensure the wording and the
table entries (0.48.x, 0.47.x, 0.46.x) are consistent.


## Reporting a vulnerability

Expand Down
60 changes: 34 additions & 26 deletions SysManager/SysManager/Services/DiskHealthService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,28 @@ private static IReadOnlyList<DiskHealthReport> Collect()
// First pull MSFT_PhysicalDisk for basic info.
using var searcher = new ManagementObjectSearcher(scope,
new ObjectQuery("SELECT ObjectId, FriendlyName, MediaType, BusType, Size, HealthStatus FROM MSFT_PhysicalDisk"));
foreach (ManagementObject mo in searcher.Get())
using var collection = searcher.Get();
foreach (ManagementObject mo in collection)
{
var report = new DiskHealthReport
using (mo)
{
FriendlyName = mo["FriendlyName"]?.ToString() ?? "Disk",
MediaType = MapMedia(Convert.ToUInt32(mo["MediaType"] ?? 0u)),
BusType = MapBus(Convert.ToUInt32(mo["BusType"] ?? 0u)),
SizeGB = Math.Round(Convert.ToDouble(mo["Size"] ?? 0) / 1024d / 1024d / 1024d, 0),
HealthStatus = MapHealth(Convert.ToUInt32(mo["HealthStatus"] ?? 0u))
};

// Get reliability counters for this disk.
var objectId = mo["ObjectId"]?.ToString();
if (!string.IsNullOrEmpty(objectId))
EnrichWithReliability(scope, objectId, report);

ApplyVerdict(report);
results.Add(report);
var report = new DiskHealthReport
{
FriendlyName = mo["FriendlyName"]?.ToString() ?? "Disk",
MediaType = MapMedia(Convert.ToUInt32(mo["MediaType"] ?? 0u)),
BusType = MapBus(Convert.ToUInt32(mo["BusType"] ?? 0u)),
SizeGB = Math.Round(Convert.ToDouble(mo["Size"] ?? 0) / 1024d / 1024d / 1024d, 0),
HealthStatus = MapHealth(Convert.ToUInt32(mo["HealthStatus"] ?? 0u))
};

// Get reliability counters for this disk.
var objectId = mo["ObjectId"]?.ToString();
if (!string.IsNullOrEmpty(objectId))
EnrichWithReliability(scope, objectId, report);

ApplyVerdict(report);
results.Add(report);
}
}
}
catch (ManagementException)
Expand All @@ -69,17 +73,21 @@ private static void EnrichWithReliability(ManagementScope scope, string objectId
var query = new ObjectQuery(
$"ASSOCIATORS OF {{MSFT_PhysicalDisk.ObjectId=\"{safeId}\"}} WHERE AssocClass=MSFT_PhysicalDiskToStorageReliabilityCounter");
using var searcher = new ManagementObjectSearcher(scope, query);
foreach (ManagementObject mo in searcher.Get())
using var collection = searcher.Get();
foreach (ManagementObject mo in collection)
{
report.TemperatureC = ToDouble(mo["Temperature"]);
report.TemperatureMaxC = ToDouble(mo["TemperatureMax"]);
var wear = ToInt(mo["Wear"]);
if (wear.HasValue) report.WearPercent = wear;
report.PowerOnHours = ToLong(mo["PowerOnHours"]);
report.ReadErrors = ToLong(mo["ReadErrorsTotal"]);
report.WriteErrors = ToLong(mo["WriteErrorsTotal"]);
report.StartStopCount = ToLong(mo["StartStopCycleCount"]);
return; // one counter per disk
using (mo)
{
report.TemperatureC = ToDouble(mo["Temperature"]);
report.TemperatureMaxC = ToDouble(mo["TemperatureMax"]);
var wear = ToInt(mo["Wear"]);
if (wear.HasValue) report.WearPercent = wear;
report.PowerOnHours = ToLong(mo["PowerOnHours"]);
report.ReadErrors = ToLong(mo["ReadErrorsTotal"]);
report.WriteErrors = ToLong(mo["WriteErrorsTotal"]);
report.StartStopCount = ToLong(mo["StartStopCycleCount"]);
return; // one counter per disk
}
}
}
catch (ManagementException) { /* driver may not expose counters */ }
Expand Down
15 changes: 10 additions & 5 deletions SysManager/SysManager/Services/SpeedTestService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,13 +196,18 @@ public async Task<SpeedTestResult> RunOoklaAsync(
};

// Start the process on a thread-pool thread so Process.Start()
// never blocks the UI thread.
// never blocks the UI thread. Link a 5-minute timeout to prevent
// indefinite hangs if the Ookla CLI freezes (CQ-003).
using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(ct);
timeoutCts.CancelAfter(TimeSpan.FromMinutes(5));
var linked = timeoutCts.Token;

using var proc = new Process();
proc.StartInfo = psi;
await Task.Run(() => proc.Start(), ct).ConfigureAwait(false);
var stdout = await proc.StandardOutput.ReadToEndAsync(ct);
var stderr = await proc.StandardError.ReadToEndAsync(ct);
await proc.WaitForExitAsync(ct);
await Task.Run(() => proc.Start(), linked).ConfigureAwait(false);
var stdout = await proc.StandardOutput.ReadToEndAsync(linked);
var stderr = await proc.StandardError.ReadToEndAsync(linked);
await proc.WaitForExitAsync(linked);
Comment on lines +201 to +210

@coderabbitai coderabbitai Bot May 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify whether RunOoklaAsync has a cancellation path that kills the spawned process.
fd -i 'SpeedTestService.cs' | while read -r f; do
  echo "== $f =="
  rg -n -C4 'RunOoklaAsync|CreateLinkedTokenSource|WaitForExitAsync|OperationCanceledException|Kill\(' "$f"
done

Repository: laurentiu021/SystemManager

Length of output: 1315

🏁 Script executed:

#!/bin/bash
# Get the complete RunOoklaAsync method to verify exception handling
rg -n -A 60 'public async Task<SpeedTestResult> RunOoklaAsync' 'SysManager/SysManager/Services/SpeedTestService.cs' | head -100

Repository: laurentiu021/SystemManager

Length of output: 2759

Kill the Ookla process on timeout/cancellation to prevent orphan speedtest.exe instances.

Lines 207–210 await with the timeout token, but have no exception handler. If the 5-minute timeout triggers, OperationCanceledException propagates uncaught, leaving speedtest.exe running. The using statement on proc only disposes handles—it doesn't terminate the child process.

Proposed fix 
 using var proc = new Process();
 proc.StartInfo = psi;
-await Task.Run(() => proc.Start(), linked).ConfigureAwait(false);
-var stdout = await proc.StandardOutput.ReadToEndAsync(linked);
-var stderr = await proc.StandardError.ReadToEndAsync(linked);
-await proc.WaitForExitAsync(linked);
+try
+{
+    await Task.Run(() => proc.Start(), linked).ConfigureAwait(false);
+    var stdout = await proc.StandardOutput.ReadToEndAsync(linked);
+    var stderr = await proc.StandardError.ReadToEndAsync(linked);
+    await proc.WaitForExitAsync(linked);
+}
+catch (OperationCanceledException) when (timeoutCts.IsCancellationRequested && !ct.IsCancellationRequested)
+{
+    try { if (!proc.HasExited) proc.Kill(entireProcessTree: true); } catch { }
+    throw new TimeoutException("Ookla CLI timed out after 5 minutes.");
+}
+catch (OperationCanceledException)
+{
+    try { if (!proc.HasExited) proc.Kill(entireProcessTree: true); } catch { }
+    throw;
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(ct);
timeoutCts.CancelAfter(TimeSpan.FromMinutes(5));
var linked = timeoutCts.Token;
using var proc = new Process();
proc.StartInfo = psi;
await Task.Run(() => proc.Start(), ct).ConfigureAwait(false);
var stdout = await proc.StandardOutput.ReadToEndAsync(ct);
var stderr = await proc.StandardError.ReadToEndAsync(ct);
await proc.WaitForExitAsync(ct);
await Task.Run(() => proc.Start(), linked).ConfigureAwait(false);
var stdout = await proc.StandardOutput.ReadToEndAsync(linked);
var stderr = await proc.StandardError.ReadToEndAsync(linked);
await proc.WaitForExitAsync(linked);
using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(ct);
timeoutCts.CancelAfter(TimeSpan.FromMinutes(5));
var linked = timeoutCts.Token;
using var proc = new Process();
proc.StartInfo = psi;
try
{
await Task.Run(() => proc.Start(), linked).ConfigureAwait(false);
var stdout = await proc.StandardOutput.ReadToEndAsync(linked);
var stderr = await proc.StandardError.ReadToEndAsync(linked);
await proc.WaitForExitAsync(linked);
}
catch (OperationCanceledException) when (timeoutCts.IsCancellationRequested && !ct.IsCancellationRequested)
{
try { if (!proc.HasExited) proc.Kill(entireProcessTree: true); } catch { }
throw new TimeoutException("Ookla CLI timed out after 5 minutes.");
}
catch (OperationCanceledException)
{
try { if (!proc.HasExited) proc.Kill(entireProcessTree: true); } catch { }
throw;
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@SysManager/SysManager/Services/SpeedTestService.cs` around lines 201 - 210,
The awaits using linked cancellation token around proc.Start(),
proc.StandardOutput.ReadToEndAsync(linked),
proc.StandardError.ReadToEndAsync(linked), and proc.WaitForExitAsync(linked) can
throw OperationCanceledException on timeout but currently do not terminate the
child process; wrap these awaits in try/catch/finally (or try/finally with a
catch for OperationCanceledException) and in the finally check the Process
instance (proc) and if !proc.HasExited call proc.Kill(entireProcessTree: true)
(or proc.Kill() if older framework) and await a safe WaitForExit to ensure the
speedtest.exe is terminated before disposing; reference the existing
variables/procedures proc, psi, timeoutCts/linked, Start,
StandardOutput.ReadToEndAsync, StandardError.ReadToEndAsync, and
WaitForExitAsync when making the change.


if (proc.ExitCode != 0)
{
Expand Down
Loading