Accurately count only newly examined bytes (#12639)

- Ensure Kestrel count all bytes read using TryRead
- Ensure Kestrel doesn't double count examined but
  not consumed bytes
This commit is contained in:
Stephen Halter 2019-07-29 10:14:14 -07:00 committed by GitHub
parent 4debc9c455
commit 75f4159e5a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 80 additions and 62 deletions

View File

@ -44,9 +44,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
public override void AdvanceTo(SequencePosition consumed, SequencePosition examined)
{
var dataLength = _readResult.Buffer.Slice(_readResult.Buffer.Start, consumed).Length;
OnAdvance(_readResult, consumed, examined);
_requestBodyPipe.Reader.AdvanceTo(consumed, examined);
OnDataRead(dataLength);
}
public override bool TryRead(out ReadResult readResult)
@ -63,6 +62,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
var boolResult = _requestBodyPipe.Reader.TryRead(out _readResult);
readResult = _readResult;
CountBytesRead(readResult.Buffer.Length);
if (_readResult.IsCompleted)
{

View File

@ -17,7 +17,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
private bool _readCompleted;
private bool _isReading;
private int _userCanceled;
private long _totalExaminedInPreviousReadResult;
private bool _finalAdvanceCalled;
public Http1ContentLengthMessageBody(bool keepAlive, long contentLength, Http1Connection context)
@ -85,6 +84,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
if (Interlocked.Exchange(ref _userCanceled, 0) == 1)
{
// Ignore the readResult if it wasn't by the user.
CreateReadResultFromConnectionReadResult();
break;
}
else
@ -156,6 +157,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
CreateReadResultFromConnectionReadResult();
readResult = _readResult;
CountBytesRead(readResult.Buffer.Length);
return true;
}
@ -174,11 +176,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
private void CreateReadResultFromConnectionReadResult()
{
if (_readResult.Buffer.Length >= _inputLength + _totalExaminedInPreviousReadResult)
if (_readResult.Buffer.Length >= _inputLength + _examinedUnconsumedBytes)
{
_readCompleted = true;
_readResult = new ReadResult(
_readResult.Buffer.Slice(0, _inputLength + _totalExaminedInPreviousReadResult),
_readResult.Buffer.Slice(0, _inputLength + _examinedUnconsumedBytes),
_readResult.IsCanceled && Interlocked.Exchange(ref _userCanceled, 0) == 1,
_readCompleted);
}
@ -217,18 +219,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
return;
}
var consumedLength = _readResult.Buffer.Slice(_readResult.Buffer.Start, consumed).Length;
var examinedLength = consumedLength + _readResult.Buffer.Slice(consumed, examined).Length;
_inputLength -= OnAdvance(_readResult, consumed, examined);
_context.Input.AdvanceTo(consumed, examined);
var newlyExamined = examinedLength - _totalExaminedInPreviousReadResult;
OnDataRead(newlyExamined);
_totalExaminedInPreviousReadResult += newlyExamined;
_inputLength -= newlyExamined;
_totalExaminedInPreviousReadResult -= consumedLength;
}
protected override void OnReadStarting()

View File

