Skip to content

Commit 4539b0d

Browse files
halter73halter73
authored
Increase shutdown timeout in Kestrel TestServer (#7082)
* Never use a null abort reason
1 parent c06f896 commit 4539b0d

File tree

9 files changed

+187
-9
lines changed

9 files changed

+187
-9
lines changed

‎src/Servers/Connections.Abstractions/src/ConnectionContext.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System.Collections.Generic;
@@ -26,6 +26,6 @@ public virtual void Abort(ConnectionAbortedException abortReason)
2626
Features.Get<IConnectionLifetimeFeature>()?.Abort();
2727
}
2828

29-
public virtual void Abort() => Abort(new ConnectionAbortedException("The connection was aborted by the application."));
29+
public virtual void Abort() => Abort(new ConnectionAbortedException("The connection was aborted by the application via ConnectionContext.Abort()."));
3030
}
3131
}

‎src/Servers/Kestrel/Core/src/CoreStrings.resx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,4 +590,10 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
590590
<data name="Http2ErrorStreamAborted" xml:space="preserve">
591591
<value>A frame of type {frameType} was received after stream {streamId} was reset or aborted.</value>
592592
</data>
593-
</root>
593+
<data name="ProtocolSelectionFailed" xml:space="preserve">
594+
<value>HTTP protocol selection failed.</value>
595+
</data>
596+
<data name="ServerShutdownDuringConnectionInitialization" xml:space="preserve">
597+
<value>Server shutdown started during connection initialization.</value>
598+
</data>
599+
</root>

‎src/Servers/Kestrel/Core/src/Internal/HttpConnection.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ public async Task ProcessRequestsAsync<TContext>(IHttpApplication<TContext> http
149149
break;
150150
case HttpProtocols.None:
151151
// An error was already logged in SelectProtocol(), but we should close the connection.
152-
Abort(ex: null);
152+
Abort(new ConnectionAbortedException(CoreStrings.ProtocolSelectionFailed));
153153
break;
154154
default:
155155
// SelectProtocol() only returns Http1, Http2 or None.
@@ -222,7 +222,7 @@ private void StopProcessingNextRequest()
222222
switch (_protocolSelectionState)
223223
{
224224
case ProtocolSelectionState.Initializing:
225-
CloseUninitializedConnection(abortReason: null);
225+
CloseUninitializedConnection(new ConnectionAbortedException(CoreStrings.ServerShutdownDuringConnectionInitialization));
226226
_protocolSelectionState = ProtocolSelectionState.Aborted;
227227
break;
228228
case ProtocolSelectionState.Selected:
@@ -241,7 +241,10 @@ private void OnInputOrOutputCompleted()
241241
switch (_protocolSelectionState)
242242
{
243243
case ProtocolSelectionState.Initializing:
244-
CloseUninitializedConnection(abortReason: null);
244+
// OnReader/WriterCompleted callbacks are not wired until after leaving the Initializing state.
245+
Debug.Assert(false);
246+
247+
CloseUninitializedConnection(new ConnectionAbortedException("HttpConnection.OnInputOrOutputCompleted() called while in the ProtocolSelectionState.Initializing state!?"));
245248
_protocolSelectionState = ProtocolSelectionState.Aborted;
246249
break;
247250
case ProtocolSelectionState.Selected:

‎src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs

Lines changed: 28 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Servers/Kestrel/Core/test/ConnectionContextTests.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using Microsoft.AspNetCore.Connections;
5+
using Moq;
6+
using Xunit;
7+
8+
namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
9+
{
10+
public class ConnectionContextTests
11+
{
12+
[Fact]
13+
public void ParameterlessAbortCreateConnectionAbortedException()
14+
{
15+
var mockConnectionContext = new Mock<ConnectionContext> { CallBase = true };
16+
ConnectionAbortedException ex = null;
17+
18+
mockConnectionContext.Setup(c => c.Abort(It.IsAny<ConnectionAbortedException>()))
19+
.Callback<ConnectionAbortedException>(abortReason => ex = abortReason);
20+
21+
mockConnectionContext.Object.Abort();
22+
23+
Assert.NotNull(ex);
24+
Assert.Equal("The connection was aborted by the application via ConnectionContext.Abort().", ex.Message);
25+
}
26+
}
27+
}

‎src/Servers/Kestrel/Transport.Abstractions/src/Internal/TransportConnection.FeatureCollection.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.IO.Pipelines;
77
using System.Net;
88
using System.Threading;
9+
using Microsoft.AspNetCore.Connections;
910
using Microsoft.AspNetCore.Connections.Features;
1011
using Microsoft.AspNetCore.Http.Features;
1112

@@ -91,7 +92,7 @@ CancellationToken IConnectionLifetimeNotificationFeature.ConnectionClosedRequest
9192
set => ConnectionClosedRequested = value;
9293
}
9394

94-
void IConnectionLifetimeFeature.Abort() => Abort(abortReason: null);
95+
void IConnectionLifetimeFeature.Abort() => Abort(new ConnectionAbortedException("The connection was aborted by the application via IConnectionLifetimeFeature.Abort()."));
9596

9697
void IConnectionLifetimeNotificationFeature.RequestClose() => RequestClose();
9798

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionAdapterTests.cs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal;
1212
using Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTransport;
1313
using Microsoft.AspNetCore.Testing;
14+
using Microsoft.Extensions.Logging.Testing;
1415
using Xunit;
1516

1617
namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests
@@ -124,6 +125,7 @@ public async Task ImmediateFinAfterThrowingClosesGracefully()
124125
}
125126

126127
[Fact]
128+
[CollectDump]
127129
public async Task ImmediateShutdownAfterOnConnectionAsyncDoesNotCrash()
128130
{
129131
var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0))
@@ -135,10 +137,43 @@ public async Task ImmediateShutdownAfterOnConnectionAsyncDoesNotCrash()
135137

