Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix pipe completion behavior#31685

Merged
pakrym merged 1 commit intodotnet:masterfrom
pakrym:pakrym/fix-completion-behaviour
Aug 10, 2018
Merged

Fix pipe completion behavior#31685
pakrym merged 1 commit intodotnet:masterfrom
pakrym:pakrym/fix-completion-behaviour

Conversation

@pakrym
Copy link

@pakrym pakrym commented Aug 9, 2018

Correct behaviour is "the first completion wins"
It was regressed in #31375 fix for https://github.com/dotnet/corefx/issues/31388

@pakrym pakrym requested review from ahsonkhan and halter73 August 9, 2018 21:56
Pipe.Writer.Complete();
Pipe.Writer.Complete(new Exception());

var result = await Pipe.Reader.ReadAsync();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, but the tests for ReadAsync are in the FlushAsync file, and the FlushAsync tests in the ReadAsync file.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I treated FlushAsyncCompletionTests.cs as writer completion tests and these tests are testing reader and writer completion behaviour.

@pakrym pakrym merged commit a3060ae into dotnet:master Aug 10, 2018
pakrym added a commit to pakrym/corefx that referenced this pull request Aug 10, 2018
pakrym added a commit to pakrym/corefx that referenced this pull request Aug 10, 2018
@karelz karelz added this to the 3.0 milestone Aug 21, 2018
danmoseley pushed a commit that referenced this pull request Sep 6, 2018
…thrown multiple times" (#31680)

* Do not expand stacktraces when completion exception is rethrown multiple times

* Fix pipe completion behavior (#31685)
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants