From 89f48508838d2539188e2c66235a08db313e265a Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Tue, 28 Aug 2018 10:56:46 -0700 Subject: [PATCH] Avoid throwing the same exception instances in parallel (#2859) --- .../Internal/Http/HttpProtocol.cs | 2 +- .../ConnectionManagerShutdownExtensions.cs | 3 +- .../Internal/Infrastructure/Streams.cs | 4 +-- ... => ThrowingWasUpgradedWriteOnlyStream.cs} | 15 +++------- .../Internal/LibuvConnection.cs | 4 +++ ...ThrowingWasUpgradedWriteOnlyStreamTests.cs | 28 ++++++++++++++++++ .../ThrowingWriteOnlyStreamTests.cs | 29 ------------------- 7 files changed, 40 insertions(+), 45 deletions(-) rename src/Kestrel.Core/Internal/Infrastructure/{ThrowingWriteOnlyStream.cs => ThrowingWasUpgradedWriteOnlyStream.cs} (75%) create mode 100644 test/Kestrel.Core.Tests/ThrowingWasUpgradedWriteOnlyStreamTests.cs delete mode 100644 test/Kestrel.Core.Tests/ThrowingWriteOnlyStreamTests.cs diff --git a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs index f272f0ba29..5340ee8a0c 100644 --- a/src/Kestrel.Core/Internal/Http/HttpProtocol.cs +++ b/src/Kestrel.Core/Internal/Http/HttpProtocol.cs @@ -1212,7 +1212,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http var ex = new InvalidOperationException(CoreStrings.FormatHeaderNotAllowedOnResponse("Transfer-Encoding", StatusCode)); if (!appCompleted) { - // Back out of header creation surface exeception in user code + // Back out of header creation surface exception in user code _requestProcessingStatus = RequestProcessingStatus.AppStarted; throw ex; } diff --git a/src/Kestrel.Core/Internal/Infrastructure/ConnectionManagerShutdownExtensions.cs b/src/Kestrel.Core/Internal/Infrastructure/ConnectionManagerShutdownExtensions.cs index 928db83c92..c4a75c97fb 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/ConnectionManagerShutdownExtensions.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/ConnectionManagerShutdownExtensions.cs @@ -27,11 +27,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure public static async Task AbortAllConnectionsAsync(this ConnectionManager connectionManager) { var abortTasks = new List(); - var canceledException = new ConnectionAbortedException(CoreStrings.ConnectionAbortedDuringServerShutdown); connectionManager.Walk(connection => { - connection.TransportConnection.Abort(canceledException); + connection.TransportConnection.Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedDuringServerShutdown)); abortTasks.Add(connection.ExecutionTask); }); diff --git a/src/Kestrel.Core/Internal/Infrastructure/Streams.cs b/src/Kestrel.Core/Internal/Infrastructure/Streams.cs index a439928e5e..64522047f8 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/Streams.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/Streams.cs @@ -10,8 +10,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { public class Streams { - private static readonly ThrowingWriteOnlyStream _throwingResponseStream - = new ThrowingWriteOnlyStream(new InvalidOperationException(CoreStrings.ResponseStreamWasUpgraded)); + private static readonly ThrowingWasUpgradedWriteOnlyStream _throwingResponseStream + = new ThrowingWasUpgradedWriteOnlyStream(); private readonly HttpResponseStream _response; private readonly HttpRequestStream _request; private readonly WrappingStream _upgradeableResponse; diff --git a/src/Kestrel.Core/Internal/Infrastructure/ThrowingWriteOnlyStream.cs b/src/Kestrel.Core/Internal/Infrastructure/ThrowingWasUpgradedWriteOnlyStream.cs similarity index 75% rename from src/Kestrel.Core/Internal/Infrastructure/ThrowingWriteOnlyStream.cs rename to src/Kestrel.Core/Internal/Infrastructure/ThrowingWasUpgradedWriteOnlyStream.cs index aff2b7a6d4..cbb4543373 100644 --- a/src/Kestrel.Core/Internal/Infrastructure/ThrowingWriteOnlyStream.cs +++ b/src/Kestrel.Core/Internal/Infrastructure/ThrowingWasUpgradedWriteOnlyStream.cs @@ -8,15 +8,8 @@ using System.Threading.Tasks; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure { - public class ThrowingWriteOnlyStream : WriteOnlyStream + public class ThrowingWasUpgradedWriteOnlyStream : WriteOnlyStream { - private readonly Exception _exception; - - public ThrowingWriteOnlyStream(Exception exception) - { - _exception = exception; - } - public override bool CanSeek => false; public override long Length => throw new NotSupportedException(); @@ -28,13 +21,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure } public override void Write(byte[] buffer, int offset, int count) - => throw _exception; + => throw new InvalidOperationException(CoreStrings.ResponseStreamWasUpgraded); public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) - => throw _exception; + => throw new InvalidOperationException(CoreStrings.ResponseStreamWasUpgraded); public override void Flush() - => throw _exception; + => throw new InvalidOperationException(CoreStrings.ResponseStreamWasUpgraded); public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); diff --git a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs index 18283bb63b..1280e0c204 100644 --- a/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs +++ b/src/Kestrel.Transport.Libuv/Internal/LibuvConnection.cs @@ -88,6 +88,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal } else { + // This is unexpected. + Log.ConnectionError(ConnectionId, ex); + inputError = ex; outputError = ex; } @@ -240,6 +243,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Internal } else { + // This is unexpected. Log.ConnectionError(ConnectionId, uvError); return new IOException(uvError.Message, uvError); } diff --git a/test/Kestrel.Core.Tests/ThrowingWasUpgradedWriteOnlyStreamTests.cs b/test/Kestrel.Core.Tests/ThrowingWasUpgradedWriteOnlyStreamTests.cs new file mode 100644 index 0000000000..160939878e --- /dev/null +++ b/test/Kestrel.Core.Tests/ThrowingWasUpgradedWriteOnlyStreamTests.cs @@ -0,0 +1,28 @@ +// 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. + +using System; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests +{ + public class ThrowingWasUpgradedWriteOnlyStreamTests + { + [Fact] + public async Task ThrowsOnWrite() + { + var stream = new ThrowingWasUpgradedWriteOnlyStream(); + + Assert.True(stream.CanWrite); + Assert.False(stream.CanRead); + Assert.False(stream.CanSeek); + Assert.False(stream.CanTimeout); + Assert.Equal(CoreStrings.ResponseStreamWasUpgraded, Assert.Throws(() => stream.Write(new byte[1], 0, 1)).Message); + Assert.Equal(CoreStrings.ResponseStreamWasUpgraded, (await Assert.ThrowsAsync(() => stream.WriteAsync(new byte[1], 0, 1))).Message); + Assert.Equal(CoreStrings.ResponseStreamWasUpgraded, Assert.Throws(() => stream.Flush()).Message); + Assert.Equal(CoreStrings.ResponseStreamWasUpgraded, (await Assert.ThrowsAsync(() => stream.FlushAsync())).Message); + } + } +} diff --git a/test/Kestrel.Core.Tests/ThrowingWriteOnlyStreamTests.cs b/test/Kestrel.Core.Tests/ThrowingWriteOnlyStreamTests.cs deleted file mode 100644 index 7e6490e4f3..0000000000 --- a/test/Kestrel.Core.Tests/ThrowingWriteOnlyStreamTests.cs +++ /dev/null @@ -1,29 +0,0 @@ -// 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. - -using System; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; -using Xunit; - -namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests -{ - public class ThrowingWriteOnlyStreamTests - { - [Fact] - public async Task ThrowsOnWrite() - { - var ex = new Exception("my error"); - var stream = new ThrowingWriteOnlyStream(ex); - - Assert.True(stream.CanWrite); - Assert.False(stream.CanRead); - Assert.False(stream.CanSeek); - Assert.False(stream.CanTimeout); - Assert.Same(ex, Assert.Throws(() => stream.Write(new byte[1], 0, 1))); - Assert.Same(ex, await Assert.ThrowsAsync(() => stream.WriteAsync(new byte[1], 0, 1))); - Assert.Same(ex, Assert.Throws(() => stream.Flush())); - Assert.Same(ex, await Assert.ThrowsAsync(() => stream.FlushAsync())); - } - } -}