From 5156206f1a413d5af396f026b4ab8edd0cabcf0d Mon Sep 17 00:00:00 2001 From: Will Godbe Date: Fri, 9 Apr 2021 13:03:12 -0700 Subject: [PATCH 01/12] Stop doing sync over async in Produce100Continue --- .../Http/Http1ChunkedEncodingMessageBody.cs | 6 ++++- .../Core/src/Internal/Http/HttpProtocol.cs | 16 ++++-------- .../src/Internal/Http/IHttpResponseControl.cs | 2 +- .../Core/src/Internal/Http/MessageBody.cs | 25 ++++++++++++++++--- .../src/Internal/Http2/Http2MessageBody.cs | 11 +++++++- 5 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs index 8747eda3d4fc..768fcf43e3ab 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs @@ -101,7 +101,11 @@ private async Task PumpAsync() if (!awaitable.IsCompleted) { - TryProduceContinue(); + ValueTask continueTask = TryProduceContinue(); + if (!continueTask.IsCompleted) + { + await continueTask; + } } while (true) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 7b6c3b025789..8b15e4f06300 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -909,27 +909,21 @@ protected bool VerifyResponseContentLength([NotNullWhen(false)] out Exception? e return true; } - public void ProduceContinue() + public ValueTask ProduceContinue() { if (HasResponseStarted) { - return; + return default; } if (_httpVersion != Http.HttpVersion.Http10 && ((IHeaderDictionary)HttpRequestHeaders).TryGetValue(HeaderNames.Expect, out var expect) && (expect.FirstOrDefault() ?? "").Equals("100-continue", StringComparison.OrdinalIgnoreCase)) { - ValueTask vt = Output.Write100ContinueAsync(); - if (vt.IsCompleted) - { - vt.GetAwaiter().GetResult(); - } - else - { - vt.AsTask().GetAwaiter().GetResult(); - } + return Output.Write100ContinueAsync(); } + + return default; } public Task InitializeResponseAsync(int firstWriteByteCount) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs index 4bd37e02f88c..5a68c808e687 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { internal interface IHttpResponseControl { - void ProduceContinue(); + ValueTask ProduceContinue(); Memory GetMemory(int sizeHint = 0); Span GetSpan(int sizeHint = 0); void Advance(int bytes); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs index fd1e1376c09e..4988b1738749 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs @@ -93,13 +93,15 @@ public virtual void Reset() _examinedUnconsumedBytes = 0; } - protected void TryProduceContinue() + protected ValueTask TryProduceContinue() { if (_send100Continue) { - _context.HttpResponseControl.ProduceContinue(); _send100Continue = false; + return _context.HttpResponseControl.ProduceContinue(); } + + return default; } protected void TryStart() @@ -183,7 +185,11 @@ protected ValueTask StartTimingReadAsync(ValueTask readA { if (!readAwaitable.IsCompleted) { - TryProduceContinue(); + ValueTask continueTask = TryProduceContinue(); + if (!continueTask.IsCompleted) + { + return StartTimingReadAwaited(continueTask, readAwaitable, cancellationToken); + } if (_timingEnabled) { @@ -195,6 +201,19 @@ protected ValueTask StartTimingReadAsync(ValueTask readA return readAwaitable; } + protected async ValueTask StartTimingReadAwaited(ValueTask continueTask, ValueTask readAwaitable, CancellationToken cancellationToken) + { + await continueTask; + + if (_timingEnabled) + { + _backpressure = true; + _context.TimeoutControl.StartTimingRead(); + } + + return await readAwaitable; + } + protected void CountBytesRead(long bytesInReadResult) { var numFirstSeenBytes = bytesInReadResult - _alreadyTimedBytes; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs index b89b89350451..7fe207bde8da 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs @@ -35,10 +35,19 @@ protected override void OnReadStarted() // Produce 100-continue if no request body data for the stream has arrived yet. if (!_context.RequestBodyStarted) { - TryProduceContinue(); + ValueTask continueTask = TryProduceContinue(); + if (!continueTask.IsCompleted) + { + OnReadStartedAwaited(continueTask); + } } } + private async void OnReadStartedAwaited(ValueTask continueTask) + { + await continueTask; + } + public override void Reset() { base.Reset(); From 5c25fa2472b34e976d32b6592cd664d1d3279f8d Mon Sep 17 00:00:00 2001 From: Will Godbe Date: Fri, 9 Apr 2021 14:40:21 -0700 Subject: [PATCH 02/12] Address feedback --- .../Http/Http1ChunkedEncodingMessageBody.cs | 16 ++++---- .../Http/Http1ContentLengthMessageBody.cs | 4 +- .../Core/src/Internal/Http/HttpProtocol.cs | 2 +- .../src/Internal/Http/IHttpResponseControl.cs | 2 +- .../Core/src/Internal/Http/MessageBody.cs | 40 ++++++++++++++----- .../src/Internal/Http2/Http2MessageBody.cs | 14 ++++--- .../src/Internal/Http3/Http3MessageBody.cs | 4 +- 7 files changed, 52 insertions(+), 30 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs index 768fcf43e3ab..cd739b616f59 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs @@ -44,7 +44,7 @@ public override void AdvanceTo(SequencePosition consumed, SequencePosition exami public override bool TryReadInternal(out ReadResult readResult) { - TryStart(); + TryStartAsync(); var boolResult = _requestBodyPipe.Reader.TryRead(out _readResult); @@ -61,7 +61,7 @@ public override bool TryReadInternal(out ReadResult readResult) public override async ValueTask ReadAsyncInternal(CancellationToken cancellationToken = default) { - TryStart(); + await TryStartAsync(); try { @@ -101,11 +101,8 @@ private async Task PumpAsync() if (!awaitable.IsCompleted) { - ValueTask continueTask = TryProduceContinue(); - if (!continueTask.IsCompleted) - { - await continueTask; - } + ValueTask continueTask = TryProduceContinueAsync(); + await continueTask; } while (true) @@ -175,7 +172,7 @@ protected override ValueTask OnStopAsync() // call complete here on the reader _requestBodyPipe.Reader.Complete(); - Debug.Assert(_pumpTask != null, "OnReadStarted must have been called."); + Debug.Assert(_pumpTask != null, "OnReadStartedAsync must have been called."); // PumpTask catches all Exceptions internally. if (_pumpTask.IsCompleted) @@ -199,9 +196,10 @@ private async ValueTask StopAsyncAwaited(Task pumpTask) _requestBodyPipe.Reset(); } - protected override void OnReadStarted() + protected override Task OnReadStartedAsync() { _pumpTask = PumpAsync(); + return Task.CompletedTask; } private bool Read(ReadOnlySequence readableBuffer, PipeWriter writableBuffer, out SequencePosition consumed, out SequencePosition examined) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs index dd9fbfe6675d..4b92e6f2cc2e 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs @@ -45,7 +45,7 @@ public override async ValueTask ReadAsyncInternal(CancellationToken KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTimeout); } - TryStart(); + await TryStartAsync(); // The while(true) loop is required because the Http1 connection calls CancelPendingRead to unblock // the call to StartTimingReadAsync to check if the request timed out. @@ -132,7 +132,7 @@ public override bool TryReadInternal(out ReadResult readResult) KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTimeout); } - TryStart(); + TryStartAsync(); // The while(true) because we don't want to return a canceled ReadResult if the user themselves didn't cancel it. while (true) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 8b15e4f06300..a45a972fb139 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -909,7 +909,7 @@ protected bool VerifyResponseContentLength([NotNullWhen(false)] out Exception? e return true; } - public ValueTask ProduceContinue() + public ValueTask ProduceContinueAsync() { if (HasResponseStarted) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs index 5a68c808e687..232726bfed02 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { internal interface IHttpResponseControl { - ValueTask ProduceContinue(); + ValueTask ProduceContinueAsync(); Memory GetMemory(int sizeHint = 0); Span GetSpan(int sizeHint = 0); void Advance(int bytes); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs index 4988b1738749..800234fd8107 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs @@ -66,11 +66,21 @@ public virtual ValueTask CompleteAsync(Exception? exception) public virtual Task ConsumeAsync() { - TryStart(); + Task startTask = TryStartAsync(); + if (!startTask.IsCompletedSuccessfully) + { + return ConsumeAwaited(startTask); + } return OnConsumeAsync(); } + private async Task ConsumeAwaited(Task startTask) + { + await startTask; + await OnConsumeAsync(); + } + public virtual ValueTask StopAsync() { TryStop(); @@ -93,22 +103,22 @@ public virtual void Reset() _examinedUnconsumedBytes = 0; } - protected ValueTask TryProduceContinue() + protected ValueTask TryProduceContinueAsync() { if (_send100Continue) { _send100Continue = false; - return _context.HttpResponseControl.ProduceContinue(); + return _context.HttpResponseControl.ProduceContinueAsync(); } return default; } - protected void TryStart() + protected Task TryStartAsync() { if (_context.HasStartedConsumingRequestBody) { - return; + return Task.CompletedTask; } OnReadStarting(); @@ -130,7 +140,18 @@ protected void TryStart() } } - OnReadStarted(); + Task readTask = OnReadStartedAsync(); + if (!readTask.IsCompletedSuccessfully) + { + return TryStartAwaited(readTask); + } + + return Task.CompletedTask; + } + + private async Task TryStartAwaited(Task readTask) + { + await readTask; } protected void TryStop() @@ -167,8 +188,9 @@ protected virtual void OnReadStarting() { } - protected virtual void OnReadStarted() + protected virtual Task OnReadStartedAsync() { + return Task.CompletedTask; } protected void AddAndCheckObservedBytes(long observedBytes) @@ -185,8 +207,8 @@ protected ValueTask StartTimingReadAsync(ValueTask readA { if (!readAwaitable.IsCompleted) { - ValueTask continueTask = TryProduceContinue(); - if (!continueTask.IsCompleted) + ValueTask continueTask = TryProduceContinueAsync(); + if (!continueTask.IsCompletedSuccessfully) { return StartTimingReadAwaited(continueTask, readAwaitable, cancellationToken); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs index 7fe207bde8da..638e927e531a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs @@ -30,20 +30,22 @@ protected override void OnReadStarting() } } - protected override void OnReadStarted() + protected override Task OnReadStartedAsync() { // Produce 100-continue if no request body data for the stream has arrived yet. if (!_context.RequestBodyStarted) { - ValueTask continueTask = TryProduceContinue(); + ValueTask continueTask = TryProduceContinueAsync(); if (!continueTask.IsCompleted) { - OnReadStartedAwaited(continueTask); + return OnReadStartedAwaited(continueTask); } } + + return Task.CompletedTask; } - private async void OnReadStartedAwaited(ValueTask continueTask) + private async Task OnReadStartedAwaited(ValueTask continueTask) { await continueTask; } @@ -68,7 +70,7 @@ public override void AdvanceTo(SequencePosition consumed, SequencePosition exami public override bool TryRead(out ReadResult readResult) { - TryStart(); + TryStartAsync(); var hasResult = _context.RequestBodyPipe.Reader.TryRead(out readResult); @@ -89,7 +91,7 @@ public override bool TryRead(out ReadResult readResult) public override async ValueTask ReadAsync(CancellationToken cancellationToken = default) { - TryStart(); + await TryStartAsync(); try { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3MessageBody.cs index 2210d4b0799d..c710593880a2 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http3/Http3MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http3/Http3MessageBody.cs @@ -46,7 +46,7 @@ public override void AdvanceTo(SequencePosition consumed, SequencePosition exami public override bool TryRead(out ReadResult readResult) { - TryStart(); + TryStartAsync(); var hasResult = _context.RequestBodyPipe.Reader.TryRead(out readResult); @@ -67,7 +67,7 @@ public override bool TryRead(out ReadResult readResult) public override async ValueTask ReadAsync(CancellationToken cancellationToken = default) { - TryStart(); + await TryStartAsync(); try { From 0503f7520b14402a19ba34afc4e8aa524989d9ea Mon Sep 17 00:00:00 2001 From: Will Godbe Date: Fri, 9 Apr 2021 14:42:51 -0700 Subject: [PATCH 03/12] IsCompleted -> IsCompletedSuccessfully --- src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs index 638e927e531a..aed7de74244c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs @@ -36,7 +36,7 @@ protected override Task OnReadStartedAsync() if (!_context.RequestBodyStarted) { ValueTask continueTask = TryProduceContinueAsync(); - if (!continueTask.IsCompleted) + if (!continueTask.IsCompletedSuccessfully) { return OnReadStartedAwaited(continueTask); } From 8d8a1464f2848e01caf5c10611135a3f50df7c4e Mon Sep 17 00:00:00 2001 From: Will Godbe Date: Fri, 9 Apr 2021 15:38:27 -0700 Subject: [PATCH 04/12] Remove unneeded awaited methods --- src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs | 7 +------ .../Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs | 7 +------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs index 800234fd8107..ee64ae2e2f07 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs @@ -143,17 +143,12 @@ protected Task TryStartAsync() Task readTask = OnReadStartedAsync(); if (!readTask.IsCompletedSuccessfully) { - return TryStartAwaited(readTask); + return readTask; } return Task.CompletedTask; } - private async Task TryStartAwaited(Task readTask) - { - await readTask; - } - protected void TryStop() { if (_stopped) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs index aed7de74244c..bd15c7e99af1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs @@ -38,18 +38,13 @@ protected override Task OnReadStartedAsync() ValueTask continueTask = TryProduceContinueAsync(); if (!continueTask.IsCompletedSuccessfully) { - return OnReadStartedAwaited(continueTask); + return continueTask.AsTask(); } } return Task.CompletedTask; } - private async Task OnReadStartedAwaited(ValueTask continueTask) - { - await continueTask; - } - public override void Reset() { base.Reset(); From 70e8ee1a5fe32cbb628bd809e0f74c55fefdf6f3 Mon Sep 17 00:00:00 2001 From: Will Godbe Date: Mon, 12 Apr 2021 09:27:54 -0700 Subject: [PATCH 05/12] Feedback --- .../Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs | 3 +-- .../Core/src/Internal/Http/Http1ContentLengthMessageBody.cs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs index cd739b616f59..433c783bb900 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs @@ -101,8 +101,7 @@ private async Task PumpAsync() if (!awaitable.IsCompleted) { - ValueTask continueTask = TryProduceContinueAsync(); - await continueTask; + await TryProduceContinueAsync(); } while (true) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs index 4b92e6f2cc2e..c771f07a47a6 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs @@ -132,7 +132,7 @@ public override bool TryReadInternal(out ReadResult readResult) KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTimeout); } - TryStartAsync(); + Debug.Assert(TryStartAsync().IsCompleted); // The while(true) because we don't want to return a canceled ReadResult if the user themselves didn't cancel it. while (true) From a10c9d999e6d687d85c3d057f6fee35e8fef03c1 Mon Sep 17 00:00:00 2001 From: Will Godbe Date: Mon, 12 Apr 2021 09:29:24 -0700 Subject: [PATCH 06/12] Brain fart --- .../Core/src/Internal/Http/Http1ContentLengthMessageBody.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs index c771f07a47a6..5e85920f800b 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs @@ -132,7 +132,8 @@ public override bool TryReadInternal(out ReadResult readResult) KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTimeout); } - Debug.Assert(TryStartAsync().IsCompleted); + Task startTask = TryStartAsync(); + Debug.Assert(startTask.IsCompleted); // The while(true) because we don't want to return a canceled ReadResult if the user themselves didn't cancel it. while (true) From f91aceae26ccfdadb9f19f2fb840bc389b5d228d Mon Sep 17 00:00:00 2001 From: William Godbe Date: Mon, 12 Apr 2021 14:41:44 -0700 Subject: [PATCH 07/12] Update Http1ChunkedEncodingMessageBody.cs --- .../Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs index 433c783bb900..c1bf37bfaef2 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs @@ -44,7 +44,8 @@ public override void AdvanceTo(SequencePosition consumed, SequencePosition exami public override bool TryReadInternal(out ReadResult readResult) { - TryStartAsync(); + Task startTask = TryStartAsync(); + Debug.Assert(startTask.IsCompleted); var boolResult = _requestBodyPipe.Reader.TryRead(out _readResult); From b93be2813c76b7fd3896e22a475be3a4515ac38d Mon Sep 17 00:00:00 2001 From: William Godbe Date: Mon, 12 Apr 2021 14:42:04 -0700 Subject: [PATCH 08/12] Update Http2MessageBody.cs --- .../Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs index bd15c7e99af1..7c39503b0fd5 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs @@ -65,7 +65,8 @@ public override void AdvanceTo(SequencePosition consumed, SequencePosition exami public override bool TryRead(out ReadResult readResult) { - TryStartAsync(); + Task startTask = TryStartAsync(); + Debug.Assert(startTask.IsCompleted); var hasResult = _context.RequestBodyPipe.Reader.TryRead(out readResult); From a99aea8331456959bdfdd09e72f4caf373d2f885 Mon Sep 17 00:00:00 2001 From: William Godbe Date: Mon, 12 Apr 2021 15:07:48 -0700 Subject: [PATCH 09/12] Update Http2MessageBody.cs --- src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs index 7c39503b0fd5..f10cd6f9e4c3 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; using System.IO.Pipelines; using System.Threading; using System.Threading.Tasks; From b147ac728228227a9f989e5e58375d964ed91015 Mon Sep 17 00:00:00 2001 From: William Godbe Date: Mon, 12 Apr 2021 16:29:25 -0700 Subject: [PATCH 10/12] Update Http1ContentLengthMessageBody.cs --- .../Core/src/Internal/Http/Http1ContentLengthMessageBody.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs index 5e85920f800b..4b92e6f2cc2e 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ContentLengthMessageBody.cs @@ -132,8 +132,7 @@ public override bool TryReadInternal(out ReadResult readResult) KestrelBadHttpRequestException.Throw(RequestRejectionReason.RequestBodyTimeout); } - Task startTask = TryStartAsync(); - Debug.Assert(startTask.IsCompleted); + TryStartAsync(); // The while(true) because we don't want to return a canceled ReadResult if the user themselves didn't cancel it. while (true) From a9a561943dec871701426b40b572123d06a3cfdb Mon Sep 17 00:00:00 2001 From: Will Godbe Date: Tue, 13 Apr 2021 09:59:59 -0700 Subject: [PATCH 11/12] Respond to feedback --- .../Internal/Http/Http1ChunkedEncodingMessageBody.cs | 3 +-- .../Kestrel/Core/src/Internal/Http/MessageBody.cs | 12 +++++------- .../Core/src/Internal/Http2/Http2MessageBody.cs | 6 +++--- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs index c1bf37bfaef2..433c783bb900 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs @@ -44,8 +44,7 @@ public override void AdvanceTo(SequencePosition consumed, SequencePosition exami public override bool TryReadInternal(out ReadResult readResult) { - Task startTask = TryStartAsync(); - Debug.Assert(startTask.IsCompleted); + TryStartAsync(); var boolResult = _requestBodyPipe.Reader.TryRead(out _readResult); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs index ee64ae2e2f07..06668bd872dc 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs @@ -140,13 +140,7 @@ protected Task TryStartAsync() } } - Task readTask = OnReadStartedAsync(); - if (!readTask.IsCompletedSuccessfully) - { - return readTask; - } - - return Task.CompletedTask; + return OnReadStartedAsync(); } protected void TryStop() @@ -207,6 +201,10 @@ protected ValueTask StartTimingReadAsync(ValueTask readA { return StartTimingReadAwaited(continueTask, readAwaitable, cancellationToken); } + else + { + continueTask.GetAwaiter().GetResult(); + } if (_timingEnabled) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs index f10cd6f9e4c3..59af4bed2f92 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2MessageBody.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Internal; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 @@ -39,7 +40,7 @@ protected override Task OnReadStartedAsync() ValueTask continueTask = TryProduceContinueAsync(); if (!continueTask.IsCompletedSuccessfully) { - return continueTask.AsTask(); + return continueTask.GetAsTask(); } } @@ -66,8 +67,7 @@ public override void AdvanceTo(SequencePosition consumed, SequencePosition exami public override bool TryRead(out ReadResult readResult) { - Task startTask = TryStartAsync(); - Debug.Assert(startTask.IsCompleted); + TryStartAsync(); var hasResult = _context.RequestBodyPipe.Reader.TryRead(out readResult); From 7893fbd7a1d58b27f40c0eec37a1c6522c327b2a Mon Sep 17 00:00:00 2001 From: Will Godbe Date: Tue, 13 Apr 2021 12:56:50 -0700 Subject: [PATCH 12/12] Quarantine failling test --- .../test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs index aa05268499f2..a00408708b3b 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http3/Http3StreamTests.cs @@ -628,6 +628,7 @@ public async Task RemoveConnectionSpecificHeaders() } [Fact] + [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/31777")] public async Task ContentLength_Received_NoDataFrames_Reset() { var headers = new[]