Clarify that PRIORITY does not close streams. #813
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
draft-ietf-httpbis-http2bis.xml
Outdated
Comment on lines
1035
to
1037
| The first use of a new stream identifier in a <xref target="HEADERS" format="none">HEADERS</xref> | ||
| or <xref target="PUSH_PROMISE" format="none">PUSH_PROMISE</xref> frame implicitly closes all streams in the "idle" | ||
| state that might have been initiated by that peer with a lower-valued stream identifier. |
There was a problem hiding this comment.
I've a little trouble with the ambiguity regarding stream identifiers in PUSH_PROMISE. I'm not clear on this, but it might be better to say:
Each type of stream transitions out of the "idle" state for one reason:
- A client-initiated stream becomes "open" when a HEADERS frame is sent on the stream.
- A server-initiated stream becomes "reserved" when a PUSH_PROMISE (sent on a request stream) promises a response on the stream.
When a stream transitions out of the "idle" state, all streams in that direction with lower-numbered stream identifiers that are also "idle" are closed. That is, an endpoint can skip a stream identifier, with the effect being that the skipped stream is immediately closed.
Too large a change?
There was a problem hiding this comment.
No, I think that's a good change. I may try to call out the specific stream identifier in the PUSH_PROMISE frame payload that is relevant here.
There was a problem hiding this comment.
Hmm, I abandoned that change a bit because it didn't read well with the paragraph above, so I've tried something else. Let me know how you feel about it.
Lukasa
force-pushed
the
cb-stream-state-priority
branch
from
8b22905
to
93c6991
Compare
martinthomson
approved these changes
Feb 4, 2021
Co-authored-by: Martin Thomson <mt@lowentropy.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a first stab at #759. I've gone for a minimal change here, simply clarifying that only HEADERS and PUSH_PROMISE frames have the effect of implicitly closing streams.
While I was writing this I did have a question that may need to be raised on-list: do HEADERS and PUSH_PROMISE only implicitly close streams if they are accepted? That is, if a client sends a HEADERS frame that triggers a stream error, does that HEADERS frame still implicitly close all streams with a lower stream ID that are in the "idle" state? This may also be worth clarifying.