Skip to content

Commit d6b0ced

Browse files
Fix race condition in ReactiveCommand cancellation test (#4196)
Fixes an intermittent test failure in `ReactiveCommandCreateFromTaskThenCancelSetsIsExecutingFalseOnlyAfterCancellationCompleteAsync` caused by a race condition. ## Root Cause The test was experiencing intermittent failures where `IsExecuting` would be `False` when expected to be `True`. This occurred due to a race condition where the subscription to the `IsExecuting` observable might not receive the initial value before the command starts executing, causing the first `IsExecuting = true` notification to be missed. ## Changes Made Added synchronization using `TaskCompletionSource` to wait for the `IsExecuting` observable to emit its first value before calling `Execute()`. This ensures the subscription is fully established and has received the initial state before execution begins, eliminating the race condition reliably. The fix waits for the first emission from `IsExecuting` (which should be `false` initially) before starting command execution, guaranteeing that subsequent emissions (including the `true` value when execution starts) will be captured by the subscription. ## Testing - ✅ Ran the failing test 30 times - all passed - ✅ Ran all 72 ReactiveCommand tests - all passed (1 skipped) - ✅ Build succeeds with .NET 8, 9, and 10 target frameworks The fix is minimal and surgical, affecting only the test timing to ensure reliable execution without changing any production code or test assertions. <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > ``` > Skipped ReactiveCommandCreateFromTaskHandlesExecuteCancellation [< 1 ms] > Failed ReactiveCommandCreateFromTaskThenCancelSetsIsExecutingFalseOnlyAfterCancellationCompleteAsync [14 ms] > Error Message: > IsExecuting should be true when execution is underway > Assert.That(Volatile.Read(ref latestIsExecutingValue), Is.True) > Expected: True > But was: False > > Stack Trace: > at ReactiveUI.Tests.ReactiveCommandTest.ReactiveCommandCreateFromTaskThenCancelSetsIsExecutingFalseOnlyAfterCancellationCompleteAsync() in /home/runner/work/ReactiveUI/ReactiveUI/src/ReactiveUI.Tests/Commands/ReactiveCommandTest.cs:line 1826 > at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.BlockUntilCompleted() > at NUnit.Framework.Internal.MessagePumpStrategy.NoMessagePumpStrategy.WaitForCompletion(AwaitAdapter awaiter) > at NUnit.Framework.Internal.AsyncToSyncAdapter.Await[TResult](TestExecutionContext context, Func`1 invoke) > at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(TestExecutionContext context, Func`1 invoke) > at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context) > at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context) > at NUnit.Framework.Internal.Commands.BeforeAndAfterTestCommand.<>c__DisplayClass1_0.<Execute>b__0() > at NUnit.Framework.Internal.Commands.DelegatingTestCommand.RunTestMethodInThreadAbortSafeZone(TestExecutionContext context, Action action) > > 1) at ReactiveUI.Tests.ReactiveCommandTest.ReactiveCommandCreateFromTaskThenCancelSetsIsExecutingFalseOnlyAfterCancellationCompleteAsync() in /home/runner/work/ReactiveUI/ReactiveUI/src/ReactiveUI.Tests/Commands/ReactiveCommandTest.cs:line 1826 > at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.ExecutionContextCallback(Object s) > ``` > > Fix the build for the errors above, these are intermetient failures, likely need a NoParallezize flag on the build, possibly a helper class similar to those already in the Tests project to reset state > > PREREQS needed - do these before anything else > 1. Install .net8/9/10 before starting via the microsoft install script. > 2. Unshallow the commits > 3. dotnet Workload restore in the /src folder. </details> <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/reactiveui/ReactiveUI/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: glennawatson <[email protected]>
1 parent 221a25e commit d6b0ced

File tree

1 file changed

+13
-0
lines changed

1 file changed

+13
-0
lines changed

src/ReactiveUI.Tests/Commands/ReactiveCommandTest.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1811,14 +1811,27 @@ await Task.Delay(
18111811
fixture.ThrownExceptions.Subscribe(_ => { });
18121812

18131813
var latestIsExecutingValue = false;
1814+
var subscriptionReady = new TaskCompletionSource<bool>();
1815+
var isFirstValue = true;
1816+
18141817
fixture.IsExecuting.Subscribe(isExec =>
18151818
{
18161819
statusTrail.Add((position++, $"command executing = {isExec}"));
18171820
Volatile.Write(
18181821
ref latestIsExecutingValue,
18191822
isExec);
1823+
1824+
// Signal that we've received the first value (should be false initially)
1825+
if (isFirstValue)
1826+
{
1827+
isFirstValue = false;
1828+
subscriptionReady.TrySetResult(true);
1829+
}
18201830
});
18211831

1832+
// Wait for the subscription to receive the initial value
1833+
await subscriptionReady.Task;
1834+
18221835
var disposable = fixture.Execute().Subscribe();
18231836

18241837
// Phase 1

0 commit comments

Comments
 (0)