Skip to content

Bugfix/27769 fix invalid output of readonlysequence tryget #89662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Bugfix/27769 fix invalid output of readonlysequence tryget #89662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ff8699f
fix(#27769): Fix invalid output of "ReadOnlySequence<T>.TryGet(ref po…
Maximys Jul 28, 2023
0040924
fix(#27769): Fix regression of "System.Text.Json".
Maximys Jul 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
237 changes: 187 additions & 50 deletions src/libraries/System.Memory/src/System/Buffers/ReadOnlySequence.Helpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,70 +11,71 @@ namespace System.Buffers
public readonly partial struct ReadOnlySequence<T>
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal bool TryGetBuffer(in SequencePosition position, out ReadOnlyMemory<T> memory, out SequencePosition next)
internal MoveResult MoveToNextItem(
ref SequencePosition position,
bool advance)
{
object? positionObject = position.GetObject();
next = default;
object? positionObject;
ReadOnlyMemory<T> item;
SequencePosition positionOfResult;
MoveStatus status;
MoveResult returnValue;

positionObject = position.GetObject();

if (positionObject == null)
{
memory = default;
return false;
}

SequenceType type = GetSequenceType();
object? endObject = _endObject;
int startIndex = position.GetInteger();
int endIndex = GetIndex(_endInteger);

if (type == SequenceType.MultiSegment)
{
Debug.Assert(positionObject is ReadOnlySequenceSegment<T>);

ReadOnlySequenceSegment<T> startSegment = (ReadOnlySequenceSegment<T>)positionObject;

if (startSegment != endObject)
{
ReadOnlySequenceSegment<T>? nextSegment = startSegment.Next;

if (nextSegment == null)
ThrowHelper.ThrowInvalidOperationException_EndPositionNotReached();

next = new SequencePosition(nextSegment, 0);
memory = startSegment.Memory.Slice(startIndex);
}
else
{
memory = startSegment.Memory.Slice(startIndex, endIndex - startIndex);
}
item = default;
positionOfResult = default;
status = MoveStatus.NotExist;
}
else
{
if (positionObject != endObject)
ThrowHelper.ThrowInvalidOperationException_EndPositionNotReached();
SequenceType type = GetSequenceType();
object? endObject = _endObject;

if (type == SequenceType.Array)
switch (type)
{
Debug.Assert(positionObject is T[]);

memory = new ReadOnlyMemory<T>((T[])positionObject, startIndex, endIndex - startIndex);
case SequenceType.Array:
{
MoveToNextItemForArray(in position, positionObject, out item);
positionOfResult = default;
break;
}
case SequenceType.MemoryManager:
{
MoveToNextItemForMemoryManager(in position, positionObject, out item);
positionOfResult = default;
break;
}
case SequenceType.MultiSegment:
{
MoveToNextItemForMultiSegment(in position, endObject, positionObject, out item, out positionOfResult);
break;
}
case SequenceType.String
when typeof(T) == typeof(char):
{
MoveToNextItemForString(in position, positionObject, out item);
positionOfResult = default;
break;
}
default:
{
throw new NotImplementedException();
}
}
else if (typeof(T) == typeof(char) && type == SequenceType.String)
{
Debug.Assert(positionObject is string);

memory = (ReadOnlyMemory<T>)(object)((string)positionObject).AsMemory(startIndex, endIndex - startIndex);
}
else // type == SequenceType.MemoryManager
{
Debug.Assert(type == SequenceType.MemoryManager);
Debug.Assert(positionObject is MemoryManager<T>);
status = item.IsEmpty ? MoveStatus.Empty : MoveStatus.Success;
}

memory = ((MemoryManager<T>)positionObject).Memory.Slice(startIndex, endIndex - startIndex);
}
if (advance)
{
position = positionOfResult;
}

return true;
returnValue = new MoveResult(item, positionOfResult, status);
return returnValue;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand Down Expand Up @@ -694,5 +695,141 @@ private static ReadOnlySpan<T> GetFirstSpanSlow(object startObject, int startInd
return ((MemoryManager<T>)startObject).Memory.Span.Slice(startIndex, endIndex - startIndex);
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void MoveToNextItemForArray(
in SequencePosition position,
object? positionObject,
out ReadOnlyMemory<T> nextItem)
{
if (positionObject != _endObject)
{
ThrowHelper.ThrowInvalidOperationException_EndPositionNotReached();
}

int startIndex = position.GetInteger();
int endIndex = GetIndex(_endInteger);

Debug.Assert(positionObject is T[]);
nextItem = new ReadOnlyMemory<T>((T[])positionObject, startIndex, endIndex - startIndex);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void MoveToNextItemForMemoryManager(
in SequencePosition position,
object? positionObject,
out ReadOnlyMemory<T> nextItem)
{
if (positionObject != _endObject)
{
ThrowHelper.ThrowInvalidOperationException_EndPositionNotReached();
}

int startIndex = position.GetInteger();
int endIndex = GetIndex(_endInteger);

Debug.Assert(positionObject is MemoryManager<T>);
nextItem = ((MemoryManager<T>)positionObject).Memory.Slice(startIndex, endIndex - startIndex);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void MoveToNextItemForMultiSegment(
in SequencePosition position,
object? endObject,
object? positionObject,
out ReadOnlyMemory<T> nextItem,
out SequencePosition next)
{
Debug.Assert(positionObject is ReadOnlySequenceSegment<T>);

ReadOnlyMemory<T> processedSlice;
ReadOnlySequenceSegment<T> startSegment = (ReadOnlySequenceSegment<T>)positionObject;
SequencePosition processedPosition;
int startIndex;

processedSlice = default;
processedPosition = default;
startIndex = position.GetInteger();

while ((startSegment != endObject)
&& (processedSlice.IsEmpty))
{
ReadOnlySequenceSegment<T>? nextSegment = startSegment.Next;

if (nextSegment == null)
{
ThrowHelper.ThrowInvalidOperationException_EndPositionNotReached();
}

processedSlice = startSegment.Memory.Slice(startIndex);
processedPosition = new SequencePosition(nextSegment, 0);
startSegment = nextSegment;
startIndex = processedPosition.GetInteger();
};

if (processedSlice.IsEmpty)
{
int endIndex = GetIndex(_endInteger);
nextItem = startSegment.Memory.Slice(startIndex, endIndex - startIndex);
next = default;
}
else
{
nextItem = processedSlice;
next = processedPosition;
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void MoveToNextItemForString(
in SequencePosition position,
object? positionObject,
out ReadOnlyMemory<T> nextItem)
{
if (positionObject != _endObject)
{
ThrowHelper.ThrowInvalidOperationException_EndPositionNotReached();
}

int startIndex = position.GetInteger();
int endIndex = GetIndex(_endInteger);

Debug.Assert(positionObject is string);
nextItem = (ReadOnlyMemory<T>)(object)((string)positionObject).AsMemory(startIndex, endIndex - startIndex);
}

internal readonly struct MoveResult
{
public MoveResult(
ReadOnlyMemory<T> item,
SequencePosition position,
MoveStatus status)
{
Item = item;
Position = position;
Status = status;
}

public bool IsSuccess
{
get
{
return Status == MoveStatus.Success;
}
}

public ReadOnlyMemory<T> Item { get; }

public SequencePosition Position { get; }

public MoveStatus Status { get; }
}

internal enum MoveStatus
{
Empty,
NotExist,
Success
}
}
}
12 changes: 6 additions & 6 deletions src/libraries/System.Memory/src/System/Buffers/ReadOnlySequence.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,13 +599,13 @@ public SequencePosition GetPosition(long offset, SequencePosition origin)
/// </summary>
public bool TryGet(ref SequencePosition position, out ReadOnlyMemory<T> memory, bool advance = true)
{
bool result = TryGetBuffer(position, out memory, out SequencePosition next);
if (advance)
{
position = next;
}
MoveResult moveResult;

moveResult = MoveToNextItem(ref position, advance);

memory = moveResult.Item;

return result;
return moveResult.IsSuccess;
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Memory/src/System/Buffers/SequenceReader.Search.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ private bool TryReadToInternal(out ReadOnlySequence<T> sequence, T delimiter, bo
AdvanceCurrentSpan(index);
}

sequence = Sequence.Slice(copy.Position, Position);
sequence = Sequence.Slice(copy.Position, copy.Sequence.End);
if (advancePastDelimiter)
{
Advance(1);
Expand Down
44 changes: 29 additions & 15 deletions src/libraries/System.Memory/src/System/Buffers/SequenceReader.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,24 +287,38 @@ private void GetNextSpan()
if (!Sequence.IsSingleSegment)
{
SequencePosition previousNextPosition = _nextPosition;
while (Sequence.TryGet(ref _nextPosition, out ReadOnlyMemory<T> memory, advance: true))
ReadOnlySequence<T>.MoveResult moveResult;

moveResult = Sequence.MoveToNextItem(ref _nextPosition, true);

switch (moveResult.Status)
{
_currentPosition = previousNextPosition;
if (memory.Length > 0)
{
CurrentSpan = memory.Span;
CurrentSpanIndex = 0;
return;
}
else
{
CurrentSpan = default;
CurrentSpanIndex = 0;
previousNextPosition = _nextPosition;
}
case ReadOnlySequence<T>.MoveStatus.Success:
{
CurrentSpan = moveResult.Item.Span;
CurrentSpanIndex = 0;
_currentPosition = previousNextPosition;
break;
}
case ReadOnlySequence<T>.MoveStatus.Empty:
{
CurrentSpan = default;
CurrentSpanIndex = 0;
_currentPosition = previousNextPosition;
_moreData = false;
break;
}
case ReadOnlySequence<T>.MoveStatus.NotExist:
{
_moreData = false;
break;
}
}
}
_moreData = false;
else
{
_moreData = false;
}
}

/// <summary>
Expand Down
Loading