Skip to content

Commit c8a73af

Browse files
jozkeeakoeplinger
jozkee
and
akoeplinger
authored
[release/7.0] TarReader should dispose underlying stream if leaveOpen 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>
1 parent 5359311 commit c8a73af

File tree

9 files changed

+95
-138
lines changed

9 files changed

+95
-138
lines changed

‎src/libraries/System.Formats.Tar/tests/WrappedStream.cs renamed to ‎src/libraries/Common/tests/System/IO/WrappedStream.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.IO;
5-
6-
namespace System.Formats.Tar
4+
namespace System.IO
75
{
86
public class WrappedStream : Stream
97
{

‎src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarReader.cs

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public TarReader(Stream archiveStream, bool leaveOpen = false)
4848
}
4949

5050
/// <summary>
51-
/// 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.
51+
/// 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.
5252
/// </summary>
5353
/// <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>
5454
public void Dispose()
@@ -57,12 +57,17 @@ public void Dispose()
5757
{
5858
_isDisposed = true;
5959

60-
if (!_leaveOpen && _dataStreamsToDispose?.Count > 0)
60+
if (!_leaveOpen)
6161
{
62-
foreach (Stream s in _dataStreamsToDispose)
62+
if (_dataStreamsToDispose?.Count > 0)
6363
{
64-
s.Dispose();
64+
foreach (Stream s in _dataStreamsToDispose)
65+
{
66+
s.Dispose();
67+
}
6568
}
69+
70+
_archiveStream.Dispose();
6671
}
6772
}
6873
}
@@ -77,12 +82,17 @@ public async ValueTask DisposeAsync()
7782
{
7883
_isDisposed = true;
7984

80-
if (!_leaveOpen && _dataStreamsToDispose?.Count > 0)
85+
if (!_leaveOpen)
8186
{
82-
foreach (Stream s in _dataStreamsToDispose)
87+
if (_dataStreamsToDispose?.Count > 0)
8388
{
84-
await s.DisposeAsync().ConfigureAwait(false);
89+
foreach (Stream s in _dataStreamsToDispose)
90+
{
91+
await s.DisposeAsync().ConfigureAwait(false);
92+
}
8593
}
94+
95+
await _archiveStream.DisposeAsync().ConfigureAwait(false);
8696
}
8797
}
8898
}

‎src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
<Compile Include="TarFile\TarFile.CreateFromDirectory.Stream.Tests.cs" />
3232
<Compile Include="TarFile\TarFile.ExtractToDirectory.File.Tests.cs" />
3333
<Compile Include="TarFile\TarFile.ExtractToDirectory.Stream.Tests.cs" />
34+
<Compile Include="TarReader\TarReader.Async.Tests.cs" />
3435
<Compile Include="TarReader\TarReader.StreamConformanceTests.cs" />
3536
<Compile Include="TarReader\TarReader.File.GlobalExtendedAttributes.Async.Tests.cs" />
3637
<Compile Include="TarReader\TarReader.File.Async.Tests.Base.cs" />
@@ -68,9 +69,9 @@
6869
<Compile Include="TarWriter\TarWriter.WriteEntry.Tests.cs" />
6970
<Compile Include="TarWriter\TarWriter.WriteEntry.Entry.Ustar.Tests.cs" />
7071
<Compile Include="TarWriter\TarWriter.WriteEntry.Entry.V7.Tests.cs" />
71-
<Compile Include="WrappedStream.cs" Link="WrappedStream.cs" />
7272
<Compile Include="$(CommonPath)DisableRuntimeMarshalling.cs" Link="Common\DisableRuntimeMarshalling.cs" />
7373
<Compile Include="$(CommonTestPath)System\IO\ReparsePointUtilities.cs" Link="Common\System\IO\ReparsePointUtilities.cs" />
74+
<Compile Include="$(CommonTestPath)System\IO\WrappedStream.cs" Link="Common\System\IO\WrappedStream.cs" />
7475
<Compile Include="$(CommonTestPath)TestUtilities\System\DisableParallelization.cs" Link="Common\TestUtilities\System\DisableParallelization.cs" />
7576
</ItemGroup>
7677
<!-- Windows specific files -->

‎src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Async.Tests.cs

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Collections.Generic;
5+
using System.IO;
6+
using System.Linq;
7+
using System.Threading.Tasks;
8+
using Xunit;
9+
10+
namespace System.Formats.Tar.Tests
11+
{
12+
public partial class TarReader_Tests : TarTestsBase
13+
{
14+
[Fact]
15+
public async Task TarReader_LeaveOpen_False_Async()
16+
{
17+
await using MemoryStream ms = GetTarMemoryStream(CompressionMethod.Uncompressed, TestTarFormat.pax, "many_small_files");
18+
List<Stream> dataStreams = new List<Stream>();
19+
await using (TarReader reader = new TarReader(ms, leaveOpen: false))
20+
{
21+
TarEntry entry;
22+
while ((entry = await reader.GetNextEntryAsync()) != null)
23+
{
24+
if (entry.DataStream != null)
25+
{
26+
dataStreams.Add(entry.DataStream);
27+
}
28+
}
29+
}
30+
31+
Assert.Throws<ObjectDisposedException>(() => ms.ReadByte());
32+
33+
Assert.True(dataStreams.Any());
34+
foreach (Stream ds in dataStreams)
35+
{
36+
Assert.Throws<ObjectDisposedException>(() => ds.ReadByte());
37+
}
38+
}
39+
40+
[Fact]
41+
public async Task TarReader_LeaveOpen_True_Async()
42+
{
43+
await using MemoryStream ms = GetTarMemoryStream(CompressionMethod.Uncompressed, TestTarFormat.pax, "many_small_files");
44+
List<Stream> dataStreams = new List<Stream>();
45+
await using (TarReader reader = new TarReader(ms, leaveOpen: true))
46+
{
47+
TarEntry entry;
48+
while ((entry = await reader.GetNextEntryAsync()) != null)
49+
{
50+
if (entry.DataStream != null)
51+
{
52+
dataStreams.Add(entry.DataStream);
53+
}
54+
}
55+
}
56+
57+
ms.ReadByte(); // Should not throw
58+
59+
Assert.True(dataStreams.Any());
60+
foreach (Stream ds in dataStreams)
61+
{
62+
ds.ReadByte(); // Should not throw
63+
ds.Dispose();
64+
}
65+
}
66+
}
67+
}

