fix: OnTimeoutElapsed crashes / refactor to use upstream version (fix…

CHANGELOG.md

@@ -9,6 +9,7 @@ All notable changes to **bUnit** will be documented in this file. The project ad
 ### Fixed
 
 - `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.
+- 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).
 
 ## [2.10.3] - 2026-09-08
 

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

@@ -1,14 +1,20 @@
+using System.Collections.Concurrent;
+
 namespace Bunit;
 
+// Invocation tracking mirrors ASP.NET Core's JSRuntime: no per-invocation state lives in instance
+// fields. Each call gets its own TaskCompletionSource in a ConcurrentDictionary keyed by an
+// Interlocked id, and the timeout closes over that entry alone, so an elapsing timeout can never
+// race a concurrently set result. See https://github.com/dotnet/aspnetcore/blob/main/src/JSInterop/Microsoft.JSInterop/src/JSRuntime.cs
 /// <summary>
 /// Represents an invocation handler for <see cref="JSRuntimeInvocation"/> instances.
 /// </summary>
 public abstract class JSRuntimeInvocationHandlerBase<TResult> : IDisposable
 {
 	private readonly InvocationMatcher invocationMatcher;
-	private TaskCompletionSource<TResult> completionSource;
-	private Timer? timeoutTimer;
-	private JSRuntimeInvocation? currentInvocation;
+	private readonly ConcurrentDictionary<long, PendingInvocation> pendingInvocations = new();
+	private long nextInvocationId;
+	private Task<TResult>? outcome;
 	private bool disposed;
 
 	/// <summary>
@@ -34,48 +40,29 @@ public abstract class JSRuntimeInvocationHandlerBase<TResult> : IDisposable
 	protected JSRuntimeInvocationHandlerBase(InvocationMatcher matcher, bool isCatchAllHandler)
 	{
 		invocationMatcher = matcher ?? throw new ArgumentNullException(nameof(matcher));
-		completionSource = new TaskCompletionSource<TResult>(TaskCreationOptions.RunContinuationsAsynchronously);
 		IsCatchAllHandler = isCatchAllHandler;
 	}
 
 	/// <summary>
 	/// Marks the <see cref="Task{TResult}"/> that invocations will receive as canceled.
 	/// </summary>
 	protected void SetCanceledBase()
-	{
-		ClearTimeoutTimer();
-		if (completionSource.Task.IsCompleted)
-			completionSource = new TaskCompletionSource<TResult>(TaskCreationOptions.RunContinuationsAsynchronously);
-
-		completionSource.SetCanceled();
-	}
+		=> CompleteAll(Task.FromCanceled<TResult>(new CancellationToken(canceled: true)));
 
 	/// <summary>
 	/// Sets the <typeparamref name="TException"/> exception that invocations will receive.
 	/// </summary>
 	/// <param name="exception">The type of exception to pass to the callers.</param>
 	protected void SetExceptionBase<TException>(TException exception)
 		where TException : Exception
-	{
-		ClearTimeoutTimer();
-		if (completionSource.Task.IsCompleted)
-			completionSource = new TaskCompletionSource<TResult>(TaskCreationOptions.RunContinuationsAsynchronously);
-
-		completionSource.SetException(exception);
-	}
+		=> CompleteAll(Task.FromException<TResult>(exception));
 
 	/// <summary>
 	/// Sets the <typeparamref name="TResult"/> result that invocations will receive.
 	/// </summary>
 	/// <param name="result">The type of result to pass to the callers.</param>
 	protected void SetResultBase(TResult result)
-	{
-		ClearTimeoutTimer();
-		if (completionSource.Task.IsCompleted)
-			completionSource = new TaskCompletionSource<TResult>(TaskCreationOptions.RunContinuationsAsynchronously);
-
-		completionSource.SetResult(result);
-	}
+		=> CompleteAll(Task.FromResult(result));
 
 	/// <summary>
 	/// Call this to have the this handler handle the <paramref name="invocation"/>.
@@ -89,18 +76,29 @@ protected internal virtual Task<TResult> HandleAsync(JSRuntimeInvocation invocat
 	{
 		Invocations.RegisterInvocation(invocation);
 
-		var task = completionSource.Task;
-		if (task is { IsCanceled: false, IsFaulted: false, IsCompletedSuccessfully: false })
+		if (Volatile.Read(ref outcome) is { } configured)
+			return configured;
+
+		var timeout = BunitContext.DefaultWaitTimeout;
+		if (timeout <= TimeSpan.Zero)
 		{
-			if (BunitContext.DefaultWaitTimeout <= TimeSpan.Zero)
-			{
-				throw new JSRuntimeInvocationNotSetException(invocation);
-			}
+			throw new JSRuntimeInvocationNotSetException(invocation);
+		}
+
+		var id = Interlocked.Increment(ref nextInvocationId);
+		var pending = new PendingInvocation(id, invocation);
+		pendingInvocations[id] = pending;
 
-			StartTimeoutTimer(invocation);
+		if (Volatile.Read(ref outcome) is { } raced && pendingInvocations.TryRemove(id, out _))
+		{
+			Transfer(raced, pending.CompletionSource);
+		}
+		else
+		{
+			pending.StartTimeout(OnTimeoutElapsed, timeout);
 		}
 
-		return task;
+		return pending.CompletionSource.Task;
 	}
 
 	/// <summary>
@@ -122,34 +120,71 @@ protected virtual void Dispose(bool disposing)
 	{
 		if (!disposed && disposing)
 		{
-			ClearTimeoutTimer();
+			foreach (var id in pendingInvocations.Keys)
+			{
+				if (pendingInvocations.TryRemove(id, out var pending))
+					pending.Dispose();
+			}
+
 			disposed = true;
 		}
 	}
 
-	private void StartTimeoutTimer(JSRuntimeInvocation invocation)
+	private void CompleteAll(Task<TResult> next)
 	{
-		ClearTimeoutTimer();
+		Volatile.Write(ref outcome, next);
 
-		currentInvocation = invocation;
-		timeoutTimer = new Timer(OnTimeoutElapsed, null, BunitContext.DefaultWaitTimeout, Timeout.InfiniteTimeSpan);
+		foreach (var id in pendingInvocations.Keys)
+		{
+			if (pendingInvocations.TryRemove(id, out var pending))
+			{
+				pending.Dispose();
+				Transfer(next, pending.CompletionSource);
+			}
+		}
 	}
 
-	private void ClearTimeoutTimer()
+	private void OnTimeoutElapsed(object? state)
 	{
-		timeoutTimer?.Dispose();
-		timeoutTimer = null;
-		currentInvocation = null;
+		if (state is not PendingInvocation pending || !pendingInvocations.TryRemove(pending.Id, out _))
+			return;
+
+		pending.Dispose();
+		pending.CompletionSource.TrySetException(new JSRuntimeInvocationNotSetException(pending.Invocation));
 	}
 
-	private void OnTimeoutElapsed(object? state)
+	private static void Transfer(Task<TResult> from, TaskCompletionSource<TResult> to)
 	{
-		if (!completionSource.Task.IsCompleted && currentInvocation.HasValue)
+		if (from.IsCanceled)
+			to.TrySetCanceled();
+		else if (from.Exception is { } exception)
+			to.TrySetException(exception.InnerExceptions);
+		else
+			to.TrySetResult(from.Result);
+	}
+
+	private sealed class PendingInvocation : IDisposable
+	{
+		private Timer? timeoutTimer;
+
+		public long Id { get; }
+
+		public JSRuntimeInvocation Invocation { get; }
+
+		public TaskCompletionSource<TResult> CompletionSource { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
+
+		public PendingInvocation(long id, JSRuntimeInvocation invocation)
+		{
+			Id = id;
+			Invocation = invocation;
+		}
+
+		public void StartTimeout(TimerCallback callback, TimeSpan timeout)
 		{
-			var exception = new JSRuntimeInvocationNotSetException(currentInvocation.Value);
-			completionSource.TrySetException(exception);
+			timeoutTimer = new Timer(callback, this, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan);
+			timeoutTimer.Change(timeout, Timeout.InfiniteTimeSpan);
 		}
 
-		ClearTimeoutTimer();
+		public void Dispose() => timeoutTimer?.Dispose();
 	}
 }

tests/bunit.tests/JSInterop/BunitJSInteropTimeoutTest.cs

@@ -1,3 +1,5 @@
+using System.Diagnostics;
+
 namespace Bunit.JSInterop;
 
 [CollectionDefinition(nameof(DefaultWaitTimeoutTestGroup), DisableParallelization = true)]
@@ -12,19 +14,105 @@ public class BunitJSInteropTimeoutTest
 	public async Task Test309()
 	{
 		const string identifier = "testFunction";
-		var originalTimeout = BunitContext.DefaultWaitTimeout;
 
-		try
+		await WithDefaultWaitTimeout(TimeSpan.FromMilliseconds(100), async () =>
 		{
-			BunitContext.DefaultWaitTimeout = TimeSpan.FromMilliseconds(100);
-
 			var sut = new BunitJSInterop { Mode = JSRuntimeMode.Strict };
 			sut.Setup<int>(identifier);
 
 			var invocationTask = sut.JSRuntime.InvokeAsync<int>(identifier);
 
 			var exception = await Should.ThrowAsync<JSRuntimeInvocationNotSetException>(invocationTask.AsTask());
 			exception.Invocation.Identifier.ShouldBe(identifier);
+		});
+	}
+
+	[Fact(DisplayName = "Each pending invocation times out with its own invocation")]
+	public async Task Test310()
+	{
+		await WithDefaultWaitTimeout(TimeSpan.FromMilliseconds(100), async () =>
+		{
+			var sut = new BunitJSInterop { Mode = JSRuntimeMode.Strict };
+			sut.Setup<int>(_ => true);
+
+			var first = sut.JSRuntime.InvokeAsync<int>("first").AsTask();
+			var second = sut.JSRuntime.InvokeAsync<int>("second").AsTask();
+
+			(await Should.ThrowAsync<JSRuntimeInvocationNotSetException>(first))
+				.Invocation.Identifier.ShouldBe("first");
+			(await Should.ThrowAsync<JSRuntimeInvocationNotSetException>(second))
+				.Invocation.Identifier.ShouldBe("second");
+		});
+	}
+
+	[Fact(DisplayName = "A timed out invocation does not affect later invocations")]
+	public async Task Test311()
+	{
+		const string identifier = "testFunction";
+
+		await WithDefaultWaitTimeout(TimeSpan.FromMilliseconds(100), async () =>
+		{
+			var sut = new BunitJSInterop { Mode = JSRuntimeMode.Strict };
+			var handler = sut.Setup<int>(identifier);
+
+			await Should.ThrowAsync<JSRuntimeInvocationNotSetException>(
+				sut.JSRuntime.InvokeAsync<int>(identifier).AsTask());
+
+			handler.SetResult(42);
+
+			(await sut.JSRuntime.InvokeAsync<int>(identifier)).ShouldBe(42);
+		});
+	}
+
+	[Fact(DisplayName = "Setting a result while the timeout elapses does not crash the test host")]
+	public async Task Test312()
+	{
+		const string identifier = "testFunction";
+		var timeout = TimeSpan.FromMilliseconds(2);
+
+		await WithDefaultWaitTimeout(timeout, async () =>
+		{
+			var workers = Enumerable
+				.Range(0, Math.Max(4, Environment.ProcessorCount))
+				.Select(_ => Task.Run(() => RaceResultAgainstTimeout(identifier, timeout, iterations: 250)));
+
+			await Task.WhenAll(workers);
+		});
+	}
+
+	private static async Task RaceResultAgainstTimeout(string identifier, TimeSpan timeout, int iterations)
+	{
+		for (var i = 0; i < iterations; i++)
+		{
+			var sut = new BunitJSInterop { Mode = JSRuntimeMode.Strict };
+			var handler = sut.Setup<int>(identifier);
+
+			var invocationTask = sut.JSRuntime.InvokeAsync<int>(identifier).AsTask();
+
+			// Spin until the timer is due so that setting the result races the elapsing timeout.
+			var spin = Stopwatch.StartNew();
+			while (spin.Elapsed < timeout)
+				Thread.SpinWait(1);
+
+			handler.SetResult(i);
+
+			// Either the result or the timeout may win the race, but the invocation must
+			// always complete and never surface anything but the timeout exception.
+			var completed = await Task.WhenAny(invocationTask, Task.Delay(TimeSpan.FromSeconds(10)));
+			completed.ShouldBe(invocationTask);
+
+			if (invocationTask.Exception is { } exception)
+				exception.InnerException.ShouldBeOfType<JSRuntimeInvocationNotSetException>();
+		}
+	}
+
+	private static async Task WithDefaultWaitTimeout(TimeSpan timeout, Func<Task> test)
+	{
+		var originalTimeout = BunitContext.DefaultWaitTimeout;
+		BunitContext.DefaultWaitTimeout = timeout;
+		try
+		{
+			await test();
 		}
 		finally
 		{