Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Nov 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions eng/restore/harvestPackages.targets
Original file line number Diff line number Diff line change
@@ -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" />
Expand Down
4 changes: 3 additions & 1 deletion src/libraries/System.IO.Pipelines/Directory.Build.props
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
46 changes: 32 additions & 14 deletions src/libraries/System.IO.Pipelines/tests/PipeReaderWriterFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
{
Expand Down
1 change: 1 addition & 0 deletions src/libraries/libraries-packages.proj
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->
Expand Down
10 changes: 9 additions & 1 deletion src/libraries/pkg/baseline/packageIndex.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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": {
Expand Down