Fix HTTP/2 race condition with concurrent window update

[stephentoub]

If a window update is received from the server between the time when we were creating an Http2Stream (initializing its available credit with the current initial window size) and when we stored the stream into the streams dictionary, we might miss out on updating the stream's window size, which could in turn lead to stalls.

@JamesNK, this addresses the repro for the hang I observed. Hopefully it's the same one you're seeing :)

Fixes dotnet/aspnetcore#22101
cc: @JamesNK, @geoffkizer

[ghost]

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

[JamesNK]

I'm guessing this is hard to unit test.

Since testing is hard, what about adding this scenario - POSTing large content with the request - be added to the HttpClient stability tests? I don't think it needs to be done now, but an issue to follow up later might be a good idea.

[stephentoub]

what about adding this scenario - POSTing large content with the request

We have such tests for that. The only way I was able to repro this fairly consistently was artificially slowing it down so much that there was a large enough time window between two lines of code pretty close to each other such that a specific response from the server could sneak in between them.

We do have stress tests, but I don't think they're currently running regularly. @alnikola, would you know?

[alnikola]

@stephentoub We have HttpStress and SslStress pipeline running nightly and on demand. You can you use the following command to run them on PR.

• /azp run runtime-libraries stress-http
• /azp run runtime-libraries stress-ssl

[stephentoub]

@alnikola, where do I see/ how do I navigate to the logs for the most recent runs?

[alnikola]

You can find them here:

• runtime-libraries stress-http
• runtime-libraries stress-ssl

Please note however, that stress infra was broken for a while and have been fixed only today by #35011 So, if you need fresh results, I'd suggest to run them explicitly on this PR.

[stephentoub]

stress infra was broken for a while and have been fixed only today by #35011. So, if you need fresh results, I'd suggest to run them explicitly on this PR.

I actually wanted to see if they flag the issue this is fixing, so running then on this PR won't help. I'll run them locally.

You can find them here

Thanks. How does someone discover those? Are they linked from somewhere?

[alnikola]

How does someone discover those? Are they linked from somewhere?

Unfortunately, currently there are no any dashboards available.
Our team have got higher priority tasks at the moment, but I think we'll come back to the stress test infra later to improve it and make more transparent.