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.

Sign up for GitHub

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

Jump to bottom

Update Async to server library #22575

Merged

halter73 merged 42 commits into dotnet:master from hueifeng:patch-10

Sep 17, 2020

Contributor

hueifeng commented Jun 5, 2020

Modify some methods to be asynchronous.

hueifeng requested review from halter73, jkotalik and Tratcher as code owners

June 5, 2020 16:10

ghost added the area-servers label

shirhatti added this to the Next sprint planning milestone

Contributor

shirhatti commented Jun 5, 2020

Solves #12334

Member

halter73 commented Jun 10, 2020

This at least partially solves #12334. There are more places we call Complete() outside of an async context which will likely be trickier to fix.

Contributor Author

hueifeng commented Jun 10, 2020

@halter73 I have modified some of the others. I am not sure whether we should keep both synchronous and asynchronous methods. I found that Abort is a synchronous method, and I am not sure whether to change it to asynchronous?

halter73 reviewed

View reviewed changes

src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs Outdated Show resolved Hide resolved

halter73 reviewed

View reviewed changes

src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs Outdated Show resolved Hide resolved

halter73 reviewed

View reviewed changes

src/Servers/Kestrel/Core/src/Internal/Http/ZeroContentLengthMessageBody.cs Outdated Show resolved Hide resolved

halter73 reviewed

View reviewed changes

src/Servers/Kestrel/Core/src/Internal/Http/MessageBody.cs Outdated Show resolved Hide resolved

halter73 reviewed

View reviewed changes

src/Servers/Kestrel/Core/src/Internal/Http3/Http3Stream.cs Outdated Show resolved Hide resolved

Member

halter73 commented Jun 11, 2020 •

edited

Loading

@hueifeng Thanks for working on this issues. I made my last comment about there being other calls to Complete() just so we don't prematurely close #12334. I wasn't asking you to address everything. I was going to merge your PR as it was.

It looks like you're on the right track though. If it gets too messy, I might just take the initial commit.

@halter73 I have modified some of the others. I am not sure whether we should keep both synchronous and asynchronous methods. I found that Abort is a synchronous method, and I am not sure whether to change it to asynchronous?

For internal interfaces and types, it's OK to get rid of the synchronous methods if they're not called.

halter73 reviewed

View reviewed changes

src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs Outdated Show resolved Hide resolved

halter73 reviewed

View reviewed changes

src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs Outdated Show resolved Hide resolved

halter73 reviewed

View reviewed changes

src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs Outdated Show resolved Hide resolved

halter73 reviewed

View reviewed changes

src/Servers/Kestrel/Core/src/Internal/Http3/Http3FrameWriter.cs Outdated Show resolved Hide resolved

halter73 reviewed

View reviewed changes

src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodingMessageBody.cs Outdated Show resolved Hide resolved

mkArtakMSFT assigned Tratcher

Tratcher assigned halter73 and unassigned Tratcher

hueifeng requested review from dougbu, javiercn, SteveSandersonMS and a team as code owners

September 10, 2020 22:23

halter73 force-pushed the patch-10 branch from 0a410a7 to 2bb13f1 Compare

September 10, 2020 22:26

Contributor

pranavkm commented Sep 10, 2020

Bad rebase?

halter73 force-pushed the patch-10 branch from 2bb13f1 to b52da76 Compare

September 10, 2020 22:31

halter73 approved these changes

View reviewed changes

hueifeng and others added 23 commits

September 15, 2020 13:25


          Update Http3MessageBody.cs

64e6588


          Update Http2MessageBody.cs

6fa3bc2


          Update Http1ChunkedEncodingMessageBody.cs

08a083e


          Update Http1ContentLengthMessageBody.cs

dbe405f


          Update MessageBody.cs

92262e5


          Update Http1ChunkedEncodingMessageBody.cs

cc1f72e


          Update Http1UpgradeMessageBody.cs

68d2f05


          Update Http2MessageBody.cs

119cc93


          Update Http3MessageBody.cs

fd090e1


          Update ZeroContentLengthMessageBody.cs


          Update MessageBody.cs

580d344


          Update Http3Stream.cs

96a3cbb


          Update ZeroContentLengthMessageBody.cs

37b08e0


          Update Http1UpgradeMessageBody.cs

b493c0e


          Update Http2Connection.cs

715a845


          Update src/Servers/Kestrel/Core/src/Internal/Http/Http1ChunkedEncodin…

eb61fc1

…gMessageBody.cs

Co-authored-by: Stephen Halter <halter73@gmail.com>


          Update src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs

934c054

Co-authored-by: Stephen Halter <halter73@gmail.com>


          Update src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs

6806e32

Co-authored-by: Stephen Halter <halter73@gmail.com>


          Update Http1ChunkedEncodingMessageBody.cs

30cb66c


          Update Http3FrameWriter.cs

d80891b


          Cleanup

eef9dd9


          Add HttpResponsePipeWriter.CompleteAsync()

43b7f95


          More cleanup

25a0c12

halter73 force-pushed the patch-10 branch from b52da76 to 25a0c12 Compare

September 15, 2020 22:41

halter73 removed the auto-merge label

halter73 merged commit 1cb3f23 into dotnet:master

halter73 mentioned this pull request

Implement IHttpResponseCompletionFeature.CompleteAsync for Kestrel HTTP/1.x #11195

Closed

runfoapp bot mentioned this pull request

Test initial triage jaredpar/runfo#50

Closed

amcasey added area-networking and removed area-runtime labels

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking community-contribution