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

* Dispose archive stream after the list of DataStreams * Add tests for TarReader.DisposeAsync properly disposing underlying stream
jozkee · Jan 13, 2023 · b1f9f65 · b1f9f65
1 parent 1fbde51
commit b1f9f65
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 5 deletions.
diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarReader.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarReader.cs
@@ -59,15 +59,15 @@ public void Dispose()
 
                 if (!_leaveOpen)
                 {
-                    _archiveStream.Dispose();
-
                     if (_dataStreamsToDispose?.Count > 0)
                     {
                         foreach (Stream s in _dataStreamsToDispose)
                         {
                             s.Dispose();
                         }
                     }
+
+                    _archiveStream.Dispose();
                 }
             }
         }
@@ -84,15 +84,15 @@ public async ValueTask DisposeAsync()
 
                 if (!_leaveOpen)
                 {
-                    await _archiveStream.DisposeAsync().ConfigureAwait(false);
-
                     if (_dataStreamsToDispose?.Count > 0)
                     {
                         foreach (Stream s in _dataStreamsToDispose)
                         {
                             await s.DisposeAsync().ConfigureAwait(false);
                         }
                     }
+
+                    await _archiveStream.DisposeAsync().ConfigureAwait(false);
                 }
             }
         }

diff --git a/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj b/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" />

diff --git a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Async.Tests.cs b/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();
+            }
+        }
+    }
+}
diff --git a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.Tests.cs b/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));