136138
var stopTask = Task.CompletedTask;
137139
using (var server = new TestServer(TestApp.EchoApp, serviceContext, listenOptions))
140+
using (var shutdownCts = new CancellationTokenSource(TimeSpan.FromSeconds(5)))
138141
{
139142
using (var connection = server.CreateConnection())
140143
{
144+
// Use the default 5 second shutdown timeout. If it hangs that long, we'll look
145+
// at the collected memory dump.
146+
stopTask = server.StopAsync(shutdownCts.Token);
147+
}
148+
149+
await stopTask;
150+
}
151+
}
152+
153+
[Fact]
154+
public async Task ImmediateShutdownDuringOnConnectionAsyncDoesNotCrash()
155+
{
156+
var waitingConnectionAdapter = new WaitingConnectionAdapter();
157+
var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0))
158+
{
159+
ConnectionAdapters = { waitingConnectionAdapter }
160+
};
161+
162+
var serviceContext = new TestServiceContext(LoggerFactory);
163+
164+
using (var server = new TestServer(TestApp.EchoApp, serviceContext, listenOptions))
165+
{
166+
Task stopTask;
167+
168+
using (var connection = server.CreateConnection())
169+
{
170+
var closingMessageTask = TestApplicationErrorLogger.WaitForMessage(m => m.Message.Contains(CoreStrings.ServerShutdownDuringConnectionInitialization));
171+
141172
stopTask = server.StopAsync();
173+
174+
await closingMessageTask.DefaultTimeout();
175+
176+
waitingConnectionAdapter.Complete();
142177
}
143178

144179
await stopTask;
@@ -232,6 +267,24 @@ public async Task<IAdaptedConnection> OnConnectionAsync(ConnectionAdapterContext
232267
}
233268
}
234269

270+
private class WaitingConnectionAdapter : IConnectionAdapter
271+
{
272+
private TaskCompletionSource<object> _waitingTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously);
273+
274+
public bool IsHttps => false;
275+
276+
public async Task<IAdaptedConnection> OnConnectionAsync(ConnectionAdapterContext context)
277+
{
278+
await _waitingTcs.Task;
279+
return new AdaptedConnection(context.ConnectionStream);
280+
}
281+
282+
public void Complete()
283+
{
284+
_waitingTcs.TrySetResult(null);
285+
}
286+
}
287+
235288
private class ThrowingConnectionAdapter : IConnectionAdapter
236289
{
237290
public bool IsHttps => false;

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,14 @@
44
using System;
55
using System.Collections.Generic;
66
using System.IO;
7+
using System.IO.Pipelines;
78
using System.Linq;
89
using System.Net;
910
using System.Text;
1011
using System.Threading;
1112
using System.Threading.Tasks;
1213
using Microsoft.AspNetCore.Connections;
14+
using Microsoft.AspNetCore.Connections.Features;
1315
using Microsoft.AspNetCore.Http;
1416
using Microsoft.AspNetCore.Http.Features;
1517
using Microsoft.AspNetCore.Server.Kestrel.Core;
@@ -2319,6 +2321,62 @@ await connection.Send(
23192321
}
23202322
}
23212323

