[release/5.0] Throw when reader is completed with an exception (#43903)

* Throw when reader is completed with an exception * fb * some infra * Update the assembly version as well * add assembly version * fix system.io.pipelines packageindex entry * Fix package harvesting by passing correct versions Co-authored-by: BrennanConroy <[email protected]> Co-authored-by: Anirudh Agnihotry <[email protected]>
dotnet · Nov 23, 2020 · 90b8f7e · 90b8f7e
1 parent e564220
commit 90b8f7e
Show file tree

Hide file tree

Showing 6 changed files with 59 additions and 25 deletions.
diff --git a/eng/restore/harvestPackages.targets b/eng/restore/harvestPackages.targets
@@ -1,16 +1,21 @@
-<Project InitialTargets="AddPackageDownload">
+<Project InitialTargets="AddPackageDownload">
   <UsingTask TaskName="GetLastStablePackage" AssemblyFile="$(PackagingTaskAssembly)"/>
-  <Target Name="AddPackageDownload">
-    <ItemGroup>
-      <_AllPkgProjs Include="$(LibrariesProjectRoot)*\pkg\**\*.pkgproj" />
-    </ItemGroup>
-    <!-- Need separate ItemGroups so right metadata gets populated -->
+
+  <Target Name="_GetBuildingPackageVersions">
     <ItemGroup>
-      <_AllPkgProjsToPackageIdentity Include="@(_AllPkgProjs -> '%(Filename)')" />
+      <_AllPkgProjs Include="$(LibrariesProjectRoot)*\pkg\**\*.pkgproj">
+        <UndefineProperties>MSBuildRestoreSessionId</UndefineProperties>
+      </_AllPkgProjs>
     </ItemGroup>
+
+    <MSBuild Projects="@(_AllPkgProjs)" Targets="GetPackageIdentityIfStable" Properties="$(ProjectProperties)">
+      <Output TaskParameter="TargetOutputs" ItemName="_AllPkgProjsWithVersion" />
+    </MSBuild>
+  </Target>
 
+  <Target Name="AddPackageDownload" DependsOnTargets="_GetBuildingPackageVersions">    
     <GetLastStablePackage
-      LatestPackages="@(_AllPkgProjsToPackageIdentity)"
+      LatestPackages="@(_AllPkgProjsWithVersion)"
       PackageIndexes="$(PackageIndexFile)"
       DoNotAllowVersionsFromSameRelease="true">
       <Output TaskParameter="LastStablePackages" ItemName="_PackageDownload" />

diff --git a/src/libraries/System.IO.Pipelines/Directory.Build.props b/src/libraries/System.IO.Pipelines/Directory.Build.props
@@ -2,5 +2,7 @@
   <Import Project="..\Directory.Build.props" />
   <PropertyGroup>
     <StrongNameKeyId>Open</StrongNameKeyId>
+    <PackageVersion>5.0.1</PackageVersion>
+    <AssemblyVersion>5.0.0.1</AssemblyVersion>
   </PropertyGroup>
-</Project>
+</Project>
diff --git a/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs b/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs
@@ -960,7 +960,7 @@ internal ValueTask<FlushResult> WriteAsync(ReadOnlyMemory<byte> source, Cancella
                 ThrowHelper.ThrowInvalidOperationException_NoWritingAllowed();
             }
 
-            if (_readerCompletion.IsCompleted)
+            if (_readerCompletion.IsCompletedOrThrow())
             {
                 return new ValueTask<FlushResult>(new FlushResult(isCanceled: false, isCompleted: true));
             }

diff --git a/src/libraries/System.IO.Pipelines/tests/PipeReaderWriterFacts.cs b/src/libraries/System.IO.Pipelines/tests/PipeReaderWriterFacts.cs
@@ -4,6 +4,7 @@
 using System.Buffers;
 using System.Collections.Generic;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using System.Text;
 using System.Text.RegularExpressions;
 using System.Threading;
@@ -199,36 +200,53 @@ public async Task HelloWorldAcrossTwoBlocks()
             Assert.Equal(" World", Encoding.ASCII.GetString(worldBytes));
         }
 
-        [Fact]
-        public async Task ReadAsync_ThrowsIfWriterCompletedWithException()
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        void ThrowTestException(Exception ex, Action<Exception> catchAction)
         {
-            void ThrowTestException()
+            try
             {
-                try
-                {
-                    throw new InvalidOperationException("Writer exception");
-                }
-                catch (Exception e)
-                {
-                    _pipe.Writer.Complete(e);
-                }
+                throw ex;
+            }
+            catch (Exception e)
+            {
+                catchAction(e);
             }
+        }
 
