Treat ECONNRESET as a connection error (#934).

This commit is contained in:
Cesar Blum Silveira 2016-06-30 15:48:32 -07:00
parent fbcb5dcb1b
commit 3bb7f4e2c4
11 changed files with 202 additions and 61 deletions

View File

@ -282,7 +282,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
}
var normalRead = status > 0;
var normalDone = status == Constants.ECONNRESET || status == Constants.EOF;
var normalDone = status == Constants.EOF;
var errorDone = !(normalDone || normalRead);
var readCount = normalRead ? status : 0;
@ -293,13 +293,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
else
{
_socket.ReadStop();
Log.ConnectionReadFin(ConnectionId);
if (normalDone)
{
Log.ConnectionReadFin(ConnectionId);
}
}
Exception error = null;
if (errorDone)
{
handle.Libuv.Check(status, out error);
Log.ConnectionError(ConnectionId, error);
}
_rawSocketInput.IncomingComplete(readCount, error);

View File

@ -34,7 +34,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
{
while (!_requestProcessingStopping && TakeStartLine(SocketInput) != RequestLineStatus.Done)
{
if (SocketInput.RemoteIntakeFin)
if (SocketInput.CheckFinOrThrow())
{
// We need to attempt to consume start lines and headers even after
// SocketInput.RemoteIntakeFin is set to true to ensure we don't close a
@ -62,7 +62,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
while (!_requestProcessingStopping && !TakeMessageHeaders(SocketInput, FrameRequestHeaders))
{
if (SocketInput.RemoteIntakeFin)
if (SocketInput.CheckFinOrThrow())
{
// We need to attempt to consume start lines and headers even after
// SocketInput.RemoteIntakeFin is set to true to ensure we don't close a

View File

@ -239,7 +239,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
{
while (_mode == Mode.Prefix)
{
var fin = input.RemoteIntakeFin;
var fin = input.CheckFinOrThrow();
ParseChunkedPrefix(input);
@ -257,7 +257,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
while (_mode == Mode.Extension)
{
var fin = input.RemoteIntakeFin;
var fin = input.CheckFinOrThrow();
ParseExtension(input);
@ -275,7 +275,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
while (_mode == Mode.Data)
{
var fin = input.RemoteIntakeFin;
var fin = input.CheckFinOrThrow();
int actual = ReadChunkedData(input, buffer.Array, buffer.Offset, buffer.Count);
@ -297,7 +297,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
while (_mode == Mode.Suffix)
{
var fin = input.RemoteIntakeFin;
var fin = input.CheckFinOrThrow();
ParseChunkedSuffix(input);
@ -317,7 +317,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
// Chunks finished, parse trailers
while (_mode == Mode.Trailer)
{
var fin = input.RemoteIntakeFin;
var fin = input.CheckFinOrThrow();
ParseChunkedTrailer(input);
@ -337,7 +337,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
{
while (!_context.TakeMessageHeaders(input, _requestHeaders))
{
if (input.RemoteIntakeFin)
if (input.CheckFinOrThrow())
{
if (_context.TakeMessageHeaders(input, _requestHeaders))
{

View File

@ -22,7 +22,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
private readonly ManualResetEventSlim _manualResetEvent = new ManualResetEventSlim(false, 0);
private Action _awaitableState;
private Exception _awaitableError;
private MemoryPoolBlock _head;
private MemoryPoolBlock _tail;
@ -33,6 +32,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
private bool _consuming;
private bool _disposed;
private TaskCompletionSource<object> _tcs = new TaskCompletionSource<object>();
public SocketInput(MemoryPool memory, IThreadPool threadPool, IBufferSizeControl bufferSizeControl = null)
{
_memory = memory;
@ -41,10 +42,16 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
_awaitableState = _awaitableIsNotCompleted;
}
public bool RemoteIntakeFin { get; set; }
public bool IsCompleted => ReferenceEquals(_awaitableState, _awaitableIsCompleted);
private bool ReadingInput => _tcs.Task.Status == TaskStatus.WaitingForActivation;
public bool CheckFinOrThrow()
{
CheckConnectionError();
return _tcs.Task.Status == TaskStatus.RanToCompletion;
}
public MemoryPoolBlock IncomingStart()
{
const int minimumSize = 2048;
@ -87,7 +94,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
}
else
{
RemoteIntakeFin = true;
FinReceived();
}
Complete();
@ -122,13 +129,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
_pinned = null;
}
if (count == 0)
{
RemoteIntakeFin = true;
}
if (error != null)
{
_awaitableError = error;
SetConnectionError(error);
}
else if (count == 0)
{
FinReceived();
}
Complete();
@ -216,8 +223,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
if (!examined.IsDefault &&
examined.IsEnd &&
RemoteIntakeFin == false &&
_awaitableError == null)
ReadingInput)
{
_manualResetEvent.Reset();
@ -252,8 +258,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
public void AbortAwaiting()
{
_awaitableError = new TaskCanceledException("The request was aborted");
SetConnectionError(new TaskCanceledException("The request was aborted"));
Complete();
}
@ -279,7 +284,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
}
else
{
_awaitableError = new InvalidOperationException("Concurrent reads are not supported.");
SetConnectionError(new InvalidOperationException("Concurrent reads are not supported."));
Interlocked.Exchange(
ref _awaitableState,
@ -303,15 +308,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
{
_manualResetEvent.Wait();
}
var error = _awaitableError;
if (error != null)
{
if (error is TaskCanceledException || error is InvalidOperationException)
{
throw error;
}
throw new IOException(error.Message, error);
}
CheckConnectionError();
}
public void Dispose()
@ -340,5 +338,28 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
returnBlock.Pool.Return(returnBlock);
}
}
private void SetConnectionError(Exception error)
{
_tcs.TrySetException(error);
}
private void FinReceived()
{
_tcs.TrySetResult(null);
}
private void CheckConnectionError()
{
var error = _tcs.Task.Exception?.InnerException;
if (error != null)
{
if (error is TaskCanceledException || error is InvalidOperationException)
{
throw error;
}
throw new IOException(error.Message, error);
}
}
}
}

View File

@ -11,7 +11,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
{
while (input.IsCompleted)
{
var fin = input.RemoteIntakeFin;
var fin = input.CheckFinOrThrow();
var begin = input.ConsumingStart();
int actual;
@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
{
await input;
var fin = input.RemoteIntakeFin;
var fin = input.CheckFinOrThrow();
var begin = input.ConsumingStart();
int actual;

View File

@ -10,7 +10,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
public const int ListenBacklog = 128;
public const int EOF = -4095;
public static readonly int? ECONNRESET = GetECONNRESET();
public static readonly int? EADDRINUSE = GetEADDRINUSE();
/// <summary>
@ -26,23 +25,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
public const string ServerName = "Kestrel";
private static int? GetECONNRESET()
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
return -4077;
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
{
return -104;
}
else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
{
return -54;
}
return null;
}
private static int? GetEADDRINUSE()
{
if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))

View File

@ -92,10 +92,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel
"ThreadCount must be positive.");
}
if (!Constants.ECONNRESET.HasValue)
{
_logger.LogWarning("Unable to determine ECONNRESET value on this platform.");
}
if (!Constants.EADDRINUSE.HasValue)
{
_logger.LogWarning("Unable to determine EADDRINUSE value on this platform.");

View File

@ -2,17 +2,23 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Net.Sockets;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Server.Kestrel.Internal.Networking;
using Microsoft.AspNetCore.Testing;
using Microsoft.AspNetCore.Testing.xunit;
using Microsoft.Extensions.Logging.Testing;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;
using Xunit;
@ -184,6 +190,102 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
}
}
[Fact]
public async Task ConnectionResetAbortsRequest()
{
var connectionErrorLogged = new SemaphoreSlim(0);
var testSink = new ConnectionErrorTestSink(() => connectionErrorLogged.Release());
var builder = new WebHostBuilder()
.UseLoggerFactory(new TestLoggerFactory(testSink, true))
.UseKestrel()
.UseUrls($"http://127.0.0.1:0")
.Configure(app => app.Run(context =>
{
return Task.FromResult(0);
}));
using (var host = builder.Build())
{
host.Start();
using (var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp))
{
socket.Connect(new IPEndPoint(IPAddress.Loopback, host.GetPort()));
socket.LingerState = new LingerOption(true, 0);
}
// Wait until connection error is logged
Assert.True(await connectionErrorLogged.WaitAsync(2500));
// Check for expected message
Assert.NotNull(testSink.ConnectionErrorMessage);
Assert.Contains("ECONNRESET", testSink.ConnectionErrorMessage);
}
}
[Fact]
public async Task ThrowsOnReadAfterConnectionError()
{
var requestStarted = new SemaphoreSlim(0);
var connectionReset = new SemaphoreSlim(0);
var appDone = new SemaphoreSlim(0);
var expectedExceptionThrown = false;
var builder = new WebHostBuilder()
.UseKestrel()
.UseUrls($"http://127.0.0.1:0")
.Configure(app => app.Run(async context =>
{
requestStarted.Release();
Assert.True(await connectionReset.WaitAsync(2500));
try
{
await context.Request.Body.ReadAsync(new byte[1], 0, 1);
}
catch (BadHttpRequestException)
{
// We need this here because BadHttpRequestException derives from IOException,
// and we're looking for an actual IOException.
}
catch (IOException ex)
{
// This is one of two exception types that might thrown in this scenario.
// An IOException is thrown if ReadAsync is awaiting on SocketInput when
// the connection is aborted.
expectedExceptionThrown = ex.InnerException is UvException;
}
catch (TaskCanceledException)
{
// This is the other exception type that might be thrown here.
// A TaskCanceledException is thrown if ReadAsync is called when the
// connection has already been aborted, since FrameRequestStream is
// aborted and returns a canceled task from ReadAsync.
expectedExceptionThrown = true;
}
appDone.Release();
}));
using (var host = builder.Build())
{
host.Start();
using (var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp))
{
socket.Connect(new IPEndPoint(IPAddress.Loopback, host.GetPort()));
socket.LingerState = new LingerOption(true, 0);
socket.Send(Encoding.ASCII.GetBytes("GET / HTTP/1.1\r\nContent-Length: 1\r\n\r\n"));
Assert.True(await requestStarted.WaitAsync(2500));
}
connectionReset.Release();
Assert.True(await appDone.WaitAsync(2500));
Assert.True(expectedExceptionThrown);
}
}
private async Task TestRemoteIPAddress(string registerAddress, string requestAddress, string expectAddress)
{
var builder = new WebHostBuilder()
@ -220,5 +322,40 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
Assert.NotEmpty(facts["RemotePort"].Value<string>());
}
}
private class ConnectionErrorTestSink : ITestSink
{
private readonly Action _connectionErrorLogged;
public ConnectionErrorTestSink(Action connectionErrorLogged)
{
_connectionErrorLogged = connectionErrorLogged;
}
public string ConnectionErrorMessage { get; set; }
public Func<BeginScopeContext, bool> BeginEnabled { get; set; }
public List<BeginScopeContext> Scopes { get; set; }
public Func<WriteContext, bool> WriteEnabled { get; set; }
public List<WriteContext> Writes { get; set; }
public void Begin(BeginScopeContext context)
{
}
public void Write(WriteContext context)
{
const int connectionErrorEventId = 14;
if (context.EventId.Id == connectionErrorEventId)
{
ConnectionErrorMessage = context.Exception?.Message;
_connectionErrorLogged();
}
}
}
}
}
}

View File

@ -6,7 +6,7 @@
"Microsoft.AspNetCore.Server.Kestrel": "1.1.0-*",
"Microsoft.AspNetCore.Server.Kestrel.Https": "1.1.0-*",
"Microsoft.AspNetCore.Testing": "1.1.0-*",
"Microsoft.Extensions.Logging.Console": "1.1.0-*",
"Microsoft.Extensions.Logging.Testing": "1.1.0-*",
"Newtonsoft.Json": "9.0.1",
"xunit": "2.2.0-*"
},

View File

@ -38,7 +38,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
Libuv.uv_buf_t ignored;
mockLibuv.AllocCallback(socket.InternalGetHandle(), 2048, out ignored);
mockLibuv.ReadCallback(socket.InternalGetHandle(), 0, ref ignored);
Assert.False(connection.SocketInput.RemoteIntakeFin);
Assert.False(connection.SocketInput.CheckFinOrThrow());
connection.ConnectionControl.End(ProduceEndType.SocketDisconnect);
}

View File

@ -40,7 +40,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
FrameContext.SocketInput.IncomingData(data, 0, data.Length);
if (fin)
{
FrameContext.SocketInput.RemoteIntakeFin = true;
FrameContext.SocketInput.IncomingFin();
}
}