Override CompleteAsync() in StreamPipeReader to call Stream.DisposeAsync (#67565)

* Override CompleteAsync() in StreamPipeReader to call Stream.DisposeAsync().

fix #67050

* Apply suggestions from code review

Co-authored-by: David Fowler <davidfowl@gmail.com>
* Update StreamPipeReader.cs

Feedback from code review

Co-authored-by: David Fowler <davidfowl@gmail.com>

diff --git a/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/StreamPipeReader.cs b/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/StreamPipeReader.cs
index 0cc899d..fad6054 100644 (file)
--- a/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/StreamPipeReader.cs
+++ b/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/StreamPipeReader.cs
@@ -170,9 +170,22 @@ namespace System.IO.Pipelines
         /// <inheritdoc />
         public override void Complete(Exception? exception = null)
         {
+            if (CompleteAndGetNeedsDispose())
+            {
+                InnerStream.Dispose();
+            }
+        }
+
+#if NETCOREAPP
+        public override ValueTask CompleteAsync(Exception? exception = null) =>
+            CompleteAndGetNeedsDispose() ? InnerStream.DisposeAsync() : default;
+#endif
+
+        private bool CompleteAndGetNeedsDispose()
+        {
             if (_isReaderCompleted)
             {
-                return;
+                return false;
             }
 
             _isReaderCompleted = true;
@@ -186,10 +199,7 @@ namespace System.IO.Pipelines
                 returnSegment.ResetMemory();
             }
 
-            if (!LeaveOpen)
-            {
-                InnerStream.Dispose();
-            }
+            return !LeaveOpen;
         }
 
         /// <inheritdoc />

diff --git a/src/libraries/System.IO.Pipelines/tests/StreamPipeReaderTests.cs b/src/libraries/System.IO.Pipelines/tests/StreamPipeReaderTests.cs
index a9cb0d3..b86151a 100644 (file)
--- a/src/libraries/System.IO.Pipelines/tests/StreamPipeReaderTests.cs
+++ b/src/libraries/System.IO.Pipelines/tests/StreamPipeReaderTests.cs
@@ -600,6 +600,27 @@ namespace System.IO.Pipelines.Tests
             await Assert.ThrowsAsync<OperationCanceledException>(async () => await reader.ReadAsync());
         }
 
+        [Fact]
+        public async Task CompleteCallsAppropriateDisposeMethodOnUnderlyingStream()
+        {
+            DisposalTrackingStream stream = new();
+            PipeReader reader = PipeReader.Create(stream);
+            reader.Complete();
+            Assert.True(stream.DisposeCalled);
+            Assert.False(stream.DisposeAsyncCalled);
+
+            stream = new();
+            reader = PipeReader.Create(stream);
+            await reader.CompleteAsync();
+#if NETCOREAPP
+            Assert.False(stream.DisposeCalled);
+            Assert.True(stream.DisposeAsyncCalled);
+#else
+            Assert.True(stream.DisposeCalled);
+            Assert.False(stream.DisposeAsyncCalled);
+#endif
+        }
+
         private static async Task<string> ReadFromPipeAsString(PipeReader reader)
         {
             ReadResult readResult = await reader.ReadAsync();
@@ -689,5 +710,24 @@ namespace System.IO.Pipelines.Tests
             }
 #endif
         }
+
+        private class DisposalTrackingStream : MemoryStream
+        {
+            public bool DisposeCalled { get; private set; }
+            public bool DisposeAsyncCalled { get; private set; }
+
+            protected override void Dispose(bool disposing)
+            {
+                DisposeCalled = true;
+            }
+
+#if NETCOREAPP
+            public override ValueTask DisposeAsync()
+            {
+                DisposeAsyncCalled = true;
+                return default;
+            }
+#endif
+        }
     }
 }