‎src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Tests.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
namespace System.Formats.Tar.Tests
1010
{
11-
public class TarReader_Tests : TarTestsBase
11+
public partial class TarReader_Tests : TarTestsBase
1212
{
1313
[Fact]
1414
public void TarReader_NullArchiveStream() => Assert.Throws<ArgumentNullException>(() => new TarReader(archiveStream: null));
@@ -38,6 +38,8 @@ public void TarReader_LeaveOpen_False()
3838
}
3939
}
4040

41+
Assert.Throws<ObjectDisposedException>(() => ms.ReadByte());
42+
4143
Assert.True(dataStreams.Any());
4244
foreach (Stream ds in dataStreams)
4345
{
@@ -62,6 +64,8 @@ public void TarReader_LeaveOpen_True()
6264
}
6365
}
6466

67+
ms.ReadByte(); // Should not throw
68+
6569
Assert.True(dataStreams.Any());
6670
foreach (Stream ds in dataStreams)
6771
{

‎src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.LongFile.Tests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public static IEnumerable<object[]> WriteEntry_LongFileSize_TheoryData()
3030
public void WriteEntry_LongFileSize(TarEntryFormat entryFormat, long size, bool unseekableStream)
3131
{
3232
// Write archive with a 8 Gb long entry.
33-
FileStream tarFile = File.Open(GetTestFilePath(), new FileStreamOptions { Access = FileAccess.ReadWrite, Mode = FileMode.Create, Options = FileOptions.DeleteOnClose });
33+
using FileStream tarFile = File.Open(GetTestFilePath(), new FileStreamOptions { Access = FileAccess.ReadWrite, Mode = FileMode.Create, Options = FileOptions.DeleteOnClose });
3434
Stream s = unseekableStream ? new WrappedStream(tarFile, tarFile.CanRead, tarFile.CanWrite, canSeek: false) : tarFile;
3535

3636
using (TarWriter writer = new(s, leaveOpen: true))

‎src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.LongFile.Tests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public static IEnumerable<object[]> WriteEntry_LongFileSize_TheoryDataAsync()
2020
public async Task WriteEntry_LongFileSizeAsync(TarEntryFormat entryFormat, long size, bool unseekableStream)
2121
{
2222
// Write archive with a 8 Gb long entry.
23-
FileStream tarFile = File.Open(GetTestFilePath(), new FileStreamOptions { Access = FileAccess.ReadWrite, Mode = FileMode.Create, Options = FileOptions.DeleteOnClose });
23+
await using FileStream tarFile = File.Open(GetTestFilePath(), new FileStreamOptions { Access = FileAccess.ReadWrite, Mode = FileMode.Create, Options = FileOptions.DeleteOnClose });
2424
Stream s = unseekableStream ? new WrappedStream(tarFile, tarFile.CanRead, tarFile.CanWrite, canSeek: false) : tarFile;
2525

2626
await using (TarWriter writer = new(s, leaveOpen: true))

‎src/libraries/System.IO.Compression/tests/System.IO.Compression.Tests.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
<Compile Include="CompressionStreamUnitTests.Deflate.cs" />
1818
<Compile Include="CompressionStreamUnitTests.Gzip.cs" />
1919
<Compile Include="Utilities\StripHeaderAndFooter.cs" />
20-
<Compile Include="Utilities\WrappedStream.cs" />
2120
<Compile Include="XunitAssemblyAttributes.cs" />
2221
<Compile Include="ZipArchive\zip_CreateTests.cs" />
2322
<Compile Include="ZipArchive\zip_CreateTests.Comments.cs" />
@@ -36,6 +35,7 @@
3635
<Compile Include="$(CommonTestPath)System\IO\Compression\LocalMemoryStream.cs" Link="Common\System\IO\Compression\LocalMemoryStream.cs" />
3736
<Compile Include="$(CommonTestPath)System\IO\Compression\StreamHelpers.cs" Link="Common\System\IO\Compression\StreamHelpers.cs" />
3837
<Compile Include="$(CommonTestPath)System\IO\TempFile.cs" Link="Common\System\IO\TempFile.cs" />
38+
<Compile Include="$(CommonTestPath)System\IO\WrappedStream.cs" Link="Common\System\IO\WrappedStream.cs" />
3939
<Compile Include="$(CommonTestPath)System\IO\Compression\ZipTestHelper.cs" Link="Common\System\IO\Compression\ZipTestHelper.cs" />
4040
<Compile Include="$(CommonTestPath)TestUtilities\System\DisableParallelization.cs" Link="Common\TestUtilities\System\DisableParallelization.cs" />
4141
<Compile Include="$(CommonPath)System\Threading\Tasks\TaskToApm.cs" Link="Common\System\Threading\Tasks\TaskToApm.cs" />

‎src/libraries/System.IO.Compression/tests/Utilities/WrappedStream.cs

Lines changed: 0 additions & 123 deletions
This file was deleted.

0 commit comments

Comments
 (0)