@ -23,6 +23,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
protected bool _timingEnabled;
protected bool _backpressure;
protected long _alreadyTimedBytes;
protected long _examinedUnconsumedBytes;
protected MessageBody(HttpProtocol context)
{
@ -165,10 +166,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
return readAwaitable;
}
protected void StopTimingRead(long bytesRead)
protected void CountBytesRead(long bytesInReadResult)
{
_context.TimeoutControl.BytesRead(bytesRead - _alreadyTimedBytes);
_alreadyTimedBytes = 0;
var numFirstSeenBytes = bytesInReadResult - _alreadyTimedBytes;
if (numFirstSeenBytes > 0)
{
_context.TimeoutControl.BytesRead(numFirstSeenBytes);
}
}
protected void StopTimingRead(long bytesInReadResult)
{
CountBytesRead(bytesInReadResult);
if (_backpressure)
{
@ -176,5 +186,62 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
_context.TimeoutControl.StopTimingRead();
}
}
protected long OnAdvance(ReadResult readResult, SequencePosition consumed, SequencePosition examined)
{
// This code path is fairly hard to understand so let's break it down with an example
// ReadAsync returns a ReadResult of length 50.
// Advance(25, 40). The examined length would be 40 and consumed length would be 25.
// _totalExaminedInPreviousReadResult starts at 0. newlyExamined is 40.
// OnDataRead is called with length 40.
// _totalExaminedInPreviousReadResult is now 40 - 25 = 15.
// The next call to ReadAsync returns 50 again
// Advance(5, 5) is called
// newlyExamined is 5 - 15, or -10.
// Update _totalExaminedInPreviousReadResult to 10 as we consumed 5.
// The next call to ReadAsync returns 50 again
// _totalExaminedInPreviousReadResult is 10
// Advance(50, 50) is called
// newlyExamined = 50 - 10 = 40
// _totalExaminedInPreviousReadResult is now 50
// _totalExaminedInPreviousReadResult is finally 0 after subtracting consumedLength.
long examinedLength, consumedLength, totalLength;
if (consumed.Equals(examined))
{
examinedLength = readResult.Buffer.Slice(readResult.Buffer.Start, examined).Length;
consumedLength = examinedLength;
}
else
{
consumedLength = readResult.Buffer.Slice(readResult.Buffer.Start, consumed).Length;
examinedLength = consumedLength + readResult.Buffer.Slice(consumed, examined).Length;
}
if (examined.Equals(readResult.Buffer.End))
{
totalLength = examinedLength;
}
else
{
totalLength = readResult.Buffer.Length;
}
var newlyExamined = examinedLength - _examinedUnconsumedBytes;
if (newlyExamined > 0)
{
OnDataRead(newlyExamined);
_examinedUnconsumedBytes += newlyExamined;
}
_examinedUnconsumedBytes -= consumedLength;
_alreadyTimedBytes = totalLength - consumedLength;
return newlyExamined;
}
}
}

View File

@ -14,7 +14,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
{
private readonly Http2Stream _context;
private ReadResult _readResult;
private long _alreadyExaminedInNextReadResult;
private Http2MessageBody(Http2Stream context)
: base(context)
@ -64,55 +63,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
public override void AdvanceTo(SequencePosition consumed, SequencePosition examined)
{
// This code path is fairly hard to understand so let's break it down with an example
// ReadAsync returns a ReadResult of length 50.
// Advance(25, 40). The examined length would be 40 and consumed length would be 25.
// _totalExaminedInPreviousReadResult starts at 0. newlyExamined is 40.
// OnDataRead is called with length 40.
// _totalExaminedInPreviousReadResult is now 40 - 25 = 15.
// The next call to ReadAsync returns 50 again
// Advance(5, 5) is called
// newlyExamined is 5 - 15, or -10.
// Update _totalExaminedInPreviousReadResult to 10 as we consumed 5.
// The next call to ReadAsync returns 50 again
// _totalExaminedInPreviousReadResult is 10
// Advance(50, 50) is called
// newlyExamined = 50 - 10 = 40
// _totalExaminedInPreviousReadResult is now 50
// _totalExaminedInPreviousReadResult is finally 0 after subtracting consumedLength.
long examinedLength;
long consumedLength;
if (consumed.Equals(examined))
{
examinedLength = _readResult.Buffer.Slice(_readResult.Buffer.Start, examined).Length;
consumedLength = examinedLength;
}
else
{
consumedLength = _readResult.Buffer.Slice(_readResult.Buffer.Start, consumed).Length;
examinedLength = consumedLength + _readResult.Buffer.Slice(consumed, examined).Length;
}
OnAdvance(_readResult, consumed, examined);
_context.RequestBodyPipe.Reader.AdvanceTo(consumed, examined);
var newlyExamined = examinedLength - _alreadyExaminedInNextReadResult;
if (newlyExamined > 0)
{
OnDataRead(newlyExamined);
_alreadyExaminedInNextReadResult += newlyExamined;
}
_alreadyExaminedInNextReadResult -= consumedLength;
}
public override bool TryRead(out ReadResult readResult)
{
var result = _context.RequestBodyPipe.Reader.TryRead(out readResult);
_readResult = readResult;
CountBytesRead(readResult.Buffer.Length);
return result;
}