2324+
[Fact]
2325+
public async Task AppAbortIsLogged()
2326+
{
2327+
var testContext = new TestServiceContext(LoggerFactory);
2328+
2329+
using (var server = new TestServer(httpContext =>
2330+
{
2331+
httpContext.Abort();
2332+
return Task.CompletedTask;
2333+
}, testContext))
2334+
{
2335+
using (var connection = server.CreateConnection())
2336+
{
2337+
await connection.Send(
2338+
"GET / HTTP/1.1",
2339+
"Host:",
2340+
"",
2341+
"");
2342+
await connection.ReceiveEnd();
2343+
}
2344+
await server.StopAsync();
2345+
}
2346+
2347+
Assert.Single(TestApplicationErrorLogger.Messages.Where(m => m.Message.Contains(CoreStrings.ConnectionAbortedByApplication)));
2348+
}
2349+
2350+
[Fact]
2351+
public async Task AppAbortViaIConnectionLifetimeFeatureIsLogged()
2352+
{
2353+
// Ensure the response doesn't get flush before the abort is observed by scheduling inline.
2354+
var testContext = new TestServiceContext(LoggerFactory)
2355+
{
2356+
Scheduler = PipeScheduler.Inline
2357+
};
2358+
2359+
using (var server = new TestServer(httpContext =>
2360+
{
2361+
httpContext.Features.Get<IConnectionLifetimeFeature>().Abort();
2362+
return Task.CompletedTask;
2363+
}, testContext))
2364+
{
2365+
using (var connection = server.CreateConnection())
2366+
{
2367+
await connection.Send(
2368+
"GET / HTTP/1.1",
2369+
"Host:",
2370+
"",
2371+
"");
2372+
await connection.ReceiveEnd();
2373+
}
2374+
await server.StopAsync();
2375+
}
2376+
2377+
Assert.Single(TestApplicationErrorLogger.Messages.Where(m => m.Message.Contains("The connection was aborted by the application via IConnectionLifetimeFeature.Abort().")));
2378+
}
2379+
23222380
[Fact]
23232381
public async Task ResponseHeadersAreResetOnEachRequest()
23242382
{

‎src/Servers/Kestrel/test/InMemory.FunctionalTests/TestTransport/TestServer.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using Microsoft.AspNetCore.Http;
1414
using Microsoft.AspNetCore.Server.Kestrel.Core;
1515
using Microsoft.AspNetCore.Testing;
16+
using Microsoft.Extensions.Configuration;
1617
using Microsoft.Extensions.DependencyInjection;
1718
using Xunit;
1819

@@ -68,6 +69,7 @@ public TestServer(RequestDelegate app, TestServiceContext context, Action<Kestre
6869
HttpClientSlim = new InMemoryHttpClientSlim(this);
6970

7071
var hostBuilder = new WebHostBuilder()
72+
.UseSetting(WebHostDefaults.ShutdownTimeoutKey, TestConstants.DefaultTimeout.TotalSeconds.ToString())
7173
.ConfigureServices(services =>
7274
{
7375
configureServices(services);
@@ -106,9 +108,9 @@ public InMemoryConnection CreateConnection()
106108
return new InMemoryConnection(transportConnection);
107109
}
108110

109-
public Task StopAsync()
111+
public Task StopAsync(CancellationToken cancellationToken = default)
110112
{
111-
return _host.StopAsync();
113+
return _host.StopAsync(cancellationToken);
112114
}
113115

114116
public void Dispose()

0 commit comments

Comments
 (0)