[release/7.0] TarReader should dispose underlying stream if leaveOpen is false

src/libraries/System.Formats.Tar/tests/WrappedStream.cs → src/libraries/Common/tests/System/IO/WrappedStream.cs

@@ -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
     {

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

@@ -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()
@@ -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();
                 }
             }
         }
@@ -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);
                 }
             }
         }

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

@@ -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" />
@@ -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 -->

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

@@ -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();
+            }
+        }
+    }
+}

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

@@ -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));
@@ -38,6 +38,8 @@ public void TarReader_LeaveOpen_False()
                 }
             }
 
+            Assert.Throws<ObjectDisposedException>(() => ms.ReadByte());
+
             Assert.True(dataStreams.Any());
             foreach (Stream ds in dataStreams)
             {
@@ -62,6 +64,8 @@ public void TarReader_LeaveOpen_True()
                 }
             }
 
+            ms.ReadByte(); // Should not throw
+
             Assert.True(dataStreams.Any());
             foreach (Stream ds in dataStreams)
             {

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

@@ -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))

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

@@ -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))

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

@@ -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" />
@@ -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" />