x/net/http2: too much coupling between connection and stream window sizes

[snaury]

I have not really reproduced the problem in the code yet, so this is based on pure source code analysis that I did a while ago (even before http2 was merged into Go 1.6), and since I was reminded of it recently and code didn't change much in that regard in my recent review I decided to finally report it.

There is too much coupling in go implementation of http2 server, which makes stream windows practically useless. The problem is that the only code path where server increments a connection window is in noteBodyRead, where both connection and stream windows are updated for the remote side. What this effectively means is that at any given time all received (but not yet read by user code) data on all streams combined cannot exceed the connection window, making only one of them dominating the flow control. And since by default connection and stream windows are equal and rather small a single stream can consume the whole connection window, making it impossible for clients to send data on all other stream.

A hypothetical problematic case would be a file uploading service, that does some database pre-processing before starting to read from Request.Body. If that pre-processing takes a long enough time it would block all other concurrent POST requests until connection window starts to finally be drained.

What's more it appears that this approach of coupling connection and stream windows does not even save memory, since newWriterAndRequest allocates the full initialWindowSize buffer, which means memory footprint is numberOfStreams * streamWindowSize anyway, so updating connection window immediately upon parsing data frames would not increase it.

As far as I could understand from the spec connection and stream windows are supposed to be separate, and one good example I've seen recently was http2 in grpc c++ implementation, where connection window is substantially larger than stream window, and the algorithm is also different, as far as I understand: connection window is used for connection-level parsing, decoupled from streams, and updates to client start when more than 3/4 of it is consumed.

Sorry for not providing a code sample to this behavior, I've been postponing this bug report for so long already because of it that I finally decided at least some bug report now would probably be better than no bug report in the foreseeable future again.

[bradfitz]

Just looking at this again. The current open bug for this is #16512 (which arose from #16510).

Even though this one came first, I'm going to close this in favor of the more succinct #16512 because I don't think there's actually any violation of the flow control specs in our http2 implementation. Let me know if I'm reading this wrong, though.