Skip to content

Commit cec88a3

Browse files
amcaseyadityamandaleeka
authored andcommitted
Merged PR 32094: [6.0] Forcibly close socket on abort
Forcibly close socket when connection is aborted.
1 parent a62c85f commit cec88a3

24 files changed

+811
-91
lines changed

src/Servers/IIS/IIS/test/Common.FunctionalTests/ShutdownTests.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,8 +486,15 @@ public async Task ClosesConnectionOnServerAbortOutOfProcess()
486486
var response = await deploymentResult.HttpClient.GetAsync("/Abort").TimeoutAfter(TimeoutExtensions.DefaultTimeoutValue);
487487

488488
Assert.Equal(HttpStatusCode.BadGateway, response.StatusCode);
489+
490+
#if NEWSHIM_FUNCTIONALS
491+
// In-proc SocketConnection isn't used and there's no abort
489492
// 0x80072f78 ERROR_HTTP_INVALID_SERVER_RESPONSE The server returned an invalid or unrecognized response
490493
Assert.Contains("0x80072f78", await response.Content.ReadAsStringAsync());
494+
#else
495+
// 0x80072efe ERROR_INTERNET_CONNECTION_ABORTED The connection with the server was terminated abnormally
496+
Assert.Contains("0x80072efe", await response.Content.ReadAsStringAsync());
497+
#endif
491498
}
492499
catch (HttpRequestException)
493500
{

src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,14 @@ protected override void OnRequestProcessingEnded()
9393
_http1Output.Dispose();
9494
}
9595

96-
public void OnInputOrOutputCompleted()
96+
void IRequestProcessor.OnInputOrOutputCompleted()
97+
{
98+
// Closed gracefully.
99+
_http1Output.Abort(ServerOptions.FinOnError ? new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient) : null!);
100+
CancelRequestAbortedToken();
101+
}
102+
103+
void IHttpOutputAborter.OnInputOrOutputCompleted()
97104
{
98105
_http1Output.Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient));
99106
CancelRequestAbortedToken();

src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ protected void ThrowUnexpectedEndOfRequestContent()
207207
// so we call OnInputOrOutputCompleted() now to prevent a race in our tests where a 400
208208
// response is written after observing the unexpected end of request content instead of just
209209
// closing the connection without a response as expected.
210-
_context.OnInputOrOutputCompleted();
210+
((IHttpOutputAborter)_context).OnInputOrOutputCompleted();
211211

212212
KestrelBadHttpRequestException.Throw(RequestRejectionReason.UnexpectedEndOfRequestContent);
213213
}

src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,8 @@ public Http2Connection(HttpConnectionContext context)
150150
public void OnInputOrOutputCompleted()
151151
{
152152
TryClose();
153-
_frameWriter.Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient));
153+
var useException = _context.ServiceContext.ServerOptions.FinOnError || _clientActiveStreamCount != 0;
154+
_frameWriter.Abort(useException ? new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient) : null!);
154155
}
155156

156157
public void Abort(ConnectionAbortedException ex)

