TarReader should dispose underlying stream if leaveOpen is false (dot…

…net#79899)
akoeplinger · Dec 22, 2022 · 5090041 · 5090041
1 parent 48e7418
commit 5090041
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 4 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
@@ -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,11 +57,16 @@ public void Dispose()
             {
                 _isDisposed = true;
 
-                if (!_leaveOpen && _dataStreamsToDispose?.Count > 0)
+                if (!_leaveOpen)
                 {
-                    foreach (Stream s in _dataStreamsToDispose)
+                    _archiveStream.Dispose();
+
+                    if (_dataStreamsToDispose?.Count > 0)
                     {
-                        s.Dispose();
+                        foreach (Stream s in _dataStreamsToDispose)
+                        {
+                            s.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
@@ -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)
             {