dotnet / dotnet/aspnetcore

TestHost: exception during ReturnResponseMessageAsync's feature-collection copy orphans the response TCS — client awaits forever, cancellation cannot break it

Open
#68,105 2 comments 0 reactions 0 assignees View on GitHub
area-networking
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 6h
Merged PRs (30d)
290

Description

### Is there an existing issue for this?

- [X] I have searched the existing issues

### Describe the bug

`HttpContextBuilder.ReturnResponseMessageAsync` copies the request's feature collection outside its `try/catch`:

```csharp
try
{
await _responseFeature.FireOnSendingHeadersAsync();
}
catch (Exception ex)
{
Abort(ex); // faults _responseTcs — but only for this one await
return;
}

var newFeatures = new FeatureCollection();
foreach (var pair in _httpContext.Features) // NOT protected
{
newFeatures[pair.Key] = pair.Value;
}
...
_responseTcs.TrySetResult(new DefaultHttpContext(newFeatures));
```

If that enumeration throws — in our case `InvalidOperationException: Collection was modified; enumeration operation may not execute`, because a concurrent party mutated `HttpContext.Features` while the response was being returned — then `Abort(ex)` never runs, so `_responseTcs` is neither completed nor faulted. The client's `SendAsync` awaits that TCS forever.

Crucially, **cancellation cannot recover this**: `HttpContextBuilder.SendAsync` registers `ClientInitiatedAbort` on the caller's token, which aborts the request/response streams but never faults `_responseTcs`. Once the copy has thrown, no `CancellationToken`, `HttpClient.Timeout`, or `WaitAsync`-visible mechanism inside TestHost ends the wait — the only exits are process death or the caller abandoning the task.

In a test runner this converts one transient exception into an infinite hang: NUnit's `AsyncToSyncAdapter` blocks on the test's task, `dotnet test --blame-hang` kills the host minutes later, and the resulting thread dump shows only waiters — the stranded continuation runs on no thread, so it is invisible to `clrstack -all` and only appears in `dumpasync`.

We hit this repeatedly in CI (~50% of affected runs over two days) via a mid-pipeline response body flush from an OAuth token endpoint handler (`ResponseBodyPipeWriter.FlushAsync → ReturnResponseMessageAsync`), with this stack:

```
System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
at Microsoft.AspNetCore.Http.Features.FeatureCollection.GetEnumerator()+MoveNext()
at Microsoft.AspNetCore.TestHost.HttpContextBuilder.ReturnResponseMessageAsync()
at Microsoft.AspNetCore.TestHost.ResponseBodyPipeWriter.FlushAsync(CancellationToken)
at AspNet.Security.OpenIdConnect.Server.OpenIdConnectServerHandler.SendPayloadAsync(...)
```

The concurrent mutation itself is arguably an application/middleware issue — but the framework response to it should be a faulted request, not an unkillable hang. Related: #54347 reports the same hang-on-exception outcome from a different throw site (logger scope during response completion), suggesting the gap is broader than this one method.

### Expected Behavior

Any exception thrown during response completion faults `_responseTcs` (e.g. widen the `try` to cover the feature copy and the rest of the method, calling `Abort(ex)`), so the awaiting `HttpClient.SendAsync` throws instead of hanging forever. Additionally/alternatively, `ClientInitiatedAbort` could fault `_responseTcs` so caller cancellation can always end the wait.

### Steps To Reproduce

The window is the first body flush: a middleware that mutates the feature collection concurrently with it reproduces the hang intermittently under load:

```csharp
[Fact]
public async Task Response_completion_exception_should_fault_not_hang()
{
using var host = await new HostBuilder()
.ConfigureWebHost(webBuilder => webBuilder
.UseTestServer()
.Configure(app => app.Run(async context =>
{
// Start returning the response mid-pipeline...
await context.Response.WriteAsync("partial");
await context.Response.Body.FlushAsync(); // triggers ReturnResponseMessageAsync

// ...while the feature collection is mutated concurrently.
// In real code this is a race; a parallel task that does
// context.Features.Set(...) during the flush
// reproduces it intermittently under load.
_ = Task.Run(() => context.Features.Set(new MyFeature()));
await Task.Delay(50);
})))
.StartAsync();

var client = host.GetTestServer().CreateClient();
using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5));

// Under the race: this await never completes, and the token does not break it.
var response = await client.GetAsync("/", cts.Token);
}
```

Because it is a race, a loop (or parallel requests) is needed to hit the window; our CI hits it on roughly half of full-suite runs. The structural claim does not depend on the repro rate: the copy is visibly outside the `try`, and `ClientInitiatedAbort` visibly does not fault the TCS.

### Exceptions (if any)

```
System.InvalidOperationException: Collection was modified; enumeration operation may not execute.
at Microsoft.AspNetCore.Http.Features.FeatureCollection.GetEnumerator()+MoveNext()
at Microsoft.AspNetCore.TestHost.HttpContextBuilder.ReturnResponseMessageAsync()
at Microsoft.AspNetCore.TestHost.ResponseBodyPipeWriter.FlushAsync(CancellationToken)
```

(then the awaiting test hangs with no further exception)

### .NET Version

10.0

### Anything else?

Microsoft.AspNetCore.TestHost 10.0.0, net10.0, Linux (observed in containerized CI) — code inspected at tag v10.0.0; the same shape is present on main.

Contributor guide

Open the contributing guide

Research direction

Start by locating HttpContextBuilder.ReturnResponseMessageAsync and inspect how its feature-collection copy, response TCS, and ClientInitiatedAbort interact. Use the supplied response-completion test scenario to reproduce the failure, then verify that an exception or caller cancellation completes the response task instead of leaving SendAsync waiting indefinitely.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
backend, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.