Always throw same exception type on read after connection error (#975).

This commit is contained in:
Cesar Blum Silveira 2016-07-15 16:51:28 -07:00
parent 6bcf7643df
commit 3c0e0f8d88
8 changed files with 46 additions and 35 deletions

View File

@ -3,6 +3,7 @@
using System; using System;
using System.Diagnostics; using System.Diagnostics;
using System.IO;
using System.Threading; using System.Threading;
using System.Threading.Tasks; using System.Threading.Tasks;
using Microsoft.AspNetCore.Server.Kestrel.Filter; using Microsoft.AspNetCore.Server.Kestrel.Filter;
@ -128,13 +129,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
return _socketClosedTcs.Task; return _socketClosedTcs.Task;
} }
public virtual void Abort() public virtual void Abort(Exception error = null)
{ {
// Frame.Abort calls user code while this method is always // Frame.Abort calls user code while this method is always
// called from a libuv thread. // called from a libuv thread.
ThreadPool.Run(() => ThreadPool.Run(() =>
{ {
_frame.Abort(); _frame.Abort(error);
}); });
} }
@ -231,18 +232,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
} }
} }
Exception error = null; IOException error = null;
if (errorDone) if (errorDone)
{ {
handle.Libuv.Check(status, out error); Exception uvError;
Log.ConnectionError(ConnectionId, error); handle.Libuv.Check(status, out uvError);
Log.ConnectionError(ConnectionId, uvError);
error = new IOException(uvError.Message, uvError);
} }
SocketInput.IncomingComplete(readCount, error); SocketInput.IncomingComplete(readCount, error);
if (errorDone) if (errorDone)
{ {
Abort(); Abort(error);
} }
} }

View File

@ -339,13 +339,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
/// <summary> /// <summary>
/// Immediate kill the connection and poison the request and response streams. /// Immediate kill the connection and poison the request and response streams.
/// </summary> /// </summary>
public void Abort() public void Abort(Exception error = null)
{ {
if (Interlocked.CompareExchange(ref _requestAborted, 1, 0) == 0) if (Interlocked.CompareExchange(ref _requestAborted, 1, 0) == 0)
{ {
_requestProcessingStopping = true; _requestProcessingStopping = true;
_frameStreams?.RequestBody.Abort(); _frameStreams?.RequestBody.Abort(error);
_frameStreams?.ResponseBody.Abort(); _frameStreams?.ResponseBody.Abort();
try try

View File

@ -13,6 +13,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
{ {
private MessageBody _body; private MessageBody _body;
private FrameStreamState _state; private FrameStreamState _state;
private Exception _error;
public FrameRequestStream() public FrameRequestStream()
{ {
@ -163,13 +164,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
_body = null; _body = null;
} }
public void Abort() public void Abort(Exception error = null)
{ {
// We don't want to throw an ODE until the app func actually completes. // We don't want to throw an ODE until the app func actually completes.
// If the request is aborted, we throw an TaskCanceledException instead. // If the request is aborted, we throw an TaskCanceledException instead,
// unless error is not null, in which case we throw it.
if (_state != FrameStreamState.Closed) if (_state != FrameStreamState.Closed)
{ {
_state = FrameStreamState.Aborted; _state = FrameStreamState.Aborted;
_error = error;
} }
} }
@ -186,7 +189,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
case FrameStreamState.Closed: case FrameStreamState.Closed:
throw new ObjectDisposedException(nameof(FrameRequestStream)); throw new ObjectDisposedException(nameof(FrameRequestStream));
case FrameStreamState.Aborted: case FrameStreamState.Aborted:
return TaskUtilities.GetCancelledZeroTask(); return _error != null ?
TaskUtilities.GetFaultedTask(_error) :
TaskUtilities.GetCancelledZeroTask();
} }
return null; return null;
} }

View File

@ -3,7 +3,6 @@
using System; using System;
using System.Diagnostics; using System.Diagnostics;
using System.IO;
using System.Runtime.CompilerServices; using System.Runtime.CompilerServices;
using System.Threading; using System.Threading;
using System.Threading.Tasks; using System.Threading.Tasks;
@ -354,11 +353,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
var error = _tcs.Task.Exception?.InnerException; var error = _tcs.Task.Exception?.InnerException;
if (error != null) if (error != null)
{ {
if (error is TaskCanceledException || error is InvalidOperationException) throw error;
{
throw error;
}
throw new IOException(error.Message, error);
} }
} }
} }

View File

@ -1,6 +1,7 @@
// Copyright (c) .NET Foundation. All rights reserved. // Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Threading; using System.Threading;
using System.Threading.Tasks; using System.Threading.Tasks;
@ -39,6 +40,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure
var tcs = new TaskCompletionSource<int>(); var tcs = new TaskCompletionSource<int>();
tcs.TrySetCanceled(); tcs.TrySetCanceled();
return tcs.Task; return tcs.Task;
#endif
}
public static Task<int> GetFaultedTask(Exception error)
{
#if NETSTANDARD1_3
return Task.FromException<int>(error);
#else
var tcs = new TaskCompletionSource<int>();
tcs.SetException(error);
return tcs.Task;
#endif #endif
} }
} }

View File

@ -248,25 +248,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
{ {
await context.Request.Body.ReadAsync(new byte[1], 0, 1); 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) catch (IOException ex)
{ {
// This is one of two exception types that might thrown in this scenario. expectedExceptionThrown = ex.InnerException is UvException && ex.InnerException.Message.Contains("ECONNRESET");
// 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(); appDone.Release();

View File

@ -114,5 +114,17 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
var task = stream.ReadAsync(new byte[1], 0, 1); var task = stream.ReadAsync(new byte[1], 0, 1);
Assert.True(task.IsCanceled); Assert.True(task.IsCanceled);
} }
[Fact]
public void AbortWithErrorCausesReadToCancel()
{
var stream = new FrameRequestStream();
stream.StartAcceptingReads(null);
var error = new Exception();
stream.Abort(error);
var task = stream.ReadAsync(new byte[1], 0, 1);
Assert.True(task.IsFaulted);
Assert.Same(error, task.Exception.InnerException);
}
} }
} }

View File

@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests.TestHelpers
RequestAbortedSource = new CancellationTokenSource(); RequestAbortedSource = new CancellationTokenSource();
} }
public override void Abort() public override void Abort(Exception error = null)
{ {
if (RequestAbortedSource != null) if (RequestAbortedSource != null)
{ {