src/Servers/Kestrel/Core/src/KestrelServerOptions.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
2929
/// </summary>
3030
public class KestrelServerOptions
3131
{
32+
private const string FinOnErrorSwitch = "Microsoft.AspNetCore.Server.Kestrel.FinOnError";
33+
private static readonly bool _finOnError;
34+
35+
static KestrelServerOptions()
36+
{
37+
AppContext.TryGetSwitch(FinOnErrorSwitch, out _finOnError);
38+
}
39+
3240
// internal to fast-path header decoding when RequestHeaderEncodingSelector is unchanged.
3341
internal static readonly Func<string, Encoding?> DefaultHeaderEncodingSelector = _ => null;
3442

43+
// Opt-out flag for back compat. Remove in 9.0 (or make public).
44+
internal bool FinOnError { get; set; } = _finOnError;
45+
3546
private Func<string, Encoding?> _requestHeaderEncodingSelector = DefaultHeaderEncodingSelector;
3647

3748
private Func<string, Encoding?> _responseHeaderEncodingSelector = DefaultHeaderEncodingSelector;

src/Servers/Kestrel/Transport.Libuv/src/Internal/ILibuvTrace.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ internal interface ILibuvTrace : ILogger
1414

1515
void ConnectionWriteFin(string connectionId, string reason);
1616

17+
void ConnectionWriteRst(string connectionId, string reason);
18+
1719
void ConnectionWrite(string connectionId, int count);
1820

1921
void ConnectionWriteCallback(string connectionId, int status);

src/Servers/Kestrel/Transport.Libuv/src/Internal/LibuvConnection.cs

Lines changed: 55 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
using System.IO;
77
using System.IO.Pipelines;
88
using System.Net;
9+
using System.Net.Sockets;
10+
using System.Reflection.Metadata;
911
using System.Threading;
1012
using System.Threading.Tasks;
1113
using Microsoft.AspNetCore.Connections;
@@ -28,6 +30,8 @@ internal partial class LibuvConnection : TransportConnection
2830
private readonly IDuplexPipe _originalTransport;
2931
private readonly CancellationTokenSource _connectionClosedTokenSource = new CancellationTokenSource();
3032

33+
private readonly bool _finOnError;
34+
3135
private volatile ConnectionAbortedException _abortReason;
3236

3337
private MemoryHandle _bufferHandle;
@@ -43,9 +47,11 @@ public LibuvConnection(UvStreamHandle socket,
4347
PipeOptions inputOptions = null,
4448
PipeOptions outputOptions = null,
4549
long? maxReadBufferSize = null,
46-
long? maxWriteBufferSize = null)
50+
long? maxWriteBufferSize = null,
51+
bool finOnError = false)
4752
{
4853
_socket = socket;
54+
_finOnError = finOnError;
4955

5056
LocalEndPoint = localEndPoint;
5157
RemoteEndPoint = remoteEndPoint;
@@ -124,6 +130,13 @@ private async Task StartCore()
124130
{
125131
inputError ??= _abortReason ?? new ConnectionAbortedException("The libuv transport's send loop completed gracefully.");
126132

133+
if (!_finOnError && _abortReason is not null)
134+
{
135+
// When shutdown isn't clean (note that we're using _abortReason, rather than inputError, to exclude that case),
136+
// we set the DontLinger socket option to cause libuv to send a RST and release any buffered response data.
137+
SetDontLingerOption(_socket);
138+
}
139+
127140
// Now, complete the input so that no more reads can happen
128141
Input.Complete(inputError);
129142
Output.Complete(outputError);
@@ -132,8 +145,16 @@ private async Task StartCore()
132145
// on the stream handle
133146
Input.CancelPendingFlush();
134147

135-
// Send a FIN
136-
Log.ConnectionWriteFin(ConnectionId, inputError.Message);
148+
if (!_finOnError && _abortReason is not null)
149+
{
150+
// Send a RST
151+
Log.ConnectionWriteRst(ConnectionId, inputError.Message);
152+
}
153+
else
154+
{
155+
// Send a FIN
156+
Log.ConnectionWriteFin(ConnectionId, inputError.Message);
157+
}
137158

138159
// We're done with the socket now
139160
_socket.Dispose();
@@ -150,15 +171,44 @@ private async Task StartCore()
150171
}
151172
}
152173

174+
/// <remarks>
175+
/// This should be called on <see cref="_socket"/> before it is disposed.
176+
/// Both <see cref="Abort"/> and <see cref="StartCore"/> call dispose but, rather than predict
177+
/// which will do so first (which varies), we make this method idempotent and call it in both.
178+
/// </remarks>
179+
private static void SetDontLingerOption(UvStreamHandle socket)
180+
{
181+
if (!socket.IsClosed && !socket.IsInvalid)
182+
{
183+
var libuv = socket.Libuv;
184+
var pSocket = IntPtr.Zero;
185+
libuv.uv_fileno(socket, ref pSocket);
186+
187+
// libuv doesn't expose setsockopt, so we take advantage of the fact that
188+
// Socket already has a PAL
189+
using var managedHandle = new SafeSocketHandle(pSocket, ownsHandle: false);
190+
using var managedSocket = new Socket(managedHandle);
191+
managedSocket.LingerState = new LingerOption(enable: true, seconds: 0);
192+
}
193+
}
194+
153195
public override void Abort(ConnectionAbortedException abortReason)
154196
{
155197
_abortReason = abortReason;
156198

157199
// Cancel WriteOutputAsync loop after setting _abortReason.
158200
Output.CancelPendingRead();
159201

160-
// This cancels any pending I/O.
161-
Thread.Post(s => s.Dispose(), _socket);
202+
Thread.Post(static (self) =>
203+
{
204+
if (!self._finOnError)
205+
{
206+
SetDontLingerOption(self._socket);
207+
}
208+
209+
// This cancels any pending I/O.
210+
self._socket.Dispose();
211+
}, this);
162212
}
163213

