Flow control may block non-flow-controlled frames

[molnarg]

Flow control may block non-flow-controlled frames

[molnarg]

Hm, what we lose with this is the guarantee that frames are sent (and received) in the order they are written in the stream. I looked this up in the spec and it says:

The order in which frames are sent on a stream is significant. Recipients process frames in the order they are received. In particular, the order of HEADERS, and DATA frames is semantically significant.

So I don't think we can optimize this.

[nwgh]

I'm not so sure about that. It may not be as simple as I was originally thinking, but I think it's doable. When I was looking at this, I wasn't thinking about HEADERS or DATA particularly, this came up when I was implementing ALTSVC. Here are some things that we would need to guard against:

• Sending any DATA before we've sent the full header block (HEADERS|PUSH_PROMISE CONTINUATION*) (because duh)
• Reordering frames within a header block or a sequence of DATA frames (because duh)
• Sending a SETTINGS that updates the compression context out-of-order with a header block (since the header block would depend on the previous max header table size)
• Sending a RST_STREAM if we have any data waiting (since it's possible that the other endpoint may want to do something with the data that we've sent, such as saving a partial download for a later range request)

The only other one that could get a little weird is sending a PRIORITY, though if we're sending DATA on an particular stream, we're probably not the one that should be sending PRIORITY frames for that stream, anyway. (Obviously we wouldn't want to send PRIORITY before HEADERS.)

I don't see any issues sending, for example, ALTSVC on a stream that's blocked on flow control. I can even see a reason to actually do so - if you believe the client isn't going to be opening its window any time soon but that it's not malicious (not an incredibly likely scenario, but still...), you may want to try to redirect it to get a new connection with a new flow control window.

[molnarg]

Yeah, it's doable in certain scenarios. I'm OK with handling the cases we know are safe specially, but I'm afraid of using very complicated logic to do this. How about implementing a _pushUpstreamWithoutFlowControl() method on the Stream class as an alternative to _pushUpstream() which could be used in cases when we know this can not cause trouble (like ALTSVC)?

It would look like this:

Stream.prototype._pushUpstreamWithoutFlowControl = function _pushUpstreamWithoutFlowControl(frame) {
  this.upstream._parentPush(frame); // Logs the frame and pushed directly into the output queue
  this._transition(true, frame);
};

[nwgh]

Yeah, I think that seems like a reasonable way to go about it. Once draft14 lands (which should be as soon as Martin has the spec revision ready, unless he surprises me with some last-minute changes), I'll work on adding this.