Skip to content

Fix Arraypool rent without try/finally block #129158

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

Open
alinpahontu2912 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix Arraypool rent without try/finally block #129158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
99 changes: 62 additions & 37 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Read.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,24 @@ internal sealed partial class TarHeader

// The four supported formats have a header that fits in the default record size
byte[] rented = ArrayPool<byte>.Shared.Rent(minimumLength: TarHelpers.RecordSize);
Memory<byte> buffer = rented.AsMemory(0, TarHelpers.RecordSize); // minimumLength means the array could've been larger
try
{
Memory<byte> buffer = rented.AsMemory(0, TarHelpers.RecordSize); // minimumLength means the array could've been larger

await archiveStream.ReadExactlyAsync(buffer, cancellationToken).ConfigureAwait(false);
await archiveStream.ReadExactlyAsync(buffer, cancellationToken).ConfigureAwait(false);

TarHeader? header = TryReadAttributes(initialFormat, buffer.Span, archiveStream);
if (header != null && processDataBlock)
TarHeader? header = TryReadAttributes(initialFormat, buffer.Span, archiveStream);
if (header != null && processDataBlock)
{
await header.ProcessDataBlockAsync(archiveStream, copyData, cancellationToken).ConfigureAwait(false);
}

return header;
}
finally
{
await header.ProcessDataBlockAsync(archiveStream, copyData, cancellationToken).ConfigureAwait(false);
ArrayPool<byte>.Shared.Return(rented);
}

ArrayPool<byte>.Shared.Return(rented);

return header;
}

private static TarHeader? TryReadAttributes(TarEntryFormat initialFormat, ReadOnlySpan<byte> buffer, Stream archiveStream)
Expand Down Expand Up @@ -691,17 +696,22 @@ private void ReadExtendedAttributesBlock(Stream archiveStream)
ValidateSize();

byte[]? buffer = null;
Span<byte> span = (ulong)size <= 256 ?
stackalloc byte[256] :
(buffer = ArrayPool<byte>.Shared.Rent((int)size));
span = span.Slice(0, (int)size);

archiveStream.ReadExactly(span);
ReadExtendedAttributesFromBuffer(span, _name);
try
{
Span<byte> span = (ulong)size <= 256 ?
stackalloc byte[256] :
(buffer = ArrayPool<byte>.Shared.Rent((int)size));
span = span.Slice(0, (int)size);

if (buffer is not null)
archiveStream.ReadExactly(span);
ReadExtendedAttributesFromBuffer(span, _name);
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
if (buffer is not null)
{
ArrayPool<byte>.Shared.Return(buffer);
}
}
}
}
Expand All @@ -716,12 +726,17 @@ private async ValueTask ReadExtendedAttributesBlockAsync(Stream archiveStream, C
{
ValidateSize();
byte[] buffer = ArrayPool<byte>.Shared.Rent((int)_size);
Memory<byte> memory = buffer.AsMemory(0, (int)_size);

await archiveStream.ReadExactlyAsync(memory, cancellationToken).ConfigureAwait(false);
ReadExtendedAttributesFromBuffer(memory.Span, _name);
try
{
Memory<byte> memory = buffer.AsMemory(0, (int)_size);

ArrayPool<byte>.Shared.Return(buffer);
await archiveStream.ReadExactlyAsync(memory, cancellationToken).ConfigureAwait(false);
ReadExtendedAttributesFromBuffer(memory.Span, _name);
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
}
}
}

Expand Down Expand Up @@ -765,17 +780,22 @@ private void ReadGnuLongPathDataBlock(Stream archiveStream)
ValidateSize();

byte[]? buffer = null;
Span<byte> span = (ulong)size <= 256 ?
stackalloc byte[256] :
(buffer = ArrayPool<byte>.Shared.Rent((int)size));
span = span.Slice(0, (int)size);

archiveStream.ReadExactly(span);
ReadGnuLongPathDataFromBuffer(span);
try
{
Span<byte> span = (ulong)size <= 256 ?
stackalloc byte[256] :
(buffer = ArrayPool<byte>.Shared.Rent((int)size));
span = span.Slice(0, (int)size);

if (buffer is not null)
archiveStream.ReadExactly(span);
ReadGnuLongPathDataFromBuffer(span);
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
if (buffer is not null)
{
ArrayPool<byte>.Shared.Return(buffer);
}
}
}
}
Expand All @@ -791,12 +811,17 @@ private async ValueTask ReadGnuLongPathDataBlockAsync(Stream archiveStream, Canc
{
ValidateSize();
byte[] buffer = ArrayPool<byte>.Shared.Rent((int)_size);
Memory<byte> memory = buffer.AsMemory(0, (int)_size);

await archiveStream.ReadExactlyAsync(memory, cancellationToken).ConfigureAwait(false);
ReadGnuLongPathDataFromBuffer(memory.Span);
try
{
Memory<byte> memory = buffer.AsMemory(0, (int)_size);

ArrayPool<byte>.Shared.Return(buffer);
await archiveStream.ReadExactlyAsync(memory, cancellationToken).ConfigureAwait(false);
ReadGnuLongPathDataFromBuffer(memory.Span);
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
}
}
}