164214
public override async ValueTask DisposeAsync()

src/Servers/Kestrel/Transport.Libuv/src/Internal/LibuvConnectionListener.cs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics;
67
using System.Linq;
78
using System.Net;
89
using System.Threading;
@@ -134,9 +135,24 @@ internal async Task BindAsync()
134135
{
135136
// TODO: Move thread management to LibuvTransportFactory
136137
// TODO: Split endpoint management from thread management
138+
139+
// When `FinOnError` is false (the default), we need to be able to forcibly abort connections.
140+
// On Windows, libuv 1.10.0 will call `shutdown`, preventing forcible abort, on any socket
141+
// not flagged as `UV_HANDLE_SHARED_TCP_SOCKET`. The only way we've found to cause socket
142+
// to be flagged as `UV_HANDLE_SHARED_TCP_SOCKET` is to share it across a named pipe (which
143+
// must, itself, be flagged `ipc`), which naturally happens when a `ListenerPrimary` dispatches
144+
// a connection to a `ListenerSecondary`. Therefore, in scenarios where this is required, we
145+
// tell the `ListenerPrimary` to dispatch *all* connections to secondary and create an
146+
// additional `ListenerSecondary` to replace the lost capacity.
147+
var dispatchAllToSecondary = Libuv.IsWindows && !TransportContext.Options.FinOnError;
148+
137149
#pragma warning disable CS0618
138-
for (var index = 0; index < TransportOptions.ThreadCount; index++)
150+
var threadCount = dispatchAllToSecondary
151+
? TransportOptions.ThreadCount + 1
152+
: TransportOptions.ThreadCount;
139153
#pragma warning restore CS0618
154+
155+
for (var index = 0; index < threadCount; index++)
140156
{
141157
Threads.Add(new LibuvThread(Libuv, TransportContext));
142158
}
@@ -148,10 +164,10 @@ internal async Task BindAsync()
148164

149165
try
150166
{
151-
#pragma warning disable CS0618
152-
if (TransportOptions.ThreadCount == 1)
153-
#pragma warning restore CS0618
167+
if (threadCount == 1)
154168
{
169+
Debug.Assert(!dispatchAllToSecondary, "Should have taken the primary/secondary code path");
170+
155171
var listener = new Listener(TransportContext);
156172
_listeners.Add(listener);
157173
await listener.StartAsync(EndPoint, Threads[0]).ConfigureAwait(false);
@@ -162,7 +178,7 @@ internal async Task BindAsync()
162178
var pipeName = (Libuv.IsWindows ? @"\\.\pipe\kestrel_" : "/tmp/kestrel_") + Guid.NewGuid().ToString("n");
163179
var pipeMessage = Guid.NewGuid().ToByteArray();
164180

165-
var listenerPrimary = new ListenerPrimary(TransportContext);
181+
var listenerPrimary = new ListenerPrimary(TransportContext, dispatchAllToSecondary);
166182
_listeners.Add(listenerPrimary);
167183
await listenerPrimary.StartAsync(pipeName, pipeMessage, EndPoint, Threads[0]).ConfigureAwait(false);
168184
EndPoint = listenerPrimary.EndPoint;

src/Servers/Kestrel/Transport.Libuv/src/Internal/LibuvTrace.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ public void ConnectionRead(string connectionId, int count)
2727
[LoggerMessage(7, LogLevel.Debug, @"Connection id ""{ConnectionId}"" sending FIN because: ""{Reason}""", EventName = nameof(ConnectionWriteFin))]
2828
public partial void ConnectionWriteFin(string connectionId, string reason);
2929

30+
[LoggerMessage(8, LogLevel.Debug, @"Connection id ""{ConnectionId}"" sending RST because: ""{Reason}""", EventName = nameof(ConnectionWriteRst))]
31+
public partial void ConnectionWriteRst(string connectionId, string reason);
32+
3033
public void ConnectionWrite(string connectionId, int count)
3134
{
3235
// Don't log for now since this could be *too* verbose.

src/Servers/Kestrel/Transport.Libuv/src/Internal/ListenerContext.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ protected internal void HandleConnection(UvStreamHandle socket)
110110

111111
var options = TransportContext.Options;
112112
#pragma warning disable CS0618
113-
var connection = new LibuvConnection(socket, TransportContext.Log, Thread, remoteEndPoint, localEndPoint, InputOptions, OutputOptions, options.MaxReadBufferSize, options.MaxWriteBufferSize);
113+
var connection = new LibuvConnection(socket, TransportContext.Log, Thread, remoteEndPoint, localEndPoint, InputOptions, OutputOptions, options.MaxReadBufferSize, options.MaxWriteBufferSize, options.FinOnError);
114114
#pragma warning restore CS0618
115115
connection.Start();
116116

src/Servers/Kestrel/Transport.Libuv/src/Internal/ListenerPrimary.cs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Diagnostics;
67
using System.IO;
78
using System.Net;
89
using System.Runtime.InteropServices;
@@ -22,6 +23,10 @@ internal class ListenerPrimary : Listener
2223
private readonly List<UvPipeHandle> _dispatchPipes = new List<UvPipeHandle>();
2324
// The list of pipes we've created but may not be part of _dispatchPipes
2425
private readonly List<UvPipeHandle> _createdPipes = new List<UvPipeHandle>();
26+
27+
// If true, dispatch all connections to _dispatchPipes - don't process any in the primary
28+
private readonly bool _dispatchAll;
29+
2530
private int _dispatchIndex;
2631
private string _pipeName;
2732
private byte[] _pipeMessage;
@@ -32,8 +37,9 @@ internal class ListenerPrimary : Listener
3237
// but it has no other functional significance
3338
private readonly ArraySegment<ArraySegment<byte>> _dummyMessage = new ArraySegment<ArraySegment<byte>>(new[] { new ArraySegment<byte>(new byte[] { 1, 2, 3, 4 }) });
3439

35-
public ListenerPrimary(LibuvTransportContext transportContext) : base(transportContext)
40+
public ListenerPrimary(LibuvTransportContext transportContext, bool dispatchAll) : base(transportContext)
3641
{
42+
_dispatchAll = dispatchAll;
3743
}
3844

3945
/// <summary>
@@ -107,9 +113,27 @@ private void OnListenPipe(UvStreamHandle pipe, int status, UvException error)
107113

108114
protected override void DispatchConnection(UvStreamHandle socket)
109115
{
110-
var index = _dispatchIndex++ % (_dispatchPipes.Count + 1);
116+
var modulus = _dispatchAll ? _dispatchPipes.Count : (_dispatchPipes.Count + 1);
117+
if (modulus == 0)
118+
{
119+
if (_createdPipes.Count == 0)
120+
{
121+
#pragma warning disable CS0618 // Type or member is obsolete
122+
Log.LogError(0, $"Connection received before listeners were initialized - see https://aka.ms/dotnet/aspnet/finonerror for possible mitigations");
123+
#pragma warning restore CS0618 // Type or member is obsolete
124+
}
125+
else
126+
{
127+
Log.LogError(0, "Unable to process connection since listeners failed to initialize - see https://aka.ms/dotnet/aspnet/finonerror for possible mitigations");
128+
}
129+
130+
return;
131+
}
132+
133+
var index = _dispatchIndex++ % modulus;
111134
if (index == _dispatchPipes.Count)
112135
{
136+
Debug.Assert(!_dispatchAll, "Should have dispatched to a secondary listener");
113137
base.DispatchConnection(socket);
114138
}
115139
else

src/Servers/Kestrel/Transport.Libuv/src/LibuvTransportOptions.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv
1212
[Obsolete("The libuv transport is obsolete and will be removed in a future release. See https://aka.ms/libuvtransport for details.", error: false)] // Remove after .NET 6.
1313
public class LibuvTransportOptions
1414
{
15+
private const string FinOnErrorSwitch = "Microsoft.AspNetCore.Server.Kestrel.FinOnError";
16+
private static readonly bool _finOnError;
17+
18+
static LibuvTransportOptions()
19+
{
20+
AppContext.TryGetSwitch(FinOnErrorSwitch, out _finOnError);
21+
}
22+
23+
// Opt-out flag for back compat. Remove in 7.0.
24+
internal bool FinOnError { get; set; } = _finOnError;
25+
1526
/// <summary>
1627
/// The number of libuv I/O threads used to process requests.
1728
/// </summary>

0 commit comments

Comments
 (0)