chore: code quality pass — modernize patterns, fix docs, add tests (QUAL-007)#468
Merged
Conversation
…UAL-007) - Seal all 27 ViewModels (none are subclassed) - Modernize null checks to pattern matching (is null / is not null) in 16 services - Replace new List<T>() with collection expressions in 14 services - Fix IDisposable: NetworkRepairService, SpeedTestHistoryService now dispose SemaphoreSlim - Fix test isolation: add CollectionDefinition for OperationLock singleton tests - Add BulkObservableCollectionTests (6 tests) and WingetTableParserTests (6 tests) - Fix docs: README badge, CONTRIBUTING, ARCHITECTURE, SECURITY versions → .NET 10 / v1.0.0 - Fix SysManager.csproj Version/FileVersion/AssemblyVersion → 1.0.0 - Fix AboutView.xaml.cs missing License header line
📝 WalkthroughWalkthroughComprehensive modernization passing: framework upgrade to .NET 10/C# 14, version bump to 1.0.0, security policy update to 1.0.x support, widespread service code refactoring applying pattern matching and collection expressions, new test suites, IDisposable resource management, and 24 sealed ViewModels. ChangesFramework and Release Modernization
Test Infrastructure and New Test Coverage
Service Code Modernization
Resource Management Enhancement
ViewModel Type Safety
Minor Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SysManager/SysManager/Services/SpeedTestHistoryService.cs (1)
17-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid disposing
_fileLockwhile SaveAsync/ClearAsync still in-flight.Both
SaveAsyncandClearAsyncfollow the pattern:await _fileLock.WaitAsync()→try→finally { _fileLock.Release() }. IfDispose()is called while either method is betweenWaitAsyncand thefinallyblock, theRelease()call will throwObjectDisposedException. This is a real race condition becauseDisposeis not thread-safe and does not synchronize with in-flight operations.Use a disposed flag with coordinated disposal to ensure no
Release()calls execute afterDispose().Proposed fix
public sealed class SpeedTestHistoryService : IDisposable { private readonly SemaphoreSlim _fileLock = new(1, 1); + private int _disposed; /// <inheritdoc /> - public void Dispose() => _fileLock.Dispose(); + public void Dispose() + { + if (Interlocked.Exchange(ref _disposed, 1) == 1) return; + _fileLock.Wait(); + _fileLock.Dispose(); + } + + private void ThrowIfDisposed() + { + if (Volatile.Read(ref _disposed) == 1) + throw new ObjectDisposedException(nameof(SpeedTestHistoryService)); + }🤖 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/SpeedTestHistoryService.cs` around lines 17 - 38, Add coordinated disposal: introduce a volatile bool _disposed, an atomic counter (e.g. int _activeOps) and a TaskCompletionSource _noActiveOpsTcs; in SaveAsync and ClearAsync increment _activeOps (Interlocked.Increment) before calling _fileLock.WaitAsync(), check _disposed after increment and if set decrement and throw ObjectDisposedException, and in the finally block decrement _activeOps (Interlocked.Decrement) and if it reaches 0 and _disposed is true call _noActiveOpsTcs.TrySetResult(true); in Dispose set _disposed = true and if Interlocked.CompareExchange(ref _activeOps, _activeOps, _activeOps) > 0 await _noActiveOpsTcs.Task before calling _fileLock.Dispose() (or await _fileLock.WaitAsync/Release if you prefer) so no in-flight SaveAsync/ClearAsync will later call _fileLock.Release() after disposal.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@SysManager/SysManager/Services/NetworkRepairService.cs`:
- Around line 13-22: NetworkRepairService.Dispose currently disposes the
SemaphoreSlim _gate immediately, which can race with in-flight async operations
that call _gate.Release and cause ObjectDisposedException; fix by adding a
private volatile bool _disposed (or use int _disposedFlag), update public
Dispose() to set the disposed flag and then wait to enter the semaphore (e.g.
Acquire/Wait) before calling _gate.Dispose(), and ensure every async path that
uses _gate checks the disposed flag before waiting/releasing (throw
ObjectDisposedException or no-op release if disposed) so that Release() is never
called on a disposed semaphore; reference symbols: class NetworkRepairService,
field _gate, method Dispose().
---
Outside diff comments:
In `@SysManager/SysManager/Services/SpeedTestHistoryService.cs`:
- Around line 17-38: Add coordinated disposal: introduce a volatile bool
_disposed, an atomic counter (e.g. int _activeOps) and a TaskCompletionSource
_noActiveOpsTcs; in SaveAsync and ClearAsync increment _activeOps
(Interlocked.Increment) before calling _fileLock.WaitAsync(), check _disposed
after increment and if set decrement and throw ObjectDisposedException, and in
the finally block decrement _activeOps (Interlocked.Decrement) and if it reaches
0 and _disposed is true call _noActiveOpsTcs.TrySetResult(true); in Dispose set
_disposed = true and if Interlocked.CompareExchange(ref _activeOps, _activeOps,
_activeOps) > 0 await _noActiveOpsTcs.Task before calling _fileLock.Dispose()
(or await _fileLock.WaitAsync/Release if you prefer) so no in-flight
SaveAsync/ClearAsync will later call _fileLock.Release() after disposal.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2708934a-5dc1-4bfc-9b40-be18c77574ee
📒 Files selected for processing (62)
ARCHITECTURE.mdCONTRIBUTING.mdREADME.mdSECURITY.mdSysManager/SysManager.Tests/BulkObservableCollectionTests.csSysManager/SysManager.Tests/OperationLockServiceTests.csSysManager/SysManager.Tests/TestCollections.csSysManager/SysManager.Tests/WingetTableParserTests.csSysManager/SysManager/Services/AppAlertService.csSysManager/SysManager/Services/AppBlockerService.csSysManager/SysManager/Services/DeepCleanupService.csSysManager/SysManager/Services/DiskHealthService.csSysManager/SysManager/Services/EventLogService.csSysManager/SysManager/Services/HealthScoreService.csSysManager/SysManager/Services/IconExtractorService.csSysManager/SysManager/Services/MemoryTestService.csSysManager/SysManager/Services/NetworkRepairService.csSysManager/SysManager/Services/PerformanceService.csSysManager/SysManager/Services/PingMonitorService.csSysManager/SysManager/Services/PowerShellRunner.csSysManager/SysManager/Services/ProcessDescriptionService.csSysManager/SysManager/Services/ShortcutCleanerService.csSysManager/SysManager/Services/SpeedTestHistoryService.csSysManager/SysManager/Services/SpeedTestService.csSysManager/SysManager/Services/StartupService.csSysManager/SysManager/Services/SystemInfoService.csSysManager/SysManager/Services/TracerouteService.csSysManager/SysManager/Services/TrayIconService.csSysManager/SysManager/Services/TuneUpService.csSysManager/SysManager/Services/UninstallerService.csSysManager/SysManager/Services/UpdateService.csSysManager/SysManager/Services/WindowsFeaturesService.csSysManager/SysManager/Services/WingetService.csSysManager/SysManager/SysManager.csprojSysManager/SysManager/ViewModels/AboutViewModel.csSysManager/SysManager/ViewModels/AppAlertsViewModel.csSysManager/SysManager/ViewModels/AppBlockerViewModel.csSysManager/SysManager/ViewModels/AppUpdatesViewModel.csSysManager/SysManager/ViewModels/BatteryHealthViewModel.csSysManager/SysManager/ViewModels/CleanupViewModel.csSysManager/SysManager/ViewModels/ConsoleViewModel.csSysManager/SysManager/ViewModels/DashboardViewModel.csSysManager/SysManager/ViewModels/DeepCleanupViewModel.csSysManager/SysManager/ViewModels/DiskAnalyzerViewModel.csSysManager/SysManager/ViewModels/DriversViewModel.csSysManager/SysManager/ViewModels/DuplicateFileViewModel.csSysManager/SysManager/ViewModels/LogsViewModel.csSysManager/SysManager/ViewModels/MainWindowViewModel.csSysManager/SysManager/ViewModels/NetworkRepairViewModel.csSysManager/SysManager/ViewModels/PerformanceViewModel.csSysManager/SysManager/ViewModels/PingViewModel.csSysManager/SysManager/ViewModels/ProcessManagerViewModel.csSysManager/SysManager/ViewModels/ServicesViewModel.csSysManager/SysManager/ViewModels/ShortcutCleanerViewModel.csSysManager/SysManager/ViewModels/SpeedTestViewModel.csSysManager/SysManager/ViewModels/StartupViewModel.csSysManager/SysManager/ViewModels/SystemHealthViewModel.csSysManager/SysManager/ViewModels/TracerouteViewModel.csSysManager/SysManager/ViewModels/UninstallerViewModel.csSysManager/SysManager/ViewModels/WindowsFeaturesViewModel.csSysManager/SysManager/ViewModels/WindowsUpdateViewModel.csSysManager/SysManager/Views/AboutView.xaml.cs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build & unit tests
- GitHub Check: Analyze (csharp)
🔇 Additional comments (61)
SysManager/SysManager/Views/AboutView.xaml.cs (1)
3-3: LGTM!ARCHITECTURE.md (1)
3-3: LGTM!CONTRIBUTING.md (1)
42-42: LGTM!README.md (1)
18-18: LGTM!SECURITY.md (1)
15-16: LGTM!SysManager/SysManager/SysManager.csproj (1)
13-15: LGTM!SysManager/SysManager/Services/AppAlertService.cs (1)
130-130: LGTM!Also applies to: 137-137, 204-205, 212-212
SysManager/SysManager/Services/AppBlockerService.cs (1)
45-45: LGTM!Also applies to: 90-90, 93-93, 96-96, 140-140, 143-143, 146-146, 163-163, 170-170, 173-173
SysManager/SysManager/Services/DeepCleanupService.cs (1)
269-269: LGTM!Also applies to: 283-283, 319-319, 404-404
SysManager/SysManager/Services/DiskHealthService.cs (1)
149-149: LGTM!Also applies to: 163-163, 172-172, 181-181
SysManager/SysManager/Services/EventLogService.cs (1)
51-51: LGTM!Also applies to: 59-59, 136-136
SysManager/SysManager/Services/HealthScoreService.cs (1)
103-103: LGTM!Also applies to: 118-118, 137-137, 153-153, 177-177, 180-180, 191-191, 203-203, 213-213
SysManager/SysManager/Services/IconExtractorService.cs (1)
79-79: LGTM!Also applies to: 87-87, 120-120, 135-135, 274-274, 295-295, 376-376, 393-393
SysManager/SysManager/Services/MemoryTestService.cs (1)
61-62: LGTM!SysManager/SysManager/Services/PerformanceService.cs (1)
93-93: LGTM!Also applies to: 147-148, 169-169, 216-216, 240-240, 295-295, 327-327, 329-329, 341-341, 343-343, 379-379, 386-386, 425-425, 445-445, 470-470, 616-616
SysManager/SysManager/Services/PingMonitorService.cs (1)
133-133: LGTM!SysManager/SysManager/Services/PowerShellRunner.cs (1)
76-76: LGTM!SysManager/SysManager/Services/ProcessDescriptionService.cs (1)
108-108: LGTM!Also applies to: 115-115, 126-126
SysManager/SysManager/Services/ShortcutCleanerService.cs (1)
106-106: LGTM!SysManager/SysManager/Services/SpeedTestService.cs (1)
60-60: LGTM!Also applies to: 391-391
SysManager/SysManager/Services/StartupService.cs (1)
99-99: LGTM!Also applies to: 120-121, 155-155, 162-162, 165-165, 225-225, 280-280, 296-296, 331-331, 416-416
SysManager/SysManager/Services/SystemInfoService.cs (1)
151-151: LGTM!Also applies to: 174-174
SysManager/SysManager/Services/TracerouteService.cs (1)
35-35: LGTM!Also applies to: 72-72, 102-102
SysManager/SysManager/Services/TrayIconService.cs (1)
155-155: LGTM!Also applies to: 205-205
SysManager/SysManager/Services/TuneUpService.cs (1)
84-84: LGTM!SysManager/SysManager/Services/UninstallerService.cs (1)
31-31: LGTM!Also applies to: 128-128, 141-142, 157-157, 189-189
SysManager/SysManager/Services/UpdateService.cs (1)
72-72: LGTM!Also applies to: 104-104, 310-310, 331-331, 334-334
SysManager/SysManager/Services/WindowsFeaturesService.cs (1)
27-27: LGTM!Also applies to: 58-58, 92-92
SysManager/SysManager/Services/WingetService.cs (1)
30-30: LGTM!SysManager/SysManager/ViewModels/AboutViewModel.cs (1)
20-20: LGTM!SysManager/SysManager/ViewModels/AppAlertsViewModel.cs (1)
20-20: LGTM!SysManager/SysManager/ViewModels/AppBlockerViewModel.cs (1)
20-20: LGTM!SysManager/SysManager/ViewModels/AppUpdatesViewModel.cs (1)
15-15: LGTM!SysManager/SysManager/ViewModels/BatteryHealthViewModel.cs (1)
17-17: LGTM!SysManager/SysManager/ViewModels/CleanupViewModel.cs (1)
15-15: LGTM!SysManager/SysManager/ViewModels/ConsoleViewModel.cs (1)
17-17: LGTM!SysManager/SysManager/ViewModels/DashboardViewModel.cs (1)
15-15: LGTM!SysManager/SysManager/ViewModels/DeepCleanupViewModel.cs (1)
17-17: LGTM!SysManager/SysManager/ViewModels/DiskAnalyzerViewModel.cs (1)
20-20: LGTM!SysManager/SysManager/ViewModels/DriversViewModel.cs (1)
17-17: LGTM!SysManager/SysManager/ViewModels/DuplicateFileViewModel.cs (1)
21-21: LGTM!SysManager/SysManager/ViewModels/LogsViewModel.cs (1)
26-26: LGTM!SysManager/SysManager/ViewModels/MainWindowViewModel.cs (1)
15-15: LGTM!SysManager/SysManager/ViewModels/NetworkRepairViewModel.cs (1)
15-15: LGTM!SysManager/SysManager/ViewModels/PerformanceViewModel.cs (1)
25-25: LGTM!SysManager/SysManager/ViewModels/PingViewModel.cs (1)
16-16: LGTM!SysManager/SysManager/ViewModels/ProcessManagerViewModel.cs (1)
19-19: LGTM!SysManager/SysManager/ViewModels/ServicesViewModel.cs (1)
21-21: LGTM!SysManager/SysManager/ViewModels/ShortcutCleanerViewModel.cs (1)
18-18: LGTM!SysManager/SysManager/ViewModels/SpeedTestViewModel.cs (1)
16-16: LGTM!SysManager/SysManager/ViewModels/StartupViewModel.cs (1)
19-19: LGTM!SysManager/SysManager/ViewModels/SystemHealthViewModel.cs (1)
18-18: LGTM!SysManager/SysManager/ViewModels/TracerouteViewModel.cs (1)
17-17: LGTM!SysManager/SysManager/ViewModels/UninstallerViewModel.cs (1)
17-17: LGTM!SysManager/SysManager/ViewModels/WindowsFeaturesViewModel.cs (1)
19-19: LGTM!SysManager/SysManager/ViewModels/WindowsUpdateViewModel.cs (1)
17-17: LGTM!SysManager/SysManager/Services/SpeedTestHistoryService.cs (1)
55-55: LGTM!Also applies to: 134-134
SysManager/SysManager.Tests/OperationLockServiceTests.cs (1)
10-10: LGTM!SysManager/SysManager.Tests/TestCollections.cs (1)
14-19: LGTM!SysManager/SysManager.Tests/WingetTableParserTests.cs (1)
1-113: LGTM!SysManager/SysManager.Tests/BulkObservableCollectionTests.cs (1)
1-90: LGTM!
Comment on lines
+13
to
+22
| public sealed class NetworkRepairService : IDisposable | ||
| { | ||
| private readonly PowerShellRunner _ps; | ||
| private readonly SemaphoreSlim _gate = new(1, 1); | ||
|
|
||
| public NetworkRepairService(PowerShellRunner ps) => _ps = ps; | ||
|
|
||
| /// <inheritdoc /> | ||
| public void Dispose() => _gate.Dispose(); | ||
|
|
There was a problem hiding this comment.
❓ Verification inconclusive
In .NET, is SemaphoreSlim.Dispose safe when concurrent WaitAsync/Release may still occur, and what disposal pattern is recommended to avoid ObjectDisposedException?
No, calling SemaphoreSlim.Dispose while concurrent WaitAsync or Release operations are ongoing is not thread-safe [1][2][3]. Official Microsoft documentation explicitly states that all public and protected members of SemaphoreSlim are thread-safe, with the sole exception of Dispose, which must be used only when all other operations on the instance have completed [1][2]. Attempting to dispose of the semaphore while other threads are waiting or attempting to release can lead to race conditions, including tasks hanging indefinitely or throwing unexpected ObjectDisposedException errors [4]. Recommended Disposal Pattern To safely dispose of a SemaphoreSlim and avoid ObjectDisposedException, you must ensure that no further operations will attempt to use the instance before calling Dispose. The following strategies are recommended: 1. Use Cooperative Cancellation: Before disposing, signal any waiting tasks to cancel their operations using a CancellationToken [5][6]. Ensure your code handles OperationCanceledException and stops calling WaitAsync or Release on that semaphore instance [5]. 2. Track Pending Operations: Maintain a mechanism to track active tasks or operations that use the semaphore. Before calling Dispose, you must guarantee these operations have finished [5]. For example, you can use a list of tasks or a reference counter to track active work and await their completion: // Example logic for safe disposal public void Dispose { // 1. Signal cancellation if applicable _cancellationTokenSource.Cancel; // 2. Wait for all tracked tasks to complete before disposing Task.WaitAll(_activeTasks.ToArray); // 3. Finally, dispose the semaphore _semaphore.Dispose; } 3. Avoid Concurrent Access During Teardown: If your object owns the SemaphoreSlim, ensure the disposing object's lifecycle logic prevents new calls to WaitAsync. Do not rely on "dirty workarounds" like skipping Dispose, as failing to dispose objects that use unmanaged resources (such as when accessing AvailableWaitHandle) can lead to resource leaks [7][8]. Always treat SemaphoreSlim as a standard IDisposable object and manage its lifetime within your class's own Dispose pattern [7][8].
Citations:
- 1: https://learn.microsoft.com/en-us/dotnet/api/system.threading.semaphoreslim?view=netframework-4.8.1
- 2: https://source.dot.net/System.Private.CoreLib/src/libraries/System.Private.CoreLib/src/System/Threading/SemaphoreSlim.cs.html
- 3: https://learn.microsoft.com/en-us/dotnet/api/system.threading.semaphoreslim.dispose?view=net-9.0
- 4: SemaphoreSlim instances should be disposed in a thread-safe manner npgsql/npgsql#6115
- 5: https://stackoverflow.com/questions/67732623/is-it-safe-to-dispose-a-semaphoreslim-while-waiting-for-pending-operations-to-ca
- 6: https://codereview.stackexchange.com/questions/215837/semaphoreslim-extension-method-for-safely-handling-cancellation-and-disposal
- 7: https://stackoverflow.com/questions/32033416/do-i-need-to-dispose-a-semaphoreslim
- 8: https://stackoverflow.com/questions/32033416/do-i-need-to-dispose-a-semaphoreslim/32033463
Script executed:
cat -n SysManager/SysManager/Services/NetworkRepairService.csRepository: laurentiu021/SystemManager
Repository: laurentiu021/SystemManager
Exit code: 0
stdout:
1 // SysManager · NetworkRepairService
2 // Author: laurentiu021 · https://github.com/laurentiu021/SystemManager
3 // License: MIT
4
5 using SysManager.Models;
6
7 namespace SysManager.Services;
8
9 /// <summary>
10 /// Runs common network repair commands: DNS flush, Winsock reset, TCP/IP reset.
11 /// Each method captures stdout/stderr and returns a <see cref="NetworkRepairResult"/>.
12 /// </summary>
13 public sealed class NetworkRepairService : IDisposable
14 {
15 private readonly PowerShellRunner _ps;
16 private readonly SemaphoreSlim _gate = new(1, 1);
17
18 public NetworkRepairService(PowerShellRunner ps) => _ps = ps;
19
20 /// <inheritdoc />
21 public void Dispose() => _gate.Dispose();
22
23 /// <summary>
24 /// Flush the DNS resolver cache. Does not require a reboot.
25 /// </summary>
26 public async Task<NetworkRepairResult> FlushDnsAsync(CancellationToken ct = default)
27 {
28 await _gate.WaitAsync(ct).ConfigureAwait(false);
29 var output = new System.Collections.Concurrent.ConcurrentQueue<string>();
30 void OnLine(PowerShellLine line) => output.Enqueue(line.Text);
31 _ps.LineReceived += OnLine;
32 try
33 {
34 var exit = await _ps.RunProcessAsync("ipconfig.exe", "/flushdns", ct, PowerShellRunner.OemEncoding)
35 .ConfigureAwait(false);
36 return new NetworkRepairResult(
37 "DNS Flush",
38 exit == 0,
39 string.Join(Environment.NewLine, output),
40 NeedsReboot: false);
41 }
42 finally { _ps.LineReceived -= OnLine; _gate.Release(); }
43 }
44
45 /// <summary>
46 /// Reset the Winsock catalog. Requires a reboot to take effect.
47 /// </summary>
48 public async Task<NetworkRepairResult> ResetWinsockAsync(CancellationToken ct = default)
49 {
50 await _gate.WaitAsync(ct).ConfigureAwait(false);
51 var output = new System.Collections.Concurrent.ConcurrentQueue<string>();
52 void OnLine(PowerShellLine line) => output.Enqueue(line.Text);
53 _ps.LineReceived += OnLine;
54 try
55 {
56 var exit = await _ps.RunProcessAsync("netsh.exe", "winsock reset", ct, PowerShellRunner.OemEncoding)
57 .ConfigureAwait(false);
58 return new NetworkRepairResult(
59 "Winsock Reset",
60 exit == 0,
61 string.Join(Environment.NewLine, output),
62 NeedsReboot: true);
63 }
64 finally { _ps.LineReceived -= OnLine; _gate.Release(); }
65 }
66
67 /// <summary>
68 /// Reset the TCP/IP stack. Requires a reboot to take effect.
69 /// </summary>
70 public async Task<NetworkRepairResult> ResetTcpIpAsync(CancellationToken ct = default)
71 {
72 await _gate.WaitAsync(ct).ConfigureAwait(false);
73 var output = new System.Collections.Concurrent.ConcurrentQueue<string>();
74 void OnLine(PowerShellLine line) => output.Enqueue(line.Text);
75 _ps.LineReceived += OnLine;
76 try
77 {
78 var exit = await _ps.RunProcessAsync("netsh.exe", "int ip reset", ct, PowerShellRunner.OemEncoding)
79 .ConfigureAwait(false);
80 return new NetworkRepairResult(
81 "TCP/IP Reset",
82 exit == 0,
83 string.Join(Environment.NewLine, output),
84 NeedsReboot: true);
85 }
86 finally { _ps.LineReceived -= OnLine; _gate.Release(); }
87 }
88 }
Synchronize semaphore disposal with in-flight async operations.
Dispose() immediately disposes _gate without coordination; if an async method is in-flight, its finally block will call _gate.Release() on a disposed semaphore, causing ObjectDisposedException. This is unsafe per .NET threading guarantees.
Implement disposal synchronization using a disposed flag and acquire the semaphore before disposal:
Proposed fix
public sealed class NetworkRepairService : IDisposable
{
private readonly PowerShellRunner _ps;
private readonly SemaphoreSlim _gate = new(1, 1);
+ private int _disposed;
public NetworkRepairService(PowerShellRunner ps) => _ps = ps;
/// <inheritdoc />
- public void Dispose() => _gate.Dispose();
+ public void Dispose()
+ {
+ if (Interlocked.Exchange(ref _disposed, 1) == 1) return;
+ _gate.Wait();
+ _gate.Dispose();
+ }
+
+ private void ThrowIfDisposed()
+ {
+ if (Volatile.Read(ref _disposed) == 1)
+ throw new ObjectDisposedException(nameof(NetworkRepairService));
+ }
}🤖 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/NetworkRepairService.cs` around lines 13 - 22,
NetworkRepairService.Dispose currently disposes the SemaphoreSlim _gate
immediately, which can race with in-flight async operations that call
_gate.Release and cause ObjectDisposedException; fix by adding a private
volatile bool _disposed (or use int _disposedFlag), update public Dispose() to
set the disposed flag and then wait to enter the semaphore (e.g. Acquire/Wait)
before calling _gate.Dispose(), and ensure every async path that uses _gate
checks the disposed flag before waiting/releasing (throw ObjectDisposedException
or no-op release if disposed) so that Release() is never called on a disposed
semaphore; reference symbols: class NetworkRepairService, field _gate, method
Dispose().
laurentiu021
added a commit
that referenced
this pull request
May 22, 2026
…bs (#468) ## Summary Reorganizes the sidebar navigation from 7 groups / 21 tabs to **9 groups / 36 tabs**, adding WIP placeholder tabs for all planned features so the structure is final and future PRs only need to replace the placeholder with the real implementation. ## Changes ### New groups - **Monitor** (4 tabs): Process Manager (moved from System), Resource History, App Alerts, Privacy Monitor - **Control** (5 tabs): Privacy Settings, Context Menu, Restore Points, Scheduled Maintenance, System Report ### Expanded groups - **System** +1: Windows Features - **Cleanup** +2: Shortcut Cleaner, File Shredder - **Network** +2: DNS Changer, Hosts Editor - **Apps** +2: Bulk Installer, App Blocker ### New files - \PlaceholderViewModel.cs\ — lightweight VM storing feature name, description, issue number - \PlaceholderView.xaml\ + code-behind — WIP page with feature name, badge, description, issue ref - \PlaceholderViewModelTests.cs\ — 4 unit tests ### Updated files - \MainWindowViewModel.cs\ — 9 groups, 36 tabs, 15 WIP placeholder instances - \MainWindowViewModelTests.cs\ (IntegrationTests) — updated assertions for new counts - \CHANGELOG.md\ — v0.29.0 entry ## Testing - Build: 0 errors (main project + tests) - All existing tests unaffected (PlaceholderViewModel is additive) - 4 new unit tests for PlaceholderViewModel Closes #393 Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
laurentiu021
added a commit
that referenced
this pull request
May 22, 2026
Updates documentation to reflect the new sidebar structure from PR #468 (v0.29.0). - README: sidebar table updated from 7 groups / 21 tabs to 9 groups / 36 tabs, with ⚙️ markers for WIP tabs - ARCHITECTURE: tabs table updated with new groups (Monitor, Control) and PlaceholderViewModel references Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
laurentiu021
added a commit
that referenced
this pull request
May 22, 2026
…UAL-007) (#468) - Seal all 27 ViewModels (none are subclassed) - Modernize null checks to pattern matching (is null / is not null) in 16 services - Replace new List<T>() with collection expressions in 14 services - Fix IDisposable: NetworkRepairService, SpeedTestHistoryService now dispose SemaphoreSlim - Fix test isolation: add CollectionDefinition for OperationLock singleton tests - Add BulkObservableCollectionTests (6 tests) and WingetTableParserTests (6 tests) - Fix docs: README badge, CONTRIBUTING, ARCHITECTURE, SECURITY versions → .NET 10 / v1.0.0 - Fix SysManager.csproj Version/FileVersion/AssemblyVersion → 1.0.0 - Fix AboutView.xaml.cs missing License header line Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive quality pass addressing audit findings:
Fixes
Modernization (62 files)
is null/is not null) in Servicesnew List<T>()with[]collection expressions in ServicesNew Tests
BulkObservableCollectionTests— 6 tests (ReplaceWith behavior, notifications, edge cases)WingetTableParserTests— 6 tests (empty input, no header, valid table, short lines, summary stop)Test plan
Summary by CodeRabbit
Documentation
New Features
Tests
Chores