Expand Down
100 changes: 55 additions & 45 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -851,11 +851,16 @@ private async Task WriteDataAsync(Stream archiveStream, Stream dataStream, Cance
if (paddingAfterData != 0)
{
byte[] buffer = ArrayPool<byte>.Shared.Rent(paddingAfterData);
Array.Clear(buffer, 0, paddingAfterData);

await archiveStream.WriteAsync(buffer.AsMemory(0, paddingAfterData), cancellationToken).ConfigureAwait(false);
try
{
Array.Clear(buffer, 0, paddingAfterData);

ArrayPool<byte>.Shared.Return(buffer);
await archiveStream.WriteAsync(buffer.AsMemory(0, paddingAfterData), cancellationToken).ConfigureAwait(false);
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
}
}
}

Expand All @@ -868,58 +873,63 @@ private async Task WriteDataAsync(Stream archiveStream, Stream dataStream, Cance
byte[]? buffer = null;
Span<byte> span = stackalloc byte[512];

if (extendedAttributes.Count > 0)
try
{
dataStream = new MemoryStream();

foreach ((string attribute, string value) in extendedAttributes)
if (extendedAttributes.Count > 0)
{
// Generates an extended attribute key value pair string saved into a byte array, following the ISO/IEC 10646-1:2000 standard UTF-8 encoding format.
// https://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html

// The format is:
// "XX attribute=value\n"
// where "XX" is the number of characters in the entry, including those required for the count itself.
// If prepending the length digits increases the number of digits, we need to expand.
int length = 3 + Encoding.UTF8.GetByteCount(attribute) + Encoding.UTF8.GetByteCount(value);
int originalDigitCount = CountDigits(length), newDigitCount;
length += originalDigitCount;
while ((newDigitCount = CountDigits(length)) != originalDigitCount)
{
length += newDigitCount - originalDigitCount;
originalDigitCount = newDigitCount;
}
Debug.Assert(length == CountDigits(length) + 3 + Encoding.UTF8.GetByteCount(attribute) + Encoding.UTF8.GetByteCount(value));
dataStream = new MemoryStream();

// Get a large enough buffer if we don't already have one.
if (span.Length < length)
foreach ((string attribute, string value) in extendedAttributes)
{
if (buffer is not null)
// Generates an extended attribute key value pair string saved into a byte array, following the ISO/IEC 10646-1:2000 standard UTF-8 encoding format.
// https://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html

// The format is:
// "XX attribute=value\n"
// where "XX" is the number of characters in the entry, including those required for the count itself.
// If prepending the length digits increases the number of digits, we need to expand.
int length = 3 + Encoding.UTF8.GetByteCount(attribute) + Encoding.UTF8.GetByteCount(value);
int originalDigitCount = CountDigits(length), newDigitCount;
length += originalDigitCount;
while ((newDigitCount = CountDigits(length)) != originalDigitCount)
{
ArrayPool<byte>.Shared.Return(buffer);
length += newDigitCount - originalDigitCount;
originalDigitCount = newDigitCount;
}
span = buffer = ArrayPool<byte>.Shared.Rent(length);
Debug.Assert(length == CountDigits(length) + 3 + Encoding.UTF8.GetByteCount(attribute) + Encoding.UTF8.GetByteCount(value));

// Get a large enough buffer if we don't already have one.
if (span.Length < length)
{
if (buffer is not null)
{
ArrayPool<byte>.Shared.Return(buffer);
}
span = buffer = ArrayPool<byte>.Shared.Rent(length);
}

// Format the contents.
bool formatted = Utf8Formatter.TryFormat(length, span, out int bytesWritten);
Debug.Assert(formatted);
span[bytesWritten++] = (byte)' ';
bytesWritten += Encoding.UTF8.GetBytes(attribute, span.Slice(bytesWritten));
span[bytesWritten++] = (byte)'=';
bytesWritten += Encoding.UTF8.GetBytes(value, span.Slice(bytesWritten));
span[bytesWritten++] = (byte)'\n';

// Write it to the stream.
dataStream.Write(span.Slice(0, bytesWritten));
}

// Format the contents.
bool formatted = Utf8Formatter.TryFormat(length, span, out int bytesWritten);
Debug.Assert(formatted);
span[bytesWritten++] = (byte)' ';
bytesWritten += Encoding.UTF8.GetBytes(attribute, span.Slice(bytesWritten));
span[bytesWritten++] = (byte)'=';
bytesWritten += Encoding.UTF8.GetBytes(value, span.Slice(bytesWritten));
span[bytesWritten++] = (byte)'\n';

// Write it to the stream.
dataStream.Write(span.Slice(0, bytesWritten));
dataStream.Position = 0; // Ensure it gets written into the archive from the beginning
}

dataStream.Position = 0; // Ensure it gets written into the archive from the beginning
}

if (buffer is not null)
finally
{
ArrayPool<byte>.Shared.Return(buffer);
if (buffer is not null)
{
ArrayPool<byte>.Shared.Return(buffer);
}
}

return dataStream;
Expand Down
70 changes: 47 additions & 23 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,19 @@ internal static void AdvanceStream(Stream archiveStream, long bytesToDiscard)
else if (bytesToDiscard > 0)
{
byte[] buffer = ArrayPool<byte>.Shared.Rent(minimumLength: (int)Math.Min(MaxBufferLength, bytesToDiscard));
while (bytesToDiscard > 0)
try
{
int currentLengthToRead = (int)Math.Min(MaxBufferLength, bytesToDiscard);
archiveStream.ReadExactly(buffer.AsSpan(0, currentLengthToRead));
bytesToDiscard -= currentLengthToRead;
while (bytesToDiscard > 0)
{
int currentLengthToRead = (int)Math.Min(MaxBufferLength, bytesToDiscard);
archiveStream.ReadExactly(buffer.AsSpan(0, currentLengthToRead));
bytesToDiscard -= currentLengthToRead;
}
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
}
ArrayPool<byte>.Shared.Return(buffer);
}
}