-            ThrowTestException();
+        [Fact]
+        public async Task ReadAsync_ThrowsIfWriterCompletedWithException()
+        {
+            ThrowTestException(new InvalidOperationException("Writer exception"), e => _pipe.Writer.Complete(e));
 
             InvalidOperationException invalidOperationException =
                 await Assert.ThrowsAsync<InvalidOperationException>(async () => await _pipe.Reader.ReadAsync());
 
             Assert.Equal("Writer exception", invalidOperationException.Message);
-            Assert.Contains("ThrowTestException", invalidOperationException.StackTrace);
+            Assert.Contains(nameof(ThrowTestException), invalidOperationException.StackTrace);
 
             invalidOperationException = await Assert.ThrowsAsync<InvalidOperationException>(async () => await _pipe.Reader.ReadAsync());
             Assert.Equal("Writer exception", invalidOperationException.Message);
-            Assert.Contains("ThrowTestException", invalidOperationException.StackTrace);
+            Assert.Contains(nameof(ThrowTestException), invalidOperationException.StackTrace);
 
             Assert.Single(Regex.Matches(invalidOperationException.StackTrace, "Pipe.GetReadResult"));
         }
 
+        [Fact]
+        public async Task WriteAsync_ThrowsIfReaderCompletedWithException()
+        {
+            ThrowTestException(new InvalidOperationException("Reader exception"), e => _pipe.Reader.Complete(e));
+
+            InvalidOperationException invalidOperationException =
+                await Assert.ThrowsAsync<InvalidOperationException>(async () => await _pipe.Writer.WriteAsync(new byte[1]));
+
+            Assert.Equal("Reader exception", invalidOperationException.Message);
+            Assert.Contains(nameof(ThrowTestException), invalidOperationException.StackTrace);
+
+            invalidOperationException = await Assert.ThrowsAsync<InvalidOperationException>(async () => await _pipe.Writer.WriteAsync(new byte[1]));
+            Assert.Equal("Reader exception", invalidOperationException.Message);
+            Assert.Contains(nameof(ThrowTestException), invalidOperationException.StackTrace);
+        }
+
         [Fact]
         public async Task ReaderShouldNotGetUnflushedBytes()
         {

diff --git a/src/libraries/libraries-packages.proj b/src/libraries/libraries-packages.proj
@@ -18,6 +18,7 @@
     <ProjectReference Include="$(PkgDir)*\*.proj" Exclude="$(PkgDir)test\*" Condition="'$(BuildAllOOBPackages)' == 'true'" />
     <ProjectReference Include="$(MSBuildThisFileDirectory)*\pkg\**\*.pkgproj" Condition="('$(BuildAllConfigurations)' == 'true' or '$(DotNetBuildFromSource)' == 'true') And '$(BuildAllOOBPackages)' == 'true'" />
     <!-- If setting BuildAllOOBPackages to false, add bellow the individual OOB packages you want to continue to build -->
+    <ProjectReference Include="$(LibrariesProjectRoot)\System.IO.Pipelines\pkg\System.IO.Pipelines.pkgproj" Condition="'$(BuildAllConfigurations)' == 'true'" />
   </ItemGroup>
 
   <!-- Need the PackageIndexFile file property from baseline.props -->

diff --git a/src/libraries/pkg/baseline/packageIndex.json b/src/libraries/pkg/baseline/packageIndex.json
@@ -3427,6 +3427,10 @@
         "4.5.2",
         "4.5.3",
         "4.6.0",
+        "4.7.0",
+        "4.7.1",
+        "4.7.2",
+        "4.7.3",
         "5.0.0"
       ],
       "BaselineVersion": "5.0.0",
@@ -3435,7 +3439,11 @@
         "4.0.0.0": "4.5.0",
         "4.0.0.1": "4.5.2",
         "4.0.1.0": "4.6.0",
-        "5.0.0.0": "5.0.0"
+        "4.0.2.0": "4.7.0",
+        "4.0.2.1": "4.7.1",
+        "4.0.2.2": "4.7.3",
+        "5.0.0.0": "5.0.0",
+        "5.0.0.1": "5.0.1"
       }
     },
     "System.IO.Pipes": {