Merged PR 32090: [7.0] Forcibly close socket on abort

Forcibly close socket when connection is aborted.

Author: amcasey

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

@@ -535,8 +535,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)
  {

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

@@ -98,7 +98,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();

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

@@ -204,7 +204,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);
  }

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

@@ -159,7 +159,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)

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

@@ -26,10 +26,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core;
 public class KestrelServerOptions
 {
  internal const string DisableHttp1LineFeedTerminatorsSwitchKey = "Microsoft.AspNetCore.Server.Kestrel.DisableHttp1LineFeedTerminators";
+ 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;

src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs

@@ -30,6 +30,7 @@ internal sealed partial class SocketConnection : TransportConnection
  private readonly TaskCompletionSource _waitForConnectionClosedTcs = new TaskCompletionSource();
  private bool _connectionClosed;
  private readonly bool _waitForData;
+ private readonly bool _finOnError;
 
  internal SocketConnection(Socket socket,
  MemoryPool<byte> memoryPool,
@@ -38,7 +39,8 @@ internal SocketConnection(Socket socket,
  SocketSenderPool socketSenderPool,
  PipeOptions inputOptions,
  PipeOptions outputOptions,
- bool waitForData = true)
+ bool waitForData = true,
+ bool finOnError = false)
  {
  Debug.Assert(socket != null);
  Debug.Assert(memoryPool != null);
@@ -49,6 +51,7 @@ internal SocketConnection(Socket socket,
  _logger = logger;
  _waitForData = waitForData;
  _socketSenderPool = socketSenderPool;
+ _finOnError = finOnError;
 
  LocalEndPoint = _socket.LocalEndPoint;
  RemoteEndPoint = _socket.RemoteEndPoint;
@@ -380,11 +383,21 @@ private void Shutdown(Exception? shutdownReason)
  // ever observe the nondescript ConnectionAbortedException except for connection middleware attempting
  // to half close the connection which is currently unsupported.
  _shutdownReason = shutdownReason ?? new ConnectionAbortedException("The Socket transport's send loop completed gracefully.");
+
+ // NB: not _shutdownReason since we don't want to do this on graceful completion
+ if (!_finOnError && shutdownReason is not null)
+ {
+ SocketsLog.ConnectionWriteRst(_logger, this, shutdownReason.Message);
+
+ // This forces an abortive close with linger time 0 (and implies Dispose)
+ _socket.Close(timeout: 0);
+ return;
+ }
+
  SocketsLog.ConnectionWriteFin(_logger, this, _shutdownReason.Message);
 
  try
  {
- // Try to gracefully close the socket even for aborts to match libuv behavior.
  _socket.Shutdown(SocketShutdown.Both);
  }
  catch

src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketsLog.cs

@@ -31,6 +31,17 @@ public static void ConnectionWriteFin(ILogger logger, SocketConnection connectio
  }
  }
 
+ [LoggerMessage(8, LogLevel.Debug, @"Connection id ""{ConnectionId}"" sending RST because: ""{Reason}""", EventName = "ConnectionWriteRst", SkipEnabledCheck = true)]
+ private static partial void ConnectionWriteRstCore(ILogger logger, string connectionId, string reason);
+
+ public static void ConnectionWriteRst(ILogger logger, SocketConnection connection, string reason)
+ {
+ if (logger.IsEnabled(LogLevel.Debug))
+ {
+ ConnectionWriteRstCore(logger, connection.ConnectionId, reason);
+ }
+ }
+
  // Reserved: Event ID 11, EventName = ConnectionWrite
 
  // Reserved: Event ID 12, EventName = ConnectionWriteCallback

src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionContextFactory.cs

@@ -113,7 +113,8 @@ public ConnectionContext Create(Socket socket)
  setting.SocketSenderPool,
  setting.InputOptions,
  setting.OutputOptions,
- waitForData: _options.WaitForDataBeforeAllocatingBuffer);
+ waitForData: _options.WaitForDataBeforeAllocatingBuffer,
+ finOnError: _options.FinOnError);
 
  connection.Start();
  return connection;

src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionFactoryOptions.cs

@@ -23,8 +23,12 @@ internal SocketConnectionFactoryOptions(SocketTransportOptions transportOptions)
  MaxWriteBufferSize = transportOptions.MaxWriteBufferSize;
  UnsafePreferInlineScheduling = transportOptions.UnsafePreferInlineScheduling;
  MemoryPoolFactory = transportOptions.MemoryPoolFactory;
+ FinOnError = transportOptions.FinOnError;
  }
 
+ // Opt-out flag for back compat. Remove in 9.0 (or make public).
+ internal bool FinOnError { get; set; }
+
  /// <summary>
  /// The number of I/O queues used to process requests. Set to 0 to directly schedule I/O to the ThreadPool.
  /// </summary>

src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs

@@ -13,6 +13,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets;
 /// </summary>
 public class SocketTransportOptions
 {
+ private const string FinOnErrorSwitch = "Microsoft.AspNetCore.Server.Kestrel.FinOnError";
+ private static readonly bool _finOnError;
+
+ static SocketTransportOptions()
+ {
+ AppContext.TryGetSwitch(FinOnErrorSwitch, out _finOnError);
+ }
+
+ // Opt-out flag for back compat. Remove in 9.0 (or make public).
+ internal bool FinOnError { get; set; } = _finOnError;
+
  /// <summary>
  /// The number of I/O queues used to process requests. Set to 0 to directly schedule I/O to the ThreadPool.
  /// </summary>