Expand All @@ -80,28 +86,40 @@ internal static async ValueTask AdvanceStreamAsync(Stream archiveStream, long by
else if (bytesToDiscard > 0)
{
byte[] buffer = ArrayPool<byte>.Shared.Rent(minimumLength: (int)Math.Min(MaxBufferLength, bytesToDiscard));
while (bytesToDiscard > 0)
try
{
int currentLengthToRead = (int)Math.Min(MaxBufferLength, bytesToDiscard);
await archiveStream.ReadExactlyAsync(buffer, 0, currentLengthToRead, cancellationToken).ConfigureAwait(false);
bytesToDiscard -= currentLengthToRead;
while (bytesToDiscard > 0)
{
int currentLengthToRead = (int)Math.Min(MaxBufferLength, bytesToDiscard);
await archiveStream.ReadExactlyAsync(buffer, 0, currentLengthToRead, cancellationToken).ConfigureAwait(false);
bytesToDiscard -= currentLengthToRead;
}
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
}
ArrayPool<byte>.Shared.Return(buffer);
}
}

// Helps copy a specific number of bytes from one stream into another.
internal static void CopyBytes(Stream origin, Stream destination, long bytesToCopy)
{
byte[] buffer = ArrayPool<byte>.Shared.Rent(minimumLength: (int)Math.Min(MaxBufferLength, bytesToCopy));
while (bytesToCopy > 0)
try
{
int currentLengthToRead = (int)Math.Min(MaxBufferLength, bytesToCopy);
origin.ReadExactly(buffer.AsSpan(0, currentLengthToRead));
destination.Write(buffer.AsSpan(0, currentLengthToRead));
bytesToCopy -= currentLengthToRead;
while (bytesToCopy > 0)
{
int currentLengthToRead = (int)Math.Min(MaxBufferLength, bytesToCopy);
origin.ReadExactly(buffer.AsSpan(0, currentLengthToRead));
destination.Write(buffer.AsSpan(0, currentLengthToRead));
bytesToCopy -= currentLengthToRead;
}
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
}
ArrayPool<byte>.Shared.Return(buffer);
}

// Asynchronously helps copy a specific number of bytes from one stream into another.
Expand All @@ -110,15 +128,21 @@ internal static async ValueTask CopyBytesAsync(Stream origin, Stream destination
cancellationToken.ThrowIfCancellationRequested();

byte[] buffer = ArrayPool<byte>.Shared.Rent(minimumLength: (int)Math.Min(MaxBufferLength, bytesToCopy));
while (bytesToCopy > 0)
try
{
int currentLengthToRead = (int)Math.Min(MaxBufferLength, bytesToCopy);
Memory<byte> memory = buffer.AsMemory(0, currentLengthToRead);
await origin.ReadExactlyAsync(buffer, 0, currentLengthToRead, cancellationToken).ConfigureAwait(false);
await destination.WriteAsync(memory, cancellationToken).ConfigureAwait(false);
bytesToCopy -= currentLengthToRead;
while (bytesToCopy > 0)
{
int currentLengthToRead = (int)Math.Min(MaxBufferLength, bytesToCopy);
Memory<byte> memory = buffer.AsMemory(0, currentLengthToRead);
await origin.ReadExactlyAsync(buffer, 0, currentLengthToRead, cancellationToken).ConfigureAwait(false);
await destination.WriteAsync(memory, cancellationToken).ConfigureAwait(false);
bytesToCopy -= currentLengthToRead;
}
}
finally
{
ArrayPool<byte>.Shared.Return(buffer);
}
ArrayPool<byte>.Shared.Return(buffer);
}

// Returns the number of bytes until the next multiple of the record size.
Expand Down
Loading
Loading