diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b047d140..525af7b12 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/bunit/JSInterop/InvocationHandlers/JSRuntimeInvocationHandlerBase{TResult}.cs b/src/bunit/JSInterop/InvocationHandlers/JSRuntimeInvocationHandlerBase{TResult}.cs index 5aa093eb2..53026ff4d 100644 --- a/src/bunit/JSInterop/InvocationHandlers/JSRuntimeInvocationHandlerBase{TResult}.cs +++ b/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 /// /// Represents an invocation handler for instances. /// public abstract class JSRuntimeInvocationHandlerBase : IDisposable { private readonly InvocationMatcher invocationMatcher; - private TaskCompletionSource completionSource; - private Timer? timeoutTimer; - private JSRuntimeInvocation? currentInvocation; + private readonly ConcurrentDictionary pendingInvocations = new(); + private long nextInvocationId; + private Task? outcome; private bool disposed; /// @@ -34,7 +40,6 @@ public abstract class JSRuntimeInvocationHandlerBase : IDisposable protected JSRuntimeInvocationHandlerBase(InvocationMatcher matcher, bool isCatchAllHandler) { invocationMatcher = matcher ?? throw new ArgumentNullException(nameof(matcher)); - completionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); IsCatchAllHandler = isCatchAllHandler; } @@ -42,13 +47,7 @@ protected JSRuntimeInvocationHandlerBase(InvocationMatcher matcher, bool isCatch /// Marks the that invocations will receive as canceled. /// protected void SetCanceledBase() - { - ClearTimeoutTimer(); - if (completionSource.Task.IsCompleted) - completionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - - completionSource.SetCanceled(); - } + => CompleteAll(Task.FromCanceled(new CancellationToken(canceled: true))); /// /// Sets the exception that invocations will receive. @@ -56,26 +55,14 @@ protected void SetCanceledBase() /// The type of exception to pass to the callers. protected void SetExceptionBase(TException exception) where TException : Exception - { - ClearTimeoutTimer(); - if (completionSource.Task.IsCompleted) - completionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - - completionSource.SetException(exception); - } + => CompleteAll(Task.FromException(exception)); /// /// Sets the result that invocations will receive. /// /// The type of result to pass to the callers. protected void SetResultBase(TResult result) - { - ClearTimeoutTimer(); - if (completionSource.Task.IsCompleted) - completionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - - completionSource.SetResult(result); - } + => CompleteAll(Task.FromResult(result)); /// /// Call this to have the this handler handle the . @@ -89,18 +76,29 @@ protected internal virtual Task 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; } /// @@ -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 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 from, TaskCompletionSource 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 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(); } } diff --git a/tests/bunit.tests/JSInterop/BunitJSInteropTimeoutTest.cs b/tests/bunit.tests/JSInterop/BunitJSInteropTimeoutTest.cs index 811bd351c..eb5cc4a28 100644 --- a/tests/bunit.tests/JSInterop/BunitJSInteropTimeoutTest.cs +++ b/tests/bunit.tests/JSInterop/BunitJSInteropTimeoutTest.cs @@ -1,3 +1,5 @@ +using System.Diagnostics; + namespace Bunit.JSInterop; [CollectionDefinition(nameof(DefaultWaitTimeoutTestGroup), DisableParallelization = true)] @@ -12,12 +14,9 @@ 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(identifier); @@ -25,6 +24,95 @@ public async Task Test309() var exception = await Should.ThrowAsync(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(_ => true); + + var first = sut.JSRuntime.InvokeAsync("first").AsTask(); + var second = sut.JSRuntime.InvokeAsync("second").AsTask(); + + (await Should.ThrowAsync(first)) + .Invocation.Identifier.ShouldBe("first"); + (await Should.ThrowAsync(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(identifier); + + await Should.ThrowAsync( + sut.JSRuntime.InvokeAsync(identifier).AsTask()); + + handler.SetResult(42); + + (await sut.JSRuntime.InvokeAsync(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(identifier); + + var invocationTask = sut.JSRuntime.InvokeAsync(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(); + } + } + + private static async Task WithDefaultWaitTimeout(TimeSpan timeout, Func test) + { + var originalTimeout = BunitContext.DefaultWaitTimeout; + BunitContext.DefaultWaitTimeout = timeout; + try + { + await test(); } finally {