From ff1ee1e301d25529e77ad54ccc4c961f45190ae0 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Fri, 12 Apr 2019 15:41:06 -0700 Subject: [PATCH 01/14] Pushing to see diff --- .../CommonLib/CommonLib.vcxproj | 4 ++ ...HttpContext.IHttpRequestLifetimeFeature.cs | 57 ++++++++----------- .../IIS/IIS/src/Core/IISHttpContext.IO.cs | 34 ++++++++--- .../IIS/IIS/src/Core/IISHttpContext.cs | 14 ++++- .../IIS/IIS/src/Core/IISHttpContextOfT.cs | 2 +- 5 files changed, 65 insertions(+), 46 deletions(-) diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj index 9fd8d246712d..9c00e7b073c2 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj @@ -34,6 +34,7 @@ true v142 Unicode + StaticLibrary @@ -41,12 +42,14 @@ v142 true Unicode + StaticLibrary true v142 Unicode + StaticLibrary @@ -54,6 +57,7 @@ v142 false Unicode + diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs index ab46f8df5c3e..bc1cc4aacd3b 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs @@ -12,8 +12,11 @@ internal partial class IISHttpContext : IHttpRequestLifetimeFeature { private CancellationTokenSource _abortedCts; private CancellationToken? _manuallySetRequestAbortToken; + private object _abortLock = new object(); + private bool _preventRequestAbortedCancellation; + protected bool _connectionAborted; - CancellationToken IHttpRequestLifetimeFeature.RequestAborted + public CancellationToken RequestAborted { get { @@ -22,16 +25,26 @@ CancellationToken IHttpRequestLifetimeFeature.RequestAborted { return _manuallySetRequestAbortToken.Value; } - // Otherwise, get the abort CTS. If we have one, which would mean that someone previously - // asked for the RequestAborted token, simply return its token. If we don't, - // check to see whether we've already aborted, in which case just return an - // already canceled token. Finally, force a source into existence if we still - // don't have one, and return its token. - var cts = _abortedCts; - return - cts != null ? cts.Token : - (_requestAborted == 1) ? new CancellationToken(true) : - RequestAbortedSource.Token; + + lock (_abortLock) + { + if (_preventRequestAbortedCancellation) + { + return new CancellationToken(false); + } + + if (_connectionAborted) + { + return new CancellationToken(true); + } + + if (_abortedCts == null) + { + _abortedCts = new CancellationTokenSource(); + } + + return _abortedCts.Token; + } } set { @@ -41,28 +54,6 @@ CancellationToken IHttpRequestLifetimeFeature.RequestAborted } } - private CancellationTokenSource RequestAbortedSource - { - get - { - // Get the abort token, lazily-initializing it if necessary. - // Make sure it's canceled if an abort request already came in. - - // EnsureInitialized can return null since _abortedCts is reset to null - // after it's already been initialized to a non-null value. - // If EnsureInitialized does return null, this property was accessed between - // requests so it's safe to return an ephemeral CancellationTokenSource. - var cts = LazyInitializer.EnsureInitialized(ref _abortedCts, () => new CancellationTokenSource()) - ?? new CancellationTokenSource(); - - if (_requestAborted == 1) - { - cts.Cancel(); - } - return cts; - } - } - void IHttpRequestLifetimeFeature.Abort() { Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByApplication)); diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs index 02bd5f8cfddd..c3d4259ea932 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs @@ -186,26 +186,42 @@ private async Task WriteBody(bool flush = false) internal void AbortIO(bool clientDisconnect) { - if (Interlocked.CompareExchange(ref _requestAborted, 1, 0) != 0) - { - return; - } + var shouldScheduleCancellation = false; - if (clientDisconnect) + lock (_abortLock) { - Log.ConnectionDisconnect(_logger, ((IHttpConnectionFeature)this).ConnectionId); + if (_connectionAborted) + { + return; + } + + shouldScheduleCancellation = _abortedCts != null && !_preventRequestAbortedCancellation; + _connectionAborted = true; } _bodyOutput.Dispose(); - var cts = _abortedCts; - if (cts != null) + if (shouldScheduleCancellation) { + // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. ThreadPool.QueueUserWorkItem(t => { try { - cts.Cancel(); + CancellationTokenSource localAbortCts = null; + + lock (_abortLock) + { + if (_abortedCts != null && !_preventRequestAbortedCancellation) + { + localAbortCts = _abortedCts; + _abortedCts = null; + } + } + + // If we cancel the cts, we don't dispose as people may still be using + // the cts. It also isn't necessary to dispose a canceled cts. + localAbortCts?.Cancel(); } catch (Exception ex) { diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs index fd4da572fa29..407f47fc7187 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs @@ -59,7 +59,6 @@ internal abstract partial class IISHttpContext : NativeRequestContext, IThreadPo protected Task _writeBodyTask; private bool _wasUpgraded; - protected int _requestAborted; protected Pipe _bodyInputPipe; protected OutputProducer _bodyOutput; @@ -68,7 +67,6 @@ internal abstract partial class IISHttpContext : NativeRequestContext, IThreadPo private const string NegotiateString = "Negotiate"; private const string BasicString = "Basic"; - internal unsafe IISHttpContext( MemoryPool memoryPool, IntPtr pInProcessHandler, @@ -509,7 +507,17 @@ protected virtual void Dispose(bool disposing) wi.Dispose(); } - _abortedCts?.Dispose(); + // Lock to prevent CancelRequestAbortedToken from attempting to cancel a disposed CTS. + CancellationTokenSource localAbortCts = null; + + lock (_abortLock) + { + _preventRequestAbortedCancellation = false; + localAbortCts = _abortedCts; + _abortedCts = null; + } + + localAbortCts?.Dispose(); disposedValue = true; } diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs index bb104f5ac156..033aba11c140 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs @@ -59,7 +59,7 @@ public override async Task ProcessRequestAsync() } } - if (Volatile.Read(ref _requestAborted) == 0) + if (_connectionAborted) { await ProduceEnd(); } From 2fb0149fdf26b24192cff2f40c7e28d58cbd9262 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 15 Apr 2019 11:29:44 -0700 Subject: [PATCH 02/14] Cleanup --- ...HttpContext.IHttpRequestLifetimeFeature.cs | 15 +++- .../IIS/IIS/src/Core/IISHttpContext.IO.cs | 75 +++++++++++++++- .../IIS/IIS/src/Core/IISHttpContext.cs | 2 +- .../IIS/IIS/src/Core/IISHttpContextOfT.cs | 8 +- src/Servers/IIS/IIS/src/CoreStrings.resx | 6 ++ .../src/Properties/CoreStrings.Designer.cs | 28 ++++++ .../test/IIS.Tests/ClientDisconnectTests.cs | 29 +++++- .../IIS/test/IIS.Tests/ResponseAbortTests.cs | 89 +++++++++++++++++++ 8 files changed, 243 insertions(+), 9 deletions(-) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs index bc1cc4aacd3b..010f571700fd 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs @@ -1,7 +1,6 @@ // 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; using Microsoft.AspNetCore.Connections; using Microsoft.AspNetCore.Http.Features; @@ -58,5 +57,19 @@ void IHttpRequestLifetimeFeature.Abort() { Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByApplication)); } + + // TODO figure out if we want to support this (probably should, but requires counting bytes. + private void PreventRequestAbortedCancellation() + { + lock (_abortLock) + { + if (_connectionAborted) + { + return; + } + + _preventRequestAbortedCancellation = true; + } + } } } diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs index c3d4259ea932..31405a22ccc2 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs @@ -3,15 +3,19 @@ using System; using System.Buffers; +using System.Net.Http; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; namespace Microsoft.AspNetCore.Server.IIS.Core { internal partial class IISHttpContext { + private long _responseBytesWritten; + /// /// Reads data from the Input pipe to the user. /// @@ -61,10 +65,21 @@ internal async Task ReadAsync(Memory memory, CancellationToken cancel async Task WriteFirstAsync() { await InitializeResponse(flushHeaders: false); + CheckLastWrite(); await _bodyOutput.WriteAsync(memory, cancellationToken); } - return !HasResponseStarted ? WriteFirstAsync() : _bodyOutput.WriteAsync(memory, cancellationToken); + VerifyAndUpdateWrite(memory.Length); + + if (!HasResponseStarted) + { + return WriteFirstAsync(); + } + else + { + CheckLastWrite(); + return _bodyOutput.WriteAsync(memory, cancellationToken); + } } /// @@ -140,7 +155,6 @@ private async Task WriteBody(bool flush = false) var result = await _bodyOutput.Reader.ReadAsync(); var buffer = result.Buffer; - try { if (!buffer.IsEmpty) @@ -184,6 +198,58 @@ private async Task WriteBody(bool flush = false) } } + private void VerifyAndUpdateWrite(int count) + { + var responseHeaders = HttpResponseHeaders; + + if (responseHeaders != null && + !ResponseHeaders.TryGetValue("Transfer-Encoding", out _) && + responseHeaders.ContentLength.HasValue && + _responseBytesWritten + count > responseHeaders.ContentLength.Value) + { + throw new InvalidOperationException( + CoreStrings.FormatTooManyBytesWritten(_responseBytesWritten + count, responseHeaders.ContentLength.Value)); + } + + _responseBytesWritten += count; + } + + protected void VerifyResponseContentLength() + { + var responseHeaders = HttpResponseHeaders; + + if (Method != HttpMethod.Head.Method && + StatusCode != StatusCodes.Status304NotModified && + !ResponseHeaders.TryGetValue("Transfer-Encoding", out _) && + responseHeaders.ContentLength.HasValue && + _responseBytesWritten < responseHeaders.ContentLength.Value) + { + // We need to close the connection if any bytes were written since the client + // cannot be certain of how many bytes it will receive. + + ReportApplicationError(new InvalidOperationException( + CoreStrings.FormatTooFewBytesWritten(_responseBytesWritten, responseHeaders.ContentLength.Value))); + } + } + + private void CheckLastWrite() + { + var responseHeaders = HttpResponseHeaders; + + // Prevent firing request aborted token if this is the last write, to avoid + // aborting the request if the app is still running when the client receives + // the final bytes of the response and gracefully closes the connection. + // + // Called after VerifyAndUpdateWrite(), so _responseBytesWritten has already been updated. + if (responseHeaders != null && + !ResponseHeaders.TryGetValue("Transfer-Encoding", out _) && // todo optimize this + responseHeaders.ContentLength.HasValue && + _responseBytesWritten == responseHeaders.ContentLength.Value) + { + PreventRequestAbortedCancellation(); + } + } + internal void AbortIO(bool clientDisconnect) { var shouldScheduleCancellation = false; @@ -199,6 +265,11 @@ internal void AbortIO(bool clientDisconnect) _connectionAborted = true; } + if (clientDisconnect) + { + Log.ConnectionDisconnect(_logger, ((IHttpConnectionFeature)this).ConnectionId); + } + _bodyOutput.Dispose(); if (shouldScheduleCancellation) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs index 407f47fc7187..c1da56bfc7ad 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs @@ -8,6 +8,7 @@ using System.IO; using System.IO.Pipelines; using System.Net; +using System.Net.Http; using System.Runtime.InteropServices; using System.Security.Claims; using System.Security.Principal; @@ -512,7 +513,6 @@ protected virtual void Dispose(bool disposing) lock (_abortLock) { - _preventRequestAbortedCancellation = false; localAbortCts = _abortedCts; _abortedCts = null; } diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs index 033aba11c140..e443168fe310 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs @@ -33,10 +33,10 @@ public override async Task ProcessRequestAsync() context = _application.CreateContext(this); await _application.ProcessRequestAsync(context); // TODO Verification of Response - //if (Volatile.Read(ref _requestAborted) == 0) - //{ - // VerifyResponseContentLength(); - //} + if (!_connectionAborted) + { + VerifyResponseContentLength(); + } } catch (Exception ex) { diff --git a/src/Servers/IIS/IIS/src/CoreStrings.resx b/src/Servers/IIS/IIS/src/CoreStrings.resx index d6c2e7b974f0..20ee92c1ae70 100644 --- a/src/Servers/IIS/IIS/src/CoreStrings.resx +++ b/src/Servers/IIS/IIS/src/CoreStrings.resx @@ -147,4 +147,10 @@ {name} cannot be set because the response has already started. + + Response Content-Length mismatch: too few bytes written ({written} of {expected}). + + + Response Content-Length mismatch: too many bytes written ({written} of {expected}). + diff --git a/src/Servers/IIS/IIS/src/Properties/CoreStrings.Designer.cs b/src/Servers/IIS/IIS/src/Properties/CoreStrings.Designer.cs index 639480f4a5d4..46cc52dafc55 100644 --- a/src/Servers/IIS/IIS/src/Properties/CoreStrings.Designer.cs +++ b/src/Servers/IIS/IIS/src/Properties/CoreStrings.Designer.cs @@ -150,6 +150,34 @@ internal static string ParameterReadOnlyAfterResponseStarted internal static string FormatParameterReadOnlyAfterResponseStarted(object name) => string.Format(CultureInfo.CurrentCulture, GetString("ParameterReadOnlyAfterResponseStarted", "name"), name); + /// + /// Response Content-Length mismatch: too few bytes written ({written} of {expected}). + /// + internal static string TooFewBytesWritten + { + get => GetString("TooFewBytesWritten"); + } + + /// + /// Response Content-Length mismatch: too few bytes written ({written} of {expected}). + /// + internal static string FormatTooFewBytesWritten(object written, object expected) + => string.Format(CultureInfo.CurrentCulture, GetString("TooFewBytesWritten", "written", "expected"), written, expected); + + /// + /// Response Content-Length mismatch: too many bytes written ({written} of {expected}). + /// + internal static string TooManyBytesWritten + { + get => GetString("TooManyBytesWritten"); + } + + /// + /// Response Content-Length mismatch: too many bytes written ({written} of {expected}). + /// + internal static string FormatTooManyBytesWritten(object written, object expected) + => string.Format(CultureInfo.CurrentCulture, GetString("TooManyBytesWritten", "written", "expected"), written, expected); + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs b/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs index 60b172fc217f..6bf7768e832d 100644 --- a/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs +++ b/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs @@ -5,10 +5,12 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Server.IntegrationTesting; using Microsoft.AspNetCore.Testing; using Microsoft.AspNetCore.Testing.xunit; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Testing; using Xunit; namespace Microsoft.AspNetCore.Server.IISIntegration.FunctionalTests @@ -52,7 +54,7 @@ public async Task WritesSucceedAfterClientDisconnect() } [ConditionalFact] - [Flaky("https://github.com/aspnet/AspNetCore-Internal/issues/1831", FlakyOn.All)] + [Repeat(20)] public async Task WritesCancelledWhenUsingAbortedToken() { var requestStartedCompletionSource = CreateTaskCompletionSource(); @@ -174,6 +176,31 @@ public async Task WriterThrowsCancelledException() } } + [ConditionalFact] + public async Task CancellationTokenIsUsableAfterRequestContentLength() + { + using (var testServer = await TestServer.Create(async ctx => + { + var token = ctx.RequestAborted; + var originalRegistration = token.Register(() => { }); + + ctx.Abort(); + + Assert.True(token.WaitHandle.WaitOne(10000)); + Assert.True(ctx.RequestAborted.WaitHandle.WaitOne(10000)); + Assert.Equal(token, originalRegistration.Token); + + await Task.CompletedTask; + }, LoggerFactory)) + { + using (var connection = testServer.CreateConnection()) + { + await SendContentLength1Post(connection); + await connection.WaitForConnectionClose(); + } + } + } + [ConditionalFact] [Flaky("https://github.com/aspnet/AspNetCore-Internal/issues/1831", FlakyOn.All)] public async Task ReaderThrowsCancelledException() diff --git a/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs b/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs index c7ec472586fd..54593a7078d5 100644 --- a/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs +++ b/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs @@ -138,6 +138,95 @@ public async Task RequestAbortedIsTrippedAfterAbort() Assert.True(tokenAborted); } + [ConditionalFact] + public async Task RequestAbortedTokenCannotBeCanceledAfterWritingFinalContentContentLength() + { + var requestCompletedCompletionSource = CreateTaskCompletionSource(); + + using (var testServer = await TestServer.Create(async ctx => + { + ctx.Response.ContentLength = 1; + await ctx.Response.WriteAsync("a"); + + ctx.Abort(); + + var token = ctx.RequestAborted; + + Assert.False(token.IsCancellationRequested); + + requestCompletedCompletionSource.SetResult(true); + }, LoggerFactory)) + { + using (var connection = testServer.CreateConnection()) + { + await SendContentLength1Post(connection); + await requestCompletedCompletionSource.Task.DefaultTimeout(); + await connection.Receive( + "HTTP/1.1 200 OK", + "Content-Length: 1", + "", + "a", + ""); + } + } + } + + [ConditionalFact] + public async Task RequestAbortedTokenIsCanceledAfterAbort() + { + var requestCompletedCompletionSource = CreateTaskCompletionSource(); + + using (var testServer = await TestServer.Create(async ctx => + { + ctx.Abort(); + + var token = ctx.RequestAborted; + + Assert.True(token.IsCancellationRequested); + + await Task.CompletedTask; + }, LoggerFactory)) + { + using (var connection = testServer.CreateConnection()) + { + await SendContentLength1Post(connection); + await connection.WaitForConnectionClose(); + } + } + } + + [ConditionalFact] + public async Task RequestAbortedTokenCannotBeCanceledAfterProduceEnd() + { + var requestCompletedCompletionSource = CreateTaskCompletionSource(); + + using (var testServer = await TestServer.Create(async ctx => + { + ctx.Response.ContentLength = 1; + await ctx.Response.WriteAsync("a"); + + ctx.Abort(); + + var token = ctx.RequestAborted; + + Assert.False(token.IsCancellationRequested); + + requestCompletedCompletionSource.SetResult(true); + }, LoggerFactory)) + { + using (var connection = testServer.CreateConnection()) + { + await SendContentLength1Post(connection); + await requestCompletedCompletionSource.Task.DefaultTimeout(); + await connection.Receive( + "HTTP/1.1 200 OK", + "Content-Length: 1", + "", + "a", + ""); + } + } + } private static async Task SendContentLength1Post(TestConnection connection) { await connection.Send( From fe6617f8b81d4a85595751b75f5fe9021db2b986 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 15 Apr 2019 11:35:42 -0700 Subject: [PATCH 03/14] nits --- .../test/IIS.Tests/ClientDisconnectTests.cs | 25 ------------------- .../IIS/test/IIS.Tests/ResponseAbortTests.cs | 25 +++++++++++++++++++ 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs b/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs index 6bf7768e832d..529195c70232 100644 --- a/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs +++ b/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs @@ -176,31 +176,6 @@ public async Task WriterThrowsCancelledException() } } - [ConditionalFact] - public async Task CancellationTokenIsUsableAfterRequestContentLength() - { - using (var testServer = await TestServer.Create(async ctx => - { - var token = ctx.RequestAborted; - var originalRegistration = token.Register(() => { }); - - ctx.Abort(); - - Assert.True(token.WaitHandle.WaitOne(10000)); - Assert.True(ctx.RequestAborted.WaitHandle.WaitOne(10000)); - Assert.Equal(token, originalRegistration.Token); - - await Task.CompletedTask; - }, LoggerFactory)) - { - using (var connection = testServer.CreateConnection()) - { - await SendContentLength1Post(connection); - await connection.WaitForConnectionClose(); - } - } - } - [ConditionalFact] [Flaky("https://github.com/aspnet/AspNetCore-Internal/issues/1831", FlakyOn.All)] public async Task ReaderThrowsCancelledException() diff --git a/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs b/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs index 54593a7078d5..c583ed74efe7 100644 --- a/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs +++ b/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs @@ -195,6 +195,31 @@ public async Task RequestAbortedTokenIsCanceledAfterAbort() } } + [ConditionalFact] + public async Task CancellationTokenIsUsableAfterRequestContentLength() + { + using (var testServer = await TestServer.Create(async ctx => + { + var token = ctx.RequestAborted; + var originalRegistration = token.Register(() => { }); + + ctx.Abort(); + + Assert.True(token.WaitHandle.WaitOne(10000)); + Assert.True(ctx.RequestAborted.WaitHandle.WaitOne(10000)); + Assert.Equal(token, originalRegistration.Token); + + await Task.CompletedTask; + }, LoggerFactory)) + { + using (var connection = testServer.CreateConnection()) + { + await SendContentLength1Post(connection); + await connection.WaitForConnectionClose(); + } + } + } + [ConditionalFact] public async Task RequestAbortedTokenCannotBeCanceledAfterProduceEnd() { From ded94e5bb8ad62a7f58a761ef118a862ef1fbc05 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 15 Apr 2019 11:40:28 -0700 Subject: [PATCH 04/14] revert --- .../IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj index 9c00e7b073c2..9fd8d246712d 100644 --- a/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj +++ b/src/Servers/IIS/AspNetCoreModuleV2/CommonLib/CommonLib.vcxproj @@ -34,7 +34,6 @@ true v142 Unicode - StaticLibrary @@ -42,14 +41,12 @@ v142 true Unicode - StaticLibrary true v142 Unicode - StaticLibrary @@ -57,7 +54,6 @@ v142 false Unicode - From ae7f17728f9284e1dc5375f8eaa9279f3fab50b6 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 15 Apr 2019 11:51:37 -0700 Subject: [PATCH 05/14] nits --- .../IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs | 3 +-- src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs | 3 ++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs index 010f571700fd..1dda53924122 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs @@ -15,7 +15,7 @@ internal partial class IISHttpContext : IHttpRequestLifetimeFeature private bool _preventRequestAbortedCancellation; protected bool _connectionAborted; - public CancellationToken RequestAborted + CancellationToken IHttpRequestLifetimeFeature.RequestAborted { get { @@ -58,7 +58,6 @@ void IHttpRequestLifetimeFeature.Abort() Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByApplication)); } - // TODO figure out if we want to support this (probably should, but requires counting bytes. private void PreventRequestAbortedCancellation() { lock (_abortLock) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs index e443168fe310..6bad455ce697 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs @@ -31,8 +31,9 @@ public override async Task ProcessRequestAsync() try { context = _application.CreateContext(this); + await _application.ProcessRequestAsync(context); - // TODO Verification of Response + if (!_connectionAborted) { VerifyResponseContentLength(); From a5a0c0614a591c7ad39aea316d928574a1a8b551 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 15 Apr 2019 12:22:18 -0700 Subject: [PATCH 06/14] reduse scope --- ...HttpContext.IHttpRequestLifetimeFeature.cs | 23 +---- .../IIS/IIS/src/Core/IISHttpContext.IO.cs | 28 +----- .../IIS/IIS/src/Core/IISHttpContextOfT.cs | 4 +- .../IIS/test/IIS.Tests/ResponseAbortTests.cs | 91 +------------------ 4 files changed, 9 insertions(+), 137 deletions(-) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs index 1dda53924122..1e8fcd038bee 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs @@ -12,8 +12,7 @@ internal partial class IISHttpContext : IHttpRequestLifetimeFeature private CancellationTokenSource _abortedCts; private CancellationToken? _manuallySetRequestAbortToken; private object _abortLock = new object(); - private bool _preventRequestAbortedCancellation; - protected bool _connectionAborted; + protected bool _requestAborted; CancellationToken IHttpRequestLifetimeFeature.RequestAborted { @@ -27,12 +26,7 @@ CancellationToken IHttpRequestLifetimeFeature.RequestAborted lock (_abortLock) { - if (_preventRequestAbortedCancellation) - { - return new CancellationToken(false); - } - - if (_connectionAborted) + if (_requestAborted) { return new CancellationToken(true); } @@ -57,18 +51,5 @@ void IHttpRequestLifetimeFeature.Abort() { Abort(new ConnectionAbortedException(CoreStrings.ConnectionAbortedByApplication)); } - - private void PreventRequestAbortedCancellation() - { - lock (_abortLock) - { - if (_connectionAborted) - { - return; - } - - _preventRequestAbortedCancellation = true; - } - } } } diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs index 31405a22ccc2..2d7efeaf3b86 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs @@ -65,7 +65,6 @@ internal async Task ReadAsync(Memory memory, CancellationToken cancel async Task WriteFirstAsync() { await InitializeResponse(flushHeaders: false); - CheckLastWrite(); await _bodyOutput.WriteAsync(memory, cancellationToken); } @@ -77,7 +76,6 @@ async Task WriteFirstAsync() } else { - CheckLastWrite(); return _bodyOutput.WriteAsync(memory, cancellationToken); } } @@ -232,37 +230,19 @@ protected void VerifyResponseContentLength() } } - private void CheckLastWrite() - { - var responseHeaders = HttpResponseHeaders; - - // Prevent firing request aborted token if this is the last write, to avoid - // aborting the request if the app is still running when the client receives - // the final bytes of the response and gracefully closes the connection. - // - // Called after VerifyAndUpdateWrite(), so _responseBytesWritten has already been updated. - if (responseHeaders != null && - !ResponseHeaders.TryGetValue("Transfer-Encoding", out _) && // todo optimize this - responseHeaders.ContentLength.HasValue && - _responseBytesWritten == responseHeaders.ContentLength.Value) - { - PreventRequestAbortedCancellation(); - } - } - internal void AbortIO(bool clientDisconnect) { var shouldScheduleCancellation = false; lock (_abortLock) { - if (_connectionAborted) + if (_requestAborted) { return; } - shouldScheduleCancellation = _abortedCts != null && !_preventRequestAbortedCancellation; - _connectionAborted = true; + shouldScheduleCancellation = _abortedCts != null; + _requestAborted = true; } if (clientDisconnect) @@ -283,7 +263,7 @@ internal void AbortIO(bool clientDisconnect) lock (_abortLock) { - if (_abortedCts != null && !_preventRequestAbortedCancellation) + if (_abortedCts != null) { localAbortCts = _abortedCts; _abortedCts = null; diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs index 6bad455ce697..f9533d0069d6 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs @@ -34,7 +34,7 @@ public override async Task ProcessRequestAsync() await _application.ProcessRequestAsync(context); - if (!_connectionAborted) + if (!_requestAborted) { VerifyResponseContentLength(); } @@ -60,7 +60,7 @@ public override async Task ProcessRequestAsync() } } - if (_connectionAborted) + if (_requestAborted) { await ProduceEnd(); } diff --git a/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs b/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs index c583ed74efe7..3cbdc2c219b7 100644 --- a/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs +++ b/src/Servers/IIS/IIS/test/IIS.Tests/ResponseAbortTests.cs @@ -139,64 +139,7 @@ public async Task RequestAbortedIsTrippedAfterAbort() } [ConditionalFact] - public async Task RequestAbortedTokenCannotBeCanceledAfterWritingFinalContentContentLength() - { - var requestCompletedCompletionSource = CreateTaskCompletionSource(); - - using (var testServer = await TestServer.Create(async ctx => - { - ctx.Response.ContentLength = 1; - await ctx.Response.WriteAsync("a"); - - ctx.Abort(); - - var token = ctx.RequestAborted; - - Assert.False(token.IsCancellationRequested); - - requestCompletedCompletionSource.SetResult(true); - }, LoggerFactory)) - { - using (var connection = testServer.CreateConnection()) - { - await SendContentLength1Post(connection); - await requestCompletedCompletionSource.Task.DefaultTimeout(); - await connection.Receive( - "HTTP/1.1 200 OK", - "Content-Length: 1", - "", - "a", - ""); - } - } - } - - [ConditionalFact] - public async Task RequestAbortedTokenIsCanceledAfterAbort() - { - var requestCompletedCompletionSource = CreateTaskCompletionSource(); - - using (var testServer = await TestServer.Create(async ctx => - { - ctx.Abort(); - - var token = ctx.RequestAborted; - - Assert.True(token.IsCancellationRequested); - - await Task.CompletedTask; - }, LoggerFactory)) - { - using (var connection = testServer.CreateConnection()) - { - await SendContentLength1Post(connection); - await connection.WaitForConnectionClose(); - } - } - } - - [ConditionalFact] - public async Task CancellationTokenIsUsableAfterRequestContentLength() + public async Task CancellationTokenIsUsableAfterAbortingRequest() { using (var testServer = await TestServer.Create(async ctx => { @@ -220,38 +163,6 @@ public async Task CancellationTokenIsUsableAfterRequestContentLength() } } - [ConditionalFact] - public async Task RequestAbortedTokenCannotBeCanceledAfterProduceEnd() - { - var requestCompletedCompletionSource = CreateTaskCompletionSource(); - - using (var testServer = await TestServer.Create(async ctx => - { - ctx.Response.ContentLength = 1; - await ctx.Response.WriteAsync("a"); - - ctx.Abort(); - - var token = ctx.RequestAborted; - - Assert.False(token.IsCancellationRequested); - - requestCompletedCompletionSource.SetResult(true); - }, LoggerFactory)) - { - using (var connection = testServer.CreateConnection()) - { - await SendContentLength1Post(connection); - await requestCompletedCompletionSource.Task.DefaultTimeout(); - await connection.Receive( - "HTTP/1.1 200 OK", - "Content-Length: 1", - "", - "a", - ""); - } - } - } private static async Task SendContentLength1Post(TestConnection connection) { await connection.Send( From 89b975ac3c74b69dd1b2046c66a8e7deed9688b8 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 15 Apr 2019 12:26:46 -0700 Subject: [PATCH 07/14] Reduce scope even more --- .../IIS/IIS/src/Core/IISHttpContext.IO.cs | 45 +------------------ .../IIS/IIS/src/Core/IISHttpContextOfT.cs | 5 --- src/Servers/IIS/IIS/src/CoreStrings.resx | 6 --- .../src/Properties/CoreStrings.Designer.cs | 28 ------------ 4 files changed, 1 insertion(+), 83 deletions(-) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs index 2d7efeaf3b86..a116259b77de 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs @@ -68,16 +68,7 @@ async Task WriteFirstAsync() await _bodyOutput.WriteAsync(memory, cancellationToken); } - VerifyAndUpdateWrite(memory.Length); - - if (!HasResponseStarted) - { - return WriteFirstAsync(); - } - else - { - return _bodyOutput.WriteAsync(memory, cancellationToken); - } + return !HasResponseStarted ? WriteFirstAsync() : _bodyOutput.WriteAsync(memory, cancellationToken); } /// @@ -196,40 +187,6 @@ private async Task WriteBody(bool flush = false) } } - private void VerifyAndUpdateWrite(int count) - { - var responseHeaders = HttpResponseHeaders; - - if (responseHeaders != null && - !ResponseHeaders.TryGetValue("Transfer-Encoding", out _) && - responseHeaders.ContentLength.HasValue && - _responseBytesWritten + count > responseHeaders.ContentLength.Value) - { - throw new InvalidOperationException( - CoreStrings.FormatTooManyBytesWritten(_responseBytesWritten + count, responseHeaders.ContentLength.Value)); - } - - _responseBytesWritten += count; - } - - protected void VerifyResponseContentLength() - { - var responseHeaders = HttpResponseHeaders; - - if (Method != HttpMethod.Head.Method && - StatusCode != StatusCodes.Status304NotModified && - !ResponseHeaders.TryGetValue("Transfer-Encoding", out _) && - responseHeaders.ContentLength.HasValue && - _responseBytesWritten < responseHeaders.ContentLength.Value) - { - // We need to close the connection if any bytes were written since the client - // cannot be certain of how many bytes it will receive. - - ReportApplicationError(new InvalidOperationException( - CoreStrings.FormatTooFewBytesWritten(_responseBytesWritten, responseHeaders.ContentLength.Value))); - } - } - internal void AbortIO(bool clientDisconnect) { var shouldScheduleCancellation = false; diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs index f9533d0069d6..fc6b1bdde04c 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs @@ -33,11 +33,6 @@ public override async Task ProcessRequestAsync() context = _application.CreateContext(this); await _application.ProcessRequestAsync(context); - - if (!_requestAborted) - { - VerifyResponseContentLength(); - } } catch (Exception ex) { diff --git a/src/Servers/IIS/IIS/src/CoreStrings.resx b/src/Servers/IIS/IIS/src/CoreStrings.resx index 20ee92c1ae70..d6c2e7b974f0 100644 --- a/src/Servers/IIS/IIS/src/CoreStrings.resx +++ b/src/Servers/IIS/IIS/src/CoreStrings.resx @@ -147,10 +147,4 @@ {name} cannot be set because the response has already started. - - Response Content-Length mismatch: too few bytes written ({written} of {expected}). - - - Response Content-Length mismatch: too many bytes written ({written} of {expected}). - diff --git a/src/Servers/IIS/IIS/src/Properties/CoreStrings.Designer.cs b/src/Servers/IIS/IIS/src/Properties/CoreStrings.Designer.cs index 46cc52dafc55..639480f4a5d4 100644 --- a/src/Servers/IIS/IIS/src/Properties/CoreStrings.Designer.cs +++ b/src/Servers/IIS/IIS/src/Properties/CoreStrings.Designer.cs @@ -150,34 +150,6 @@ internal static string ParameterReadOnlyAfterResponseStarted internal static string FormatParameterReadOnlyAfterResponseStarted(object name) => string.Format(CultureInfo.CurrentCulture, GetString("ParameterReadOnlyAfterResponseStarted", "name"), name); - /// - /// Response Content-Length mismatch: too few bytes written ({written} of {expected}). - /// - internal static string TooFewBytesWritten - { - get => GetString("TooFewBytesWritten"); - } - - /// - /// Response Content-Length mismatch: too few bytes written ({written} of {expected}). - /// - internal static string FormatTooFewBytesWritten(object written, object expected) - => string.Format(CultureInfo.CurrentCulture, GetString("TooFewBytesWritten", "written", "expected"), written, expected); - - /// - /// Response Content-Length mismatch: too many bytes written ({written} of {expected}). - /// - internal static string TooManyBytesWritten - { - get => GetString("TooManyBytesWritten"); - } - - /// - /// Response Content-Length mismatch: too many bytes written ({written} of {expected}). - /// - internal static string FormatTooManyBytesWritten(object written, object expected) - => string.Format(CultureInfo.CurrentCulture, GetString("TooManyBytesWritten", "written", "expected"), written, expected); - private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); From 3255f85a566fc0ecc0561882306f9a2a2145eb06 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 15 Apr 2019 12:59:19 -0700 Subject: [PATCH 08/14] nit --- src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs index a116259b77de..a1c2d712e2d0 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs @@ -14,8 +14,6 @@ namespace Microsoft.AspNetCore.Server.IIS.Core { internal partial class IISHttpContext { - private long _responseBytesWritten; - /// /// Reads data from the Input pipe to the user. /// @@ -212,7 +210,7 @@ internal void AbortIO(bool clientDisconnect) if (shouldScheduleCancellation) { // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. - ThreadPool.QueueUserWorkItem(t => + ThreadPool.UnsafeQueueUserWorkItem(t => { try { @@ -235,7 +233,7 @@ internal void AbortIO(bool clientDisconnect) { Log.ApplicationError(_logger, ((IHttpConnectionFeature)this).ConnectionId, TraceIdentifier, ex); } - }); + }, this, preferLocal: false); } } From 2c622cdea135408678aa0cc472309f1e8e93b4f3 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 15 Apr 2019 13:54:42 -0700 Subject: [PATCH 09/14] fb --- src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs index a1c2d712e2d0..7741c64efc31 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs @@ -210,18 +210,18 @@ internal void AbortIO(bool clientDisconnect) if (shouldScheduleCancellation) { // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. - ThreadPool.UnsafeQueueUserWorkItem(t => + ThreadPool.UnsafeQueueUserWorkItem(ctx => { try { CancellationTokenSource localAbortCts = null; - lock (_abortLock) + lock (ctx._abortLock) { - if (_abortedCts != null) + if (ctx._abortedCts != null) { - localAbortCts = _abortedCts; - _abortedCts = null; + localAbortCts = ctx._abortedCts; + ctx._abortedCts = null; } } From 68e0badf6ef32b0a887bb9844302a4f9aa3e9c8e Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 16 Apr 2019 08:57:59 -0700 Subject: [PATCH 10/14] feedback --- src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs | 9 +++++++-- .../IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs index 7741c64efc31..9b6fecd00035 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs @@ -210,7 +210,13 @@ internal void AbortIO(bool clientDisconnect) if (shouldScheduleCancellation) { // Potentially calling user code. CancelRequestAbortedToken logs any exceptions. - ThreadPool.UnsafeQueueUserWorkItem(ctx => + CancelRequestAbortedToken(); + } + } + + private void CancelRequestAbortedToken() + { + ThreadPool.UnsafeQueueUserWorkItem(ctx => { try { @@ -234,7 +240,6 @@ internal void AbortIO(bool clientDisconnect) Log.ApplicationError(_logger, ((IHttpConnectionFeature)this).ConnectionId, TraceIdentifier, ex); } }, this, preferLocal: false); - } } public void Abort(Exception reason) diff --git a/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs b/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs index 529195c70232..f3eb964ca370 100644 --- a/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs +++ b/src/Servers/IIS/IIS/test/IIS.Tests/ClientDisconnectTests.cs @@ -55,7 +55,7 @@ public async Task WritesSucceedAfterClientDisconnect() [ConditionalFact] [Repeat(20)] - public async Task WritesCancelledWhenUsingAbortedToken() + public async Task WritesCanceledWhenUsingAbortedToken() { var requestStartedCompletionSource = CreateTaskCompletionSource(); var requestCompletedCompletionSource = CreateTaskCompletionSource(); @@ -71,6 +71,7 @@ public async Task WritesCancelledWhenUsingAbortedToken() while (true) { await ctx.Response.Body.WriteAsync(data, ctx.RequestAborted); + await Task.Delay(10); // Small delay to not constantly call WriteAsync. } } catch (Exception e) From 989962006241a90b649cf0d70be9fdc19a4428de Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 16 Apr 2019 10:31:12 -0700 Subject: [PATCH 11/14] Run failed test 100 times --- src/Servers/test/FunctionalTests/NtlmAuthenticationTest.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Servers/test/FunctionalTests/NtlmAuthenticationTest.cs b/src/Servers/test/FunctionalTests/NtlmAuthenticationTest.cs index 254418ca2338..29a371dd069c 100644 --- a/src/Servers/test/FunctionalTests/NtlmAuthenticationTest.cs +++ b/src/Servers/test/FunctionalTests/NtlmAuthenticationTest.cs @@ -21,12 +21,13 @@ public NtlmAuthenticationTests(ITestOutputHelper output) : base(output) } public static TestMatrix TestVariants - => TestMatrix.ForServers(ServerType.IISExpress, ServerType.HttpSys) + => TestMatrix.ForServers(ServerType.IISExpress) .WithTfms(Tfm.NetCoreApp30) - .WithAllHostingModels(); + .WithHostingModels(HostingModel.InProcess); [ConditionalTheory] [MemberData(nameof(TestVariants))] + [Repeat(100)] public async Task NtlmAuthentication(TestVariant variant) { var testName = $"NtlmAuthentication_{variant.Server}_{variant.Tfm}_{variant.Architecture}_{variant.ApplicationType}"; @@ -97,4 +98,4 @@ public async Task NtlmAuthentication(TestVariant variant) } } } -} \ No newline at end of file +} From de57933d974793b64d95c4d5a57e65178f5c54f6 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 16 Apr 2019 11:23:43 -0700 Subject: [PATCH 12/14] Check requestAborted --- src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs index fc6b1bdde04c..8bced196b022 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs @@ -55,7 +55,7 @@ public override async Task ProcessRequestAsync() } } - if (_requestAborted) + if (!_requestAborted) { await ProduceEnd(); } From 78ecd48f749471cacf074cc0c64fa4d8c580a025 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 16 Apr 2019 11:25:31 -0700 Subject: [PATCH 13/14] nit --- src/Servers/test/FunctionalTests/NtlmAuthenticationTest.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Servers/test/FunctionalTests/NtlmAuthenticationTest.cs b/src/Servers/test/FunctionalTests/NtlmAuthenticationTest.cs index 29a371dd069c..254418ca2338 100644 --- a/src/Servers/test/FunctionalTests/NtlmAuthenticationTest.cs +++ b/src/Servers/test/FunctionalTests/NtlmAuthenticationTest.cs @@ -21,13 +21,12 @@ public NtlmAuthenticationTests(ITestOutputHelper output) : base(output) } public static TestMatrix TestVariants - => TestMatrix.ForServers(ServerType.IISExpress) + => TestMatrix.ForServers(ServerType.IISExpress, ServerType.HttpSys) .WithTfms(Tfm.NetCoreApp30) - .WithHostingModels(HostingModel.InProcess); + .WithAllHostingModels(); [ConditionalTheory] [MemberData(nameof(TestVariants))] - [Repeat(100)] public async Task NtlmAuthentication(TestVariant variant) { var testName = $"NtlmAuthentication_{variant.Server}_{variant.Tfm}_{variant.Architecture}_{variant.ApplicationType}"; @@ -98,4 +97,4 @@ public async Task NtlmAuthentication(TestVariant variant) } } } -} +} \ No newline at end of file From 6de58f565f71b1e708a26fc046dc6469ec492157 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 16 Apr 2019 11:48:14 -0700 Subject: [PATCH 14/14] Update IISHttpContext.IHttpRequestLifetimeFeature.cs --- .../IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs index 1e8fcd038bee..55303ab7d573 100644 --- a/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs +++ b/src/Servers/IIS/IIS/src/Core/IISHttpContext.IHttpRequestLifetimeFeature.cs @@ -12,7 +12,7 @@ internal partial class IISHttpContext : IHttpRequestLifetimeFeature private CancellationTokenSource _abortedCts; private CancellationToken? _manuallySetRequestAbortToken; private object _abortLock = new object(); - protected bool _requestAborted; + protected volatile bool _requestAborted; CancellationToken IHttpRequestLifetimeFeature.RequestAborted {