Skip to content

test: fix flaky thread-safety test, update README tech stack #433

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 2 commits into from
May 19, 2026
Merged

test: fix flaky thread-safety test, update README tech stack #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
48a70e3
test: fix flaky thread-safety test, update README tech stack
laurentiu021 May 19, 2026
2fb22aa
test: hold lock during concurrent acquisition attempts to prevent fal…
laurentiu021 May 19, 2026
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
9 changes: 9 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,15 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Fixed
- **OperationLockServiceTests** — replaced flaky `Barrier` + `Thread.Sleep`
thread-safety test with deterministic `CountdownEvent` + `ManualResetEventSlim`
synchronization; asserts exactly 1 acquisition instead of `>= 1`.

### Changed
- **README.md** — added missing tech stack entries: Microsoft.Extensions.DependencyInjection,
H.NotifyIcon.Wpf, NSubstitute.

## [0.48.34] - 2026-05-19

### Fixed
Expand Down
4 changes: 3 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,10 +452,12 @@ and elevation state in a format ready to paste into a bug report.

- .NET 9 (WPF, C# 13)
- CommunityToolkit.Mvvm for MVVM plumbing
- Microsoft.Extensions.DependencyInjection for IoC
- ModernWpfUI for the modern title bar
- LiveCharts2 for the real-time latency chart
- H.NotifyIcon.Wpf for system tray integration
- Serilog for structured logging
- xUnit and FlaUI for unit, integration, and UI-automation tests
- xUnit, NSubstitute, and FlaUI for unit, integration, and UI-automation tests

## Privacy

Expand Down
20 changes: 14 additions & 6 deletions SysManager/SysManager.Tests/OperationLockServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,20 +93,28 @@ public void GetActiveOperationName_ReturnsNull_WhenNotLocked()
public async Task TryAcquire_IsThreadSafe()
{
int successCount = 0;
var barrier = new Barrier(10);
var ready = new CountdownEvent(10);
var go = new ManualResetEventSlim(false);
Comment on lines +96 to +97

@coderabbitai coderabbitai Bot May 19, 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 | 🟡 Minor | ⚡ Quick win

Dispose synchronization primitives to prevent handle leaks.

CountdownEvent and ManualResetEventSlim both implement IDisposable and should be disposed to release their underlying kernel handles.

🛠️ Proposed fix 
-        var ready = new CountdownEvent(10);
-        var go = new ManualResetEventSlim(false);
+        using var ready = new CountdownEvent(10);
+        using var go = new ManualResetEventSlim(false);
📝 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
var ready = new CountdownEvent(10);
var go = new ManualResetEventSlim(false);
using var ready = new CountdownEvent(10);
using var go = new ManualResetEventSlim(false);
🤖 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.Tests/OperationLockServiceTests.cs` around lines 96 -
97, The test creates CountdownEvent (ready) and ManualResetEventSlim (go) which
implement IDisposable and must be disposed to avoid handle leaks; update the
test in OperationLockServiceTests (the scope where ready and go are created) to
ensure both ready and go are disposed—either wrap their creation in using blocks
(or nested using statements) or dispose them in a finally/tear-down block so
ready.Dispose() and go.Dispose() are always called after the test completes.

var allTried = new CountdownEvent(10);

var tasks = Enumerable.Range(0, 10).Select(_ => Task.Run(() =>
{
barrier.SignalAndWait();
using var handle = Sut.TryAcquire(OperationCategory.Network, "Race");
ready.Signal();
go.Wait();
var handle = Sut.TryAcquire(OperationCategory.SystemModification, "Race");
if (handle != null)
Interlocked.Increment(ref successCount);
Thread.Sleep(50);
// Wait until all threads have attempted acquisition before releasing
allTried.Signal();
allTried.Wait();
handle?.Dispose();
})).ToArray();

ready.Wait();
go.Set();
await Task.WhenAll(tasks);

// Only one thread should have acquired the lock at a time
Assert.True(successCount >= 1);
// Exactly one thread should have acquired the lock
Assert.Equal(1, successCount);
}
}
Loading