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

[release/3.0] Don't dispose StreamPipeWriter CancellationToken until after Flush #39610

Merged
merged 1 commit into from Jul 19, 2019

Conversation

jkotalik
Copy link

Hit this in ASP.NET Core. If you call FlushAsync and then CompleteAsync, you'll hit an ODE in CompleteAsync. Previous tests didn't call FlushAsync and then CompleteAsync.

@jkotalik
Copy link
Author

@stephentoub I think we need to get this into 3.0. What is the process of doing that.

@davidfowl
Copy link
Member

cc @danmosemsft We need this for 3.0

@danmoseley
Copy link
Member

@stephentoub I think we need to get this into 3.0. What is the process of doing that.

@jkotalik presumably this is something you think customers will encounter? Is it a regression?

@davidfowl
Copy link
Member

@jkotalik presumably this is something you think customers will encounter? Is it a regression?

Late change made in preview8 exploding in some scenarios in ASP.NET Core when completing the http response body.

@danmoseley
Copy link
Member

OK, please merge into release/3.0 when ready. Put the "tell-mode" label on the PR. You can paste this text into it.

@jkotalik jkotalik changed the base branch from master to release/3.0 July 19, 2019 02:25
@jkotalik jkotalik changed the title Don't dispose StreamPipeWriter CancellationToken until after Flush [release/3.0] Don't dispose StreamPipeWriter CancellationToken until after Flush Jul 19, 2019
@jkotalik
Copy link
Author

Seems to be a hanging test on linux...

@jkotalik jkotalik closed this Jul 19, 2019
@jkotalik jkotalik reopened this Jul 19, 2019
@jkotalik jkotalik merged commit eb86f69 into release/3.0 Jul 19, 2019
@jkotalik jkotalik deleted the jkotalik/fixCompleteAsync branch July 19, 2019 15:50
@danmoseley
Copy link
Member

@jkotalik note that we do not mirror from release/3.0 to master. So this needs a separate PR into master. (In general, folks merge into master first (with no process), then port to release/3.0.)

@jkotalik
Copy link
Author

@danmosemsft to make sure I understand, corefx never merges from release/3.0 into master. Should I just cherry-pick this commit onto master?

@danmoseley
Copy link
Member

@jkotalik yep

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
5 participants