Skip to content

Commit

Permalink
Merged PR 32094: [6.0] Forcibly close socket on abort
Browse files Browse the repository at this point in the history
Forcibly close socket when connection is aborted.
  • Loading branch information
amcasey authored and adityamandaleeka committed Jul 15, 2023
1 parent a62c85f commit cec88a3
Show file tree
Hide file tree
Showing 24 changed files with 811 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -486,8 +486,15 @@ public async Task ClosesConnectionOnServerAbortOutOfProcess()
var response = await deploymentResult.HttpClient.GetAsync("/Abort").TimeoutAfter(TimeoutExtensions.DefaultTimeoutValue);

Assert.Equal(HttpStatusCode.BadGateway, response.StatusCode);

#if NEWSHIM_FUNCTIONALS
// In-proc SocketConnection isn't used and there's no abort
// 0x80072f78 ERROR_HTTP_INVALID_SERVER_RESPONSE The server returned an invalid or unrecognized response
Assert.Contains("0x80072f78", await response.Content.ReadAsStringAsync());
#else
// 0x80072efe ERROR_INTERNET_CONNECTION_ABORTED The connection with the server was terminated abnormally
Assert.Contains("0x80072efe", await response.Content.ReadAsStringAsync());
#endif
}
catch (HttpRequestException)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,14 @@ protected override void OnRequestProcessingEnded()
_http1Output.Dispose();
}

public void OnInputOrOutputCompleted()
void IRequestProcessor.OnInputOrOutputCompleted()
{
// Closed gracefully.
_http1Output.Abort(ServerOptions.FinOnError ? new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient) : null!);
CancelRequestAbortedToken();
}

void IHttpOutputAborter.OnInputOrOutputCompleted()
{
_http1Output.Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient));
CancelRequestAbortedToken();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ protected void ThrowUnexpectedEndOfRequestContent()
// so we call OnInputOrOutputCompleted() now to prevent a race in our tests where a 400
// response is written after observing the unexpected end of request content instead of just
// closing the connection without a response as expected.
_context.OnInputOrOutputCompleted();
((IHttpOutputAborter)_context).OnInputOrOutputCompleted();

KestrelBadHttpRequestException.Throw(RequestRejectionReason.UnexpectedEndOfRequestContent);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ public Http2Connection(HttpConnectionContext context)
public void OnInputOrOutputCompleted()
{
TryClose();
_frameWriter.Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient));
var useException = _context.ServiceContext.ServerOptions.FinOnError || _clientActiveStreamCount != 0;
_frameWriter.Abort(useException ? new ConnectionAbortedException(CoreStrings.ConnectionAbortedByClient) : null!);
}

