Skip to content

Commit c2ff608

Browse files
linkdotnetlinkdotnet
committed
fix: OnTimeoutElapsed crashes / refactor to use upstream version (fixes #1920)
1 parent 5439727 commit c2ff608

3 files changed

Lines changed: 175 additions & 51 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ All notable changes to **bUnit** will be documented in this file. The project ad
99
### Fixed
1010

1111
- `InvokeOnSpacerBeforeVisible` now uses 4 parameters on .NET 11.0. Reported by [@vnbaaij](https://github.com/vnbaaij) in #1915. Fixed by [@vnbaaij](https://github.com/vnbaaij) in #1919.
12+
- A JSInterop timeout elapsing while a result was set could crash the test host with `InvalidOperationException: Nullable object must have a value`. Reported by [@calebcwells](https://github.com/calebcwells) in [#1920](https://github.com/bUnit-dev/bUnit/issues/1920). Fixed by [@linkdotnet](https://github.com/linkdotnet).
1213

1314
## [2.10.3] - 2026-09-08
1415

‎src/bunit/JSInterop/InvocationHandlers/JSRuntimeInvocationHandlerBase{TResult}.cs‎

Lines changed: 82 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,20 @@
1+
using System.Collections.Concurrent;
2+
13
namespace Bunit;
24

5+
// Invocation tracking mirrors ASP.NET Core's JSRuntime: no per-invocation state lives in instance
6+
// fields. Each call gets its own TaskCompletionSource in a ConcurrentDictionary keyed by an
7+
// Interlocked id, and the timeout closes over that entry alone, so an elapsing timeout can never
8+
// race a concurrently set result. See https://github.com/dotnet/aspnetcore/blob/main/src/JSInterop/Microsoft.JSInterop/src/JSRuntime.cs
39
/// <summary>
410
/// Represents an invocation handler for <see cref="JSRuntimeInvocation"/> instances.
511
/// </summary>
612
public abstract class JSRuntimeInvocationHandlerBase<TResult> : IDisposable
713
{
814
private readonly InvocationMatcher invocationMatcher;
9-
private TaskCompletionSource<TResult> completionSource;
10-
private Timer? timeoutTimer;
11-
private JSRuntimeInvocation? currentInvocation;
15+
private readonly ConcurrentDictionary<long, PendingInvocation> pendingInvocations = new();
16+
private long nextInvocationId;
17+
private Task<TResult>? outcome;
1218
private bool disposed;
1319

1420
/// <summary>
@@ -34,48 +40,29 @@ public abstract class JSRuntimeInvocationHandlerBase<TResult> : IDisposable
3440
protected JSRuntimeInvocationHandlerBase(InvocationMatcher matcher, bool isCatchAllHandler)
3541
{
3642
invocationMatcher = matcher ?? throw new ArgumentNullException(nameof(matcher));
37-
completionSource = new TaskCompletionSource<TResult>(TaskCreationOptions.RunContinuationsAsynchronously);
3843
IsCatchAllHandler = isCatchAllHandler;
3944
}
4045

4146
/// <summary>
4247
/// Marks the <see cref="Task{TResult}"/> that invocations will receive as canceled.
4348
/// </summary>
4449
protected void SetCanceledBase()
45-
{
46-
ClearTimeoutTimer();
47-
if (completionSource.Task.IsCompleted)
48-
completionSource = new TaskCompletionSource<TResult>(TaskCreationOptions.RunContinuationsAsynchronously);
49-
50-
completionSource.SetCanceled();
51-
}
50+
=> CompleteAll(Task.FromCanceled<TResult>(new CancellationToken(canceled: true)));
5251

5352
/// <summary>
5453
/// Sets the <typeparamref name="TException"/> exception that invocations will receive.
5554
/// </summary>
5655
/// <param name="exception">The type of exception to pass to the callers.</param>
5756
protected void SetExceptionBase<TException>(TException exception)
5857
where TException : Exception
59-
{
60-
ClearTimeoutTimer();
61-
if (completionSource.Task.IsCompleted)
62-
completionSource = new TaskCompletionSource<TResult>(TaskCreationOptions.RunContinuationsAsynchronously);
63-
64-
completionSource.SetException(exception);
65-
}
58+
=> CompleteAll(Task.FromException<TResult>(exception));
6659

6760
/// <summary>
6861
/// Sets the <typeparamref name="TResult"/> result that invocations will receive.
6962
/// </summary>
7063
/// <param name="result">The type of result to pass to the callers.</param>
7164
protected void SetResultBase(TResult result)
72-
{
73-
ClearTimeoutTimer();
74-
if (completionSource.Task.IsCompleted)
75-
completionSource = new TaskCompletionSource<TResult>(TaskCreationOptions.RunContinuationsAsynchronously);
76-
77-
completionSource.SetResult(result);
78-
}
65+
=> CompleteAll(Task.FromResult(result));
7966

8067
/// <summary>
8168
/// Call this to have the this handler handle the <paramref name="invocation"/>.
@@ -89,18 +76,29 @@ protected internal virtual Task<TResult> HandleAsync(JSRuntimeInvocation invocat
8976
{
9077
Invocations.RegisterInvocation(invocation);
9178

92-
var task = completionSource.Task;
93-
if (task is { IsCanceled: false, IsFaulted: false, IsCompletedSuccessfully: false })
79+
if (Volatile.Read(ref outcome) is { } configured)
80+
return configured;
81+
82+
var timeout = BunitContext.DefaultWaitTimeout;
83+
if (timeout <= TimeSpan.Zero)
9484
{
95-
if (BunitContext.DefaultWaitTimeout <= TimeSpan.Zero)
96-
{
97-
throw new JSRuntimeInvocationNotSetException(invocation);
98-
}
85+
throw new JSRuntimeInvocationNotSetException(invocation);
86+
}
87+
88+
var id = Interlocked.Increment(ref nextInvocationId);
89+
var pending = new PendingInvocation(id, invocation);
90+
pendingInvocations[id] = pending;
9991

100-
StartTimeoutTimer(invocation);
92+
if (Volatile.Read(ref outcome) is { } raced && pendingInvocations.TryRemove(id, out _))
93+
{
94+
Transfer(raced, pending.CompletionSource);
95+
}
96+
else
97+
{
98+
pending.StartTimeout(OnTimeoutElapsed, timeout);
10199
}
102100

103-
return task;
101+
return pending.CompletionSource.Task;
104102
}
105103

106104
/// <summary>
@@ -122,34 +120,71 @@ protected virtual void Dispose(bool disposing)
122120
{
123121
if (!disposed && disposing)
124122
{
125-
ClearTimeoutTimer();
123+
foreach (var id in pendingInvocations.Keys)
124+
{
125+
if (pendingInvocations.TryRemove(id, out var pending))
126+
pending.Dispose();
127+
}
128+
126129
disposed = true;
127130
}
128131
}
129132

130-
private void StartTimeoutTimer(JSRuntimeInvocation invocation)
133+
private void CompleteAll(Task<TResult> next)
131134
{
132-
ClearTimeoutTimer();
135+
Volatile.Write(ref outcome, next);
133136

134-
currentInvocation = invocation;
135-
timeoutTimer = new Timer(OnTimeoutElapsed, null, BunitContext.DefaultWaitTimeout, Timeout.InfiniteTimeSpan);
137+
foreach (var id in pendingInvocations.Keys)
138+
{
139+
if (pendingInvocations.TryRemove(id, out var pending))
140+
{
141+
pending.Dispose();
142+
Transfer(next, pending.CompletionSource);
143+
}
144+
}
136145
}
137146

138-
private void ClearTimeoutTimer()
147+
private void OnTimeoutElapsed(object? state)
139148
{
140-
timeoutTimer?.Dispose();
141-
timeoutTimer = null;
142-
currentInvocation = null;
149+
if (state is not PendingInvocation pending || !pendingInvocations.TryRemove(pending.Id, out _))
150+
return;
151+
152+
pending.Dispose();
153+
pending.CompletionSource.TrySetException(new JSRuntimeInvocationNotSetException(pending.Invocation));
143154
}
144155

145-
private void OnTimeoutElapsed(object? state)
156+
private static void Transfer(Task<TResult> from, TaskCompletionSource<TResult> to)
146157
{
147-
if (!completionSource.Task.IsCompleted && currentInvocation.HasValue)
158+
if (from.IsCanceled)
159+
to.TrySetCanceled();
160+
else if (from.Exception is { } exception)
161+
to.TrySetException(exception.InnerExceptions);
162+
else
163+
to.TrySetResult(from.Result);
164+
}
165+
166+
private sealed class PendingInvocation : IDisposable
167+
{
168+
private Timer? timeoutTimer;
169+
170+
public long Id { get; }
171+
172+
public JSRuntimeInvocation Invocation { get; }
173+
174+
public TaskCompletionSource<TResult> CompletionSource { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
175+
176+
public PendingInvocation(long id, JSRuntimeInvocation invocation)
177+
{
178+
Id = id;
179+
Invocation = invocation;
180+
}
181+
182+
public void StartTimeout(TimerCallback callback, TimeSpan timeout)
148183
{
149-
var exception = new JSRuntimeInvocationNotSetException(currentInvocation.Value);
150-
completionSource.TrySetException(exception);
184+
timeoutTimer = new Timer(callback, this, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);
185+
timeoutTimer.Change(timeout, Timeout.InfiniteTimeSpan);
151186
}
152187

153-
ClearTimeoutTimer();
188+
public void Dispose() => timeoutTimer?.Dispose();
154189
}
155190
}

‎tests/bunit.tests/JSInterop/BunitJSInteropTimeoutTest.cs‎

Lines changed: 92 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
using System.Diagnostics;
2+
13
namespace Bunit.JSInterop;
24

35
[CollectionDefinition(nameof(DefaultWaitTimeoutTestGroup), DisableParallelization = true)]
@@ -12,19 +14,105 @@ public class BunitJSInteropTimeoutTest
1214
public async Task Test309()
1315
{
1416
const string identifier = "testFunction";
15-
var originalTimeout = BunitContext.DefaultWaitTimeout;
1617

17-
try
18+
await WithDefaultWaitTimeout(TimeSpan.FromMilliseconds(100), async () =>
1819
{
19-
BunitContext.DefaultWaitTimeout = TimeSpan.FromMilliseconds(100);
20-
2120
var sut = new BunitJSInterop { Mode = JSRuntimeMode.Strict };
2221
sut.Setup<int>(identifier);
2322

2423
var invocationTask = sut.JSRuntime.InvokeAsync<int>(identifier);
2524

2625
var exception = await Should.ThrowAsync<JSRuntimeInvocationNotSetException>(invocationTask.AsTask());
2726
exception.Invocation.Identifier.ShouldBe(identifier);
27+
});
28+
}
29+
30+
[Fact(DisplayName = "Each pending invocation times out with its own invocation")]
31+
public async Task Test310()
32+
{
33+
await WithDefaultWaitTimeout(TimeSpan.FromMilliseconds(100), async () =>
34+
{
35+
var sut = new BunitJSInterop { Mode = JSRuntimeMode.Strict };
36+
sut.Setup<int>(_ => true);
37+
38+
var first = sut.JSRuntime.InvokeAsync<int>("first").AsTask();
39+
var second = sut.JSRuntime.InvokeAsync<int>("second").AsTask();
40+
41+
(await Should.ThrowAsync<JSRuntimeInvocationNotSetException>(first))
42+
.Invocation.Identifier.ShouldBe("first");
43+
(await Should.ThrowAsync<JSRuntimeInvocationNotSetException>(second))
44+
.Invocation.Identifier.ShouldBe("second");
45+
});
46+
}
47+
48+
[Fact(DisplayName = "A timed out invocation does not affect later invocations")]
49+
public async Task Test311()
50+
{
51+
const string identifier = "testFunction";
52+
53+
await WithDefaultWaitTimeout(TimeSpan.FromMilliseconds(100), async () =>
54+
{
55+
var sut = new BunitJSInterop { Mode = JSRuntimeMode.Strict };
56+
var handler = sut.Setup<int>(identifier);
57+
58+
await Should.ThrowAsync<JSRuntimeInvocationNotSetException>(
59+
sut.JSRuntime.InvokeAsync<int>(identifier).AsTask());
60+
61+
handler.SetResult(42);
62+
63+
(await sut.JSRuntime.InvokeAsync<int>(identifier)).ShouldBe(42);
64+
});
65+
}
66+
67+
[Fact(DisplayName = "Setting a result while the timeout elapses does not crash the test host")]
68+
public async Task Test312()
69+
{
70+
const string identifier = "testFunction";
71+
var timeout = TimeSpan.FromMilliseconds(2);
72+
73+
await WithDefaultWaitTimeout(timeout, async () =>
74+
{
75+
var workers = Enumerable
76+
.Range(0, Math.Max(4, Environment.ProcessorCount))
77+
.Select(_ => Task.Run(() => RaceResultAgainstTimeout(identifier, timeout, iterations: 250)));
78+
79+
await Task.WhenAll(workers);
80+
});
81+
}
82+
83+
private static async Task RaceResultAgainstTimeout(string identifier, TimeSpan timeout, int iterations)
84+
{
85+
for (var i = 0; i < iterations; i++)
86+
{
87+
var sut = new BunitJSInterop { Mode = JSRuntimeMode.Strict };
88+
var handler = sut.Setup<int>(identifier);
89+
90+
var invocationTask = sut.JSRuntime.InvokeAsync<int>(identifier).AsTask();
91+
92+
// Spin until the timer is due so that setting the result races the elapsing timeout.
93+
var spin = Stopwatch.StartNew();
94+
while (spin.Elapsed < timeout)
95+
Thread.SpinWait(1);
96+
97+
handler.SetResult(i);
98+
99+
// Either the result or the timeout may win the race, but the invocation must
100+
// always complete and never surface anything but the timeout exception.
101+
var completed = await Task.WhenAny(invocationTask, Task.Delay(TimeSpan.FromSeconds(10)));
102+
completed.ShouldBe(invocationTask);
103+
104+
if (invocationTask.Exception is { } exception)
105+
exception.InnerException.ShouldBeOfType<JSRuntimeInvocationNotSetException>();
106+
}
107+
}
108+
109+
private static async Task WithDefaultWaitTimeout(TimeSpan timeout, Func<Task> test)
110+
{
111+
var originalTimeout = BunitContext.DefaultWaitTimeout;
112+
BunitContext.DefaultWaitTimeout = timeout;
113+
try
114+
{
115+
await test();
28116
}
29117
finally
30118
{

0 commit comments

Comments
 (0)