Skip to content

Commit

Permalink
[release/7.0] TarReader should dispose underlying stream if leaveOpen…
Browse files Browse the repository at this point in the history
… is false (#80598)

* TarReader should dispose underlying stream if leaveOpen is false (#79899)

* Dispose underlying stream in TarReader.DisposeAsync() as well (#79920)

* Dispose underlying stream in TarReader.DisposeAsync() as well

Same as #79899

* Consolidate duplicated WrappedStream test helpers to Common sources

* Dispose stream passed to WrappedStream

* Dispose archive stream after the list of DataStreams (#80572)

* Dispose archive stream after the list of DataStreams

* Add tests for TarReader.DisposeAsync properly disposing underlying stream

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
  • Loading branch information
jozkee and akoeplinger authored Jan 13, 2023
1 parent 5359311 commit c8a73af
Show file tree
Hide file tree
Showing 9 changed files with 95 additions and 138 deletions.
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO;

namespace System.Formats.Tar
namespace System.IO
{
public class WrappedStream : Stream
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public TarReader(Stream archiveStream, bool leaveOpen = false)
}

/// <summary>
/// Disposes the current <see cref="TarReader"/> instance, and disposes the non-null <see cref="TarEntry.DataStream"/> instances of all the entries that were read from the archive.
/// Disposes the current <see cref="TarReader"/> instance, closes the archive stream, and disposes the non-null <see cref="TarEntry.DataStream"/> instances of all the entries that were read from the archive if the <c>leaveOpen</c> argument was set to <see langword="false"/> in the constructor.
/// </summary>
/// <remarks>The <see cref="TarEntry.DataStream"/> property of any entry can be replaced with a new stream. If the user decides to replace it on a <see cref="TarEntry"/> instance that was obtained using a <see cref="TarReader"/>, the underlying stream gets disposed immediately, freeing the <see cref="TarReader"/> of origin from the responsibility of having to dispose it.</remarks>
public void Dispose()
Expand All @@ -57,12 +57,17 @@ public void Dispose()
{
_isDisposed = true;

if (!_leaveOpen && _dataStreamsToDispose?.Count > 0)
if (!_leaveOpen)
{
foreach (Stream s in _dataStreamsToDispose)
if (_dataStreamsToDispose?.Count > 0)
{
s.Dispose();
foreach (Stream s in _dataStreamsToDispose)
{
s.Dispose();
}
}

_archiveStream.Dispose();
}
}
}
Expand All @@ -77,12 +82,17 @@ public async ValueTask DisposeAsync()
{
_isDisposed = true;

if (!_leaveOpen && _dataStreamsToDispose?.Count > 0)
if (!_leaveOpen)
{
foreach (Stream s in _dataStreamsToDispose)
if (_dataStreamsToDispose?.Count > 0)
{
await s.DisposeAsync().ConfigureAwait(false);
foreach (Stream s in _dataStreamsToDispose)
{
await s.DisposeAsync().ConfigureAwait(false);
}
}

await _archiveStream.DisposeAsync().ConfigureAwait(false);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
<Compile Include="TarFile\TarFile.CreateFromDirectory.Stream.Tests.cs" />
<Compile Include="TarFile\TarFile.ExtractToDirectory.File.Tests.cs" />
<Compile Include="TarFile\TarFile.ExtractToDirectory.Stream.Tests.cs" />
<Compile Include="TarReader\TarReader.Async.Tests.cs" />
<Compile Include="TarReader\TarReader.StreamConformanceTests.cs" />
<Compile Include="TarReader\TarReader.File.GlobalExtendedAttributes.Async.Tests.cs" />
<Compile Include="TarReader\TarReader.File.Async.Tests.Base.cs" />
Expand Down Expand Up @@ -68,9 +69,9 @@
<Compile Include="TarWriter\TarWriter.WriteEntry.Tests.cs" />
<Compile Include="TarWriter\TarWriter.WriteEntry.Entry.Ustar.Tests.cs" />
<Compile Include="TarWriter\TarWriter.WriteEntry.Entry.V7.Tests.cs" />
<Compile Include="WrappedStream.cs" Link="WrappedStream.cs" />
<Compile Include="$(CommonPath)DisableRuntimeMarshalling.cs" Link="Common\DisableRuntimeMarshalling.cs" />
<Compile Include="$(CommonTestPath)System\IO\ReparsePointUtilities.cs" Link="Common\System\IO\ReparsePointUtilities.cs" />
<Compile Include="$(CommonTestPath)System\IO\WrappedStream.cs" Link="Common\System\IO\WrappedStream.cs" />
<Compile Include="$(CommonTestPath)TestUtilities\System\DisableParallelization.cs" Link="Common\TestUtilities\System\DisableParallelization.cs" />
</ItemGroup>
<!-- Windows specific files -->
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
using Xunit;

namespace System.Formats.Tar.Tests
{
public partial class TarReader_Tests : TarTestsBase
{
[Fact]
public async Task TarReader_LeaveOpen_False_Async()
{
await using MemoryStream ms = GetTarMemoryStream(CompressionMethod.Uncompressed, TestTarFormat.pax, "many_small_files");
List<Stream> dataStreams = new List<Stream>();
await using (TarReader reader = new TarReader(ms, leaveOpen: false))
{
TarEntry entry;
while ((entry = await reader.GetNextEntryAsync()) != null)
{
if (entry.DataStream != null)
{
dataStreams.Add(entry.DataStream);
}
}
}

Assert.Throws<ObjectDisposedException>(() => ms.ReadByte());

Assert.True(dataStreams.Any());
foreach (Stream ds in dataStreams)
{
Assert.Throws<ObjectDisposedException>(() => ds.ReadByte());
}
}

[Fact]
public async Task TarReader_LeaveOpen_True_Async()
{
await using MemoryStream ms = GetTarMemoryStream(CompressionMethod.Uncompressed, TestTarFormat.pax, "many_small_files");
List<Stream> dataStreams = new List<Stream>();
await using (TarReader reader = new TarReader(ms, leaveOpen: true))
{
TarEntry entry;
while ((entry = await reader.GetNextEntryAsync()) != null)
{
if (entry.DataStream != null)
{
dataStreams.Add(entry.DataStream);
}
}
}

ms.ReadByte(); // Should not throw

Assert.True(dataStreams.Any());
foreach (Stream ds in dataStreams)
{
ds.ReadByte(); // Should not throw
ds.Dispose();
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

namespace System.Formats.Tar.Tests
{
public class TarReader_Tests : TarTestsBase
public partial class TarReader_Tests : TarTestsBase
{
[Fact]
public void TarReader_NullArchiveStream() => Assert.Throws<ArgumentNullException>(() => new TarReader(archiveStream: null));
Expand Down Expand Up @@ -38,6 +38,8 @@ public void TarReader_LeaveOpen_False()
}
}

Assert.Throws<ObjectDisposedException>(() => ms.ReadByte());

Assert.True(dataStreams.Any());
foreach (Stream ds in dataStreams)
{
Expand All @@ -62,6 +64,8 @@ public void TarReader_LeaveOpen_True()
}
}

ms.ReadByte(); // Should not throw

Assert.True(dataStreams.Any());
foreach (Stream ds in dataStreams)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static IEnumerable<object[]> WriteEntry_LongFileSize_TheoryData()
public void WriteEntry_LongFileSize(TarEntryFormat entryFormat, long size, bool unseekableStream)
{
// Write archive with a 8 Gb long entry.
FileStream tarFile = File.Open(GetTestFilePath(), new FileStreamOptions { Access = FileAccess.ReadWrite, Mode = FileMode.Create, Options = FileOptions.DeleteOnClose });
using FileStream tarFile = File.Open(GetTestFilePath(), new FileStreamOptions { Access = FileAccess.ReadWrite, Mode = FileMode.Create, Options = FileOptions.DeleteOnClose });
Stream s = unseekableStream ? new WrappedStream(tarFile, tarFile.CanRead, tarFile.CanWrite, canSeek: false) : tarFile;

using (TarWriter writer = new(s, leaveOpen: true))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public static IEnumerable<object[]> WriteEntry_LongFileSize_TheoryDataAsync()
public async Task WriteEntry_LongFileSizeAsync(TarEntryFormat entryFormat, long size, bool unseekableStream)
{
// Write archive with a 8 Gb long entry.
FileStream tarFile = File.Open(GetTestFilePath(), new FileStreamOptions { Access = FileAccess.ReadWrite, Mode = FileMode.Create, Options = FileOptions.DeleteOnClose });
await using FileStream tarFile = File.Open(GetTestFilePath(), new FileStreamOptions { Access = FileAccess.ReadWrite, Mode = FileMode.Create, Options = FileOptions.DeleteOnClose });
Stream s = unseekableStream ? new WrappedStream(tarFile, tarFile.CanRead, tarFile.CanWrite, canSeek: false) : tarFile;

await using (TarWriter writer = new(s, leaveOpen: true))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
<Compile Include="CompressionStreamUnitTests.Deflate.cs" />
<Compile Include="CompressionStreamUnitTests.Gzip.cs" />
<Compile Include="Utilities\StripHeaderAndFooter.cs" />
<Compile Include="Utilities\WrappedStream.cs" />
<Compile Include="XunitAssemblyAttributes.cs" />
<Compile Include="ZipArchive\zip_CreateTests.cs" />
<Compile Include="ZipArchive\zip_CreateTests.Comments.cs" />
Expand All @@ -36,6 +35,7 @@
<Compile Include="$(CommonTestPath)System\IO\Compression\LocalMemoryStream.cs" Link="Common\System\IO\Compression\LocalMemoryStream.cs" />
<Compile Include="$(CommonTestPath)System\IO\Compression\StreamHelpers.cs" Link="Common\System\IO\Compression\StreamHelpers.cs" />
<Compile Include="$(CommonTestPath)System\IO\TempFile.cs" Link="Common\System\IO\TempFile.cs" />
<Compile Include="$(CommonTestPath)System\IO\WrappedStream.cs" Link="Common\System\IO\WrappedStream.cs" />
<Compile Include="$(CommonTestPath)System\IO\Compression\ZipTestHelper.cs" Link="Common\System\IO\Compression\ZipTestHelper.cs" />
<Compile Include="$(CommonTestPath)TestUtilities\System\DisableParallelization.cs" Link="Common\TestUtilities\System\DisableParallelization.cs" />
<Compile Include="$(CommonPath)System\Threading\Tasks\TaskToApm.cs" Link="Common\System\Threading\Tasks\TaskToApm.cs" />
Expand Down
123 changes: 0 additions & 123 deletions src/libraries/System.IO.Compression/tests/Utilities/WrappedStream.cs

This file was deleted.

0 comments on commit c8a73af

Please sign in to comment.