public void Abort(ConnectionAbortedException ex)
Expand Down
11 changes: 11 additions & 0 deletions src/Servers/Kestrel/Core/src/KestrelServerOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
/// </summary>
public class KestrelServerOptions
{
private const string FinOnErrorSwitch = "Microsoft.AspNetCore.Server.Kestrel.FinOnError";
private static readonly bool _finOnError;

static KestrelServerOptions()
{
AppContext.TryGetSwitch(FinOnErrorSwitch, out _finOnError);
}

// internal to fast-path header decoding when RequestHeaderEncodingSelector is unchanged.
internal static readonly Func<string, Encoding?> DefaultHeaderEncodingSelector = _ => null;

// Opt-out flag for back compat. Remove in 9.0 (or make public).
internal bool FinOnError { get; set; } = _finOnError;

private Func<string, Encoding?> _requestHeaderEncodingSelector = DefaultHeaderEncodingSelector;

private Func<string, Encoding?> _responseHeaderEncodingSelector = DefaultHeaderEncodingSelector;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ internal interface ILibuvTrace : ILogger

void ConnectionWriteFin(string connectionId, string reason);

void ConnectionWriteRst(string connectionId, string reason);

void ConnectionWrite(string connectionId, int count);

void ConnectionWriteCallback(string connectionId, int status);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
using System.IO;
using System.IO.Pipelines;
using System.Net;
using System.Net.Sockets;
using System.Reflection.Metadata;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Connections;
Expand All @@ -28,6 +30,8 @@ internal partial class LibuvConnection : TransportConnection
private readonly IDuplexPipe _originalTransport;
private readonly CancellationTokenSource _connectionClosedTokenSource = new CancellationTokenSource();

private readonly bool _finOnError;

private volatile ConnectionAbortedException _abortReason;

private MemoryHandle _bufferHandle;
Expand All @@ -43,9 +47,11 @@ public LibuvConnection(UvStreamHandle socket,
PipeOptions inputOptions = null,
PipeOptions outputOptions = null,
long? maxReadBufferSize = null,
long? maxWriteBufferSize = null)
long? maxWriteBufferSize = null,
bool finOnError = false)
{
_socket = socket;
_finOnError = finOnError;

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

if (!_finOnError && _abortReason is not null)
{
// When shutdown isn't clean (note that we're using _abortReason, rather than inputError, to exclude that case),
// we set the DontLinger socket option to cause libuv to send a RST and release any buffered response data.
SetDontLingerOption(_socket);
}

// Now, complete the input so that no more reads can happen
Input.Complete(inputError);
Output.Complete(outputError);
Expand All @@ -132,8 +145,16 @@ private async Task StartCore()
// on the stream handle
Input.CancelPendingFlush();

// Send a FIN
Log.ConnectionWriteFin(ConnectionId, inputError.Message);
if (!_finOnError && _abortReason is not null)
{
// Send a RST
Log.ConnectionWriteRst(ConnectionId, inputError.Message);
}
else
{
// Send a FIN
Log.ConnectionWriteFin(ConnectionId, inputError.Message);
}

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

/// <remarks>
/// This should be called on <see cref="_socket"/> before it is disposed.
/// Both <see cref="Abort"/> and <see cref="StartCore"/> call dispose but, rather than predict
/// which will do so first (which varies), we make this method idempotent and call it in both.
/// </remarks>
private static void SetDontLingerOption(UvStreamHandle socket)
{
if (!socket.IsClosed && !socket.IsInvalid)
{
var libuv = socket.Libuv;
var pSocket = IntPtr.Zero;
libuv.uv_fileno(socket, ref pSocket);

// libuv doesn't expose setsockopt, so we take advantage of the fact that
// Socket already has a PAL
using var managedHandle = new SafeSocketHandle(pSocket, ownsHandle: false);
using var managedSocket = new Socket(managedHandle);
managedSocket.LingerState = new LingerOption(enable: true, seconds: 0);
}
}

public override void Abort(ConnectionAbortedException abortReason)
{
_abortReason = abortReason;

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

// This cancels any pending I/O.
Thread.Post(s => s.Dispose(), _socket);
Thread.Post(static (self) =>
{
if (!self._finOnError)
{
SetDontLingerOption(self._socket);
}

// This cancels any pending I/O.
self._socket.Dispose();
}, this);
}

public override async ValueTask DisposeAsync()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Net;
using System.Threading;
Expand Down Expand Up @@ -134,9 +135,24 @@ internal async Task BindAsync()
{
// TODO: Move thread management to LibuvTransportFactory
// TODO: Split endpoint management from thread management

// When `FinOnError` is false (the default), we need to be able to forcibly abort connections.
// On Windows, libuv 1.10.0 will call `shutdown`, preventing forcible abort, on any socket
// not flagged as `UV_HANDLE_SHARED_TCP_SOCKET`. The only way we've found to cause socket
// to be flagged as `UV_HANDLE_SHARED_TCP_SOCKET` is to share it across a named pipe (which
// must, itself, be flagged `ipc`), which naturally happens when a `ListenerPrimary` dispatches
// a connection to a `ListenerSecondary`. Therefore, in scenarios where this is required, we
// tell the `ListenerPrimary` to dispatch *all* connections to secondary and create an
// additional `ListenerSecondary` to replace the lost capacity.
var dispatchAllToSecondary = Libuv.IsWindows && !TransportContext.Options.FinOnError;

#pragma warning disable CS0618
for (var index = 0; index < TransportOptions.ThreadCount; index++)
var threadCount = dispatchAllToSecondary
? TransportOptions.ThreadCount + 1
: TransportOptions.ThreadCount;
#pragma warning restore CS0618

for (var index = 0; index < threadCount; index++)
{
Threads.Add(new LibuvThread(Libuv, TransportContext));
}
Expand All @@ -148,10 +164,10 @@ internal async Task BindAsync()

try
{
#pragma warning disable CS0618
if (TransportOptions.ThreadCount == 1)
#pragma warning restore CS0618
if (threadCount == 1)
{
Debug.Assert(!dispatchAllToSecondary, "Should have taken the primary/secondary code path");

var listener = new Listener(TransportContext);
_listeners.Add(listener);
await listener.StartAsync(EndPoint, Threads[0]).ConfigureAwait(false);
Expand All @@ -162,7 +178,7 @@ internal async Task BindAsync()
var pipeName = (Libuv.IsWindows ? @"\\.\pipe\kestrel_" : "/tmp/kestrel_") + Guid.NewGuid().ToString("n");
var pipeMessage = Guid.NewGuid().ToByteArray();

var listenerPrimary = new ListenerPrimary(TransportContext);
var listenerPrimary = new ListenerPrimary(TransportContext, dispatchAllToSecondary);
_listeners.Add(listenerPrimary);
await listenerPrimary.StartAsync(pipeName, pipeMessage, EndPoint, Threads[0]).ConfigureAwait(false);
EndPoint = listenerPrimary.EndPoint;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ public void ConnectionRead(string connectionId, int count)
[LoggerMessage(7, LogLevel.Debug, @"Connection id ""{ConnectionId}"" sending FIN because: ""{Reason}""", EventName = nameof(ConnectionWriteFin))]
public partial void ConnectionWriteFin(string connectionId, string reason);

[LoggerMessage(8, LogLevel.Debug, @"Connection id ""{ConnectionId}"" sending RST because: ""{Reason}""", EventName = nameof(ConnectionWriteRst))]
public partial void ConnectionWriteRst(string connectionId, string reason);

public void ConnectionWrite(string connectionId, int count)
{
// Don't log for now since this could be *too* verbose.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ protected internal void HandleConnection(UvStreamHandle socket)

var options = TransportContext.Options;
#pragma warning disable CS0618
var connection = new LibuvConnection(socket, TransportContext.Log, Thread, remoteEndPoint, localEndPoint, InputOptions, OutputOptions, options.MaxReadBufferSize, options.MaxWriteBufferSize);
var connection = new LibuvConnection(socket, TransportContext.Log, Thread, remoteEndPoint, localEndPoint, InputOptions, OutputOptions, options.MaxReadBufferSize, options.MaxWriteBufferSize, options.FinOnError);
#pragma warning restore CS0618
connection.Start();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Net;
using System.Runtime.InteropServices;
Expand All @@ -22,6 +23,10 @@ internal class ListenerPrimary : Listener
private readonly List<UvPipeHandle> _dispatchPipes = new List<UvPipeHandle>();
// The list of pipes we've created but may not be part of _dispatchPipes
private readonly List<UvPipeHandle> _createdPipes = new List<UvPipeHandle>();

// If true, dispatch all connections to _dispatchPipes - don't process any in the primary
private readonly bool _dispatchAll;

private int _dispatchIndex;
private string _pipeName;
private byte[] _pipeMessage;
Expand All @@ -32,8 +37,9 @@ internal class ListenerPrimary : Listener
// but it has no other functional significance
private readonly ArraySegment<ArraySegment<byte>> _dummyMessage = new ArraySegment<ArraySegment<byte>>(new[] { new ArraySegment<byte>(new byte[] { 1, 2, 3, 4 }) });

public ListenerPrimary(LibuvTransportContext transportContext) : base(transportContext)
public ListenerPrimary(LibuvTransportContext transportContext, bool dispatchAll) : base(transportContext)
{
_dispatchAll = dispatchAll;
}

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

protected override void DispatchConnection(UvStreamHandle socket)
{
var index = _dispatchIndex++ % (_dispatchPipes.Count + 1);
var modulus = _dispatchAll ? _dispatchPipes.Count : (_dispatchPipes.Count + 1);
if (modulus == 0)
{
if (_createdPipes.Count == 0)
{
#pragma warning disable CS0618 // Type or member is obsolete
Log.LogError(0, $"Connection received before listeners were initialized - see https://aka.ms/dotnet/aspnet/finonerror for possible mitigations");
#pragma warning restore CS0618 // Type or member is obsolete
}
else
{
Log.LogError(0, "Unable to process connection since listeners failed to initialize - see https://aka.ms/dotnet/aspnet/finonerror for possible mitigations");
}

return;
}

var index = _dispatchIndex++ % modulus;
if (index == _dispatchPipes.Count)
{
Debug.Assert(!_dispatchAll, "Should have dispatched to a secondary listener");
base.DispatchConnection(socket);
}
else
Expand Down
11 changes: 11 additions & 0 deletions src/Servers/Kestrel/Transport.Libuv/src/LibuvTransportOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv
[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.
public class LibuvTransportOptions
{
private const string FinOnErrorSwitch = "Microsoft.AspNetCore.Server.Kestrel.FinOnError";
private static readonly bool _finOnError;

static LibuvTransportOptions()
{
AppContext.TryGetSwitch(FinOnErrorSwitch, out _finOnError);
}

// Opt-out flag for back compat. Remove in 7.0.
internal bool FinOnError { get; set; } = _finOnError;

/// <summary>
/// The number of libuv I/O threads used to process requests.
/// </summary>
Expand Down
Loading

0 comments on commit cec88a3

Please sign in to comment.