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

H2C connection to HTTP.sys on localhost causes infinite loop #42259

Closed
ManickaP opened this issue Sep 15, 2020 · 5 comments
Closed

H2C connection to HTTP.sys on localhost causes infinite loop #42259

ManickaP opened this issue Sep 15, 2020 · 5 comments

Comments

@ManickaP
Copy link
Member

ManickaP commented Sep 15, 2020

If ProcessIncommingFrames encounters an exception it tries to abort the HTTP 2 connection: https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs#L394. This will remove it from the connection pool. But if this happens synchronously from SetupAsync, the connection is not yet in the pool and still gets added: https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs#L642-L644.

As a result, any subsequent requests will end up in an infinite loop in SendWithRetryAsync since isNewConnection will be false: https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs#L913

@ManickaP ManickaP added this to the 6.0.0 milestone Sep 15, 2020
@ghost
Copy link

ghost commented Sep 15, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Sep 15, 2020
@ManickaP ManickaP changed the title H2C connection to HTTP.sys causes infinite loop H2C connection to HTTP.sys on localhost causes infinite loop Sep 15, 2020
@ManickaP ManickaP removed the untriaged New issue has not been triaged by the area owner label Sep 15, 2020
@karelz karelz added the bug label May 4, 2021
@karelz
Copy link
Member

karelz commented May 6, 2021

Triage: @JamesNK do you know how common this scenario is for gRPC? We are considering moving it to Future.

@JamesNK
Copy link
Member

JamesNK commented May 6, 2021

Using H2C with gRPC is very common.

Is this bug specific to HTTP.sys as a server, or is it a problem when doing H2C in general?

@karelz
Copy link
Member

karelz commented May 7, 2021

Is this bug specific to HTTP.sys as a server, or is it a problem when doing H2C in general?

We do not know. That said, if it wasn't reported on gRPC yet, I think it is safe to assume that either the scenario is rare, or that it works outside of HTTP.sys.
I'll move it to Future until we have more evidence this is painful.

@karelz karelz modified the milestones: 6.0.0, Future May 7, 2021
@ManickaP
Copy link
Member Author

ManickaP commented May 7, 2021

This code (retry logic and connection construction) has been substantially reworked and the culprit: isNewConnection has been removed.
I'm closing this, since this should not be a problem anymore. If we encounter it out in the wild we might reopen a re-investigate.

@ManickaP ManickaP closed this as completed May 7, 2021
@karelz karelz modified the milestones: Future, 6.0.0 May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants