Implicit close of idle streams

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

Implicit close of idle streams

James M Snell
Just noticed that this clause has been added to Section 5.1.1:

   The first use of a new stream identifier implicitly closes all idle
streams that might have been initiated by that peer with a
lower-valued stream identifier.

This could use some clarification around what "idle stream" means. I
know we define it elsewhere but the context gets a bit lost here.

Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

Martin Thomson-3
On 13 August 2013 20:18, James M Snell <[hidden email]> wrote:
>    The first use of a new stream identifier implicitly closes all idle
> streams that might have been initiated by that peer with a
> lower-valued stream identifier.
>
> This could use some clarification around what "idle stream" means. I
> know we define it elsewhere but the context gets a bit lost here.

Would it be clearer if it said "... implicitly closes all streams in
the "idle" state ..." ?

Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

Alexey Melnikov
On 13/08/2013 21:37, Martin Thomson wrote:
> On 13 August 2013 20:18, James M Snell <[hidden email]> wrote:
>>     The first use of a new stream identifier implicitly closes all idle
>> streams that might have been initiated by that peer with a
>> lower-valued stream identifier.
>>
>> This could use some clarification around what "idle stream" means. I
>> know we define it elsewhere but the context gets a bit lost here.
> Would it be clearer if it said "... implicitly closes all streams in
> the "idle" state ..." ?
Yes. I initially got confused by this text as well.


Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

James M Snell
In reply to this post by Martin Thomson-3
Just saw you checked in the following change:

+  The first use of a new stream identifier implicitly closes all
streams in the "idle"
+  state that might have been initiated by that peer with a
lower-valued stream identifier.

That part that is confusing the most I think is the "might have been
initiated by that peer" part. The way I understood the state model,
"idle" streams are not "initiated"... All streams start in the "idle"
state. The word "initiate" is used in a number of places when talking
about putting streams into the "open" state.

-  James

On Tue, Aug 13, 2013 at 1:37 PM, Martin Thomson
<[hidden email]> wrote:

> On 13 August 2013 20:18, James M Snell <[hidden email]> wrote:
>>    The first use of a new stream identifier implicitly closes all idle
>> streams that might have been initiated by that peer with a
>> lower-valued stream identifier.
>>
>> This could use some clarification around what "idle stream" means. I
>> know we define it elsewhere but the context gets a bit lost here.
>
> Would it be clearer if it said "... implicitly closes all streams in
> the "idle" state ..." ?

Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

Martin Thomson-3
On 13 August 2013 23:38, James M Snell <[hidden email]> wrote:
> That part that is confusing the most I think is the "might have been
> initiated by that peer" part. The way I understood the state model,
> "idle" streams are not "initiated"... All streams start in the "idle"
> state. The word "initiate" is used in a number of places when talking
> about putting streams into the "open" state.

Let's whittle this down a little: would s/initiated/opened/ fix this,
or is this something related more generally to the "might have" thing
(i.e., a client opening stream 5 implicitly closes streams 1 and 3,
but not streams 2 or 4).  Do you think that the latter needs more
clarification too?

Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

James M Snell
On Tue, Aug 13, 2013 at 3:46 PM, Martin Thomson
<[hidden email]> wrote:

> On 13 August 2013 23:38, James M Snell <[hidden email]> wrote:
>> That part that is confusing the most I think is the "might have been
>> initiated by that peer" part. The way I understood the state model,
>> "idle" streams are not "initiated"... All streams start in the "idle"
>> state. The word "initiate" is used in a number of places when talking
>> about putting streams into the "open" state.
>
> Let's whittle this down a little: would s/initiated/opened/ fix this,
> or is this something related more generally to the "might have" thing
> (i.e., a client opening stream 5 implicitly closes streams 1 and 3,
> but not streams 2 or 4).  Do you think that the latter needs more
> clarification too?

The part that is confusing is: What if stream 3 is open. The way the
text currently reads, it's not clear if opening 5 causes open streams
to close... vs. opening 5 causes all *previously unused* streams with
lower stream ID's to be automatically closed, while leaving non-idle
streams with ids < 5 alone. (I know how it's supposed to work, of
course, but the way it's worded currently, it's not clear, and I'm
afraid that it could be confusing and misleading, particularly to
non-native english readers.

Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

Tatsuhiro Tsujikawa



On Wed, Aug 14, 2013 at 10:25 AM, James M Snell <[hidden email]> wrote:
On Tue, Aug 13, 2013 at 3:46 PM, Martin Thomson
<[hidden email]> wrote:
> On 13 August 2013 23:38, James M Snell <[hidden email]> wrote:
>> That part that is confusing the most I think is the "might have been
>> initiated by that peer" part. The way I understood the state model,
>> "idle" streams are not "initiated"... All streams start in the "idle"
>> state. The word "initiate" is used in a number of places when talking
>> about putting streams into the "open" state.
>
> Let's whittle this down a little: would s/initiated/opened/ fix this,
> or is this something related more generally to the "might have" thing
> (i.e., a client opening stream 5 implicitly closes streams 1 and 3,
> but not streams 2 or 4).  Do you think that the latter needs more
> clarification too?

The part that is confusing is: What if stream 3 is open. The way the
text currently reads, it's not clear if opening 5 causes open streams
to close... vs. opening 5 causes all *previously unused* streams with
lower stream ID's to be automatically closed, while leaving non-idle
streams with ids < 5 alone. (I know how it's supposed to work, of
course, but the way it's worded currently, it's not clear, and I'm
afraid that it could be confusing and misleading, particularly to
non-native english readers.


I'm non-native English readers and if you ask me it is confusing then I'll answers yes.
My understanding is that that explains the monotonically increasing stream identifier scheme
in formal way. I think it would be less confusing to convey that notion.


Best regards,

Tatsuhiro Tsujikawa

Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

Martin Thomson-3
On 14 August 2013 16:15, Tatsuhiro Tsujikawa <[hidden email]> wrote:
> I'm non-native English readers and if you ask me it is confusing then I'll
> answers yes.
> My understanding is that that explains the monotonically increasing stream
> identifier scheme
> in formal way. I think it would be less confusing to convey that notion.

I'm having trouble with this confusion because every time I read the
section in its entirety, it's perfectly clear to me.  Even after doing
what I can do compensate for the usual I-wrote-this blindness.

I'll add an example, which I hope clarifies this, but in the absence
of specific suggestion, I don't know how I can address these comments.

Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

William Chan (陈智昌)
Sorry Martin, I think I'm with everyone else in thinking this is confusing. You're being very precise with your language, and it's technically correct, but I believe that's because you've internalized the state diagram. The simplest, yet perhaps imprecise, way of describing this is to say that stream ids are monotonically increasing, so you once you use a new stream identifier X, you cannot newly use a stream identifier < X. I'm terrible at this editorial stuff and don't know if my new proposed wording is any better. I suspect it's very possibly worse :P


On Wed, Aug 14, 2013 at 6:51 PM, Martin Thomson <[hidden email]> wrote:
On 14 August 2013 16:15, Tatsuhiro Tsujikawa <[hidden email]> wrote:
> I'm non-native English readers and if you ask me it is confusing then I'll
> answers yes.
> My understanding is that that explains the monotonically increasing stream
> identifier scheme
> in formal way. I think it would be less confusing to convey that notion.

I'm having trouble with this confusion because every time I read the
section in its entirety, it's perfectly clear to me.  Even after doing
what I can do compensate for the usual I-wrote-this blindness.

I'll add an example, which I hope clarifies this, but in the absence
of specific suggestion, I don't know how I can address these comments.


Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

James M Snell
Yes, with a bit of refinement that is better. I struggled to come up
with a good alternative also...

On Wed, Aug 14, 2013 at 10:02 AM, William Chan (陈智昌)
<[hidden email]> wrote:

> Sorry Martin, I think I'm with everyone else in thinking this is confusing.
> You're being very precise with your language, and it's technically correct,
> but I believe that's because you've internalized the state diagram. The
> simplest, yet perhaps imprecise, way of describing this is to say that
> stream ids are monotonically increasing, so you once you use a new stream
> identifier X, you cannot newly use a stream identifier < X. I'm terrible at
> this editorial stuff and don't know if my new proposed wording is any
> better. I suspect it's very possibly worse :P
>
>
> On Wed, Aug 14, 2013 at 6:51 PM, Martin Thomson <[hidden email]>
> wrote:
>>
>> On 14 August 2013 16:15, Tatsuhiro Tsujikawa <[hidden email]>
>> wrote:
>> > I'm non-native English readers and if you ask me it is confusing then
>> > I'll
>> > answers yes.
>> > My understanding is that that explains the monotonically increasing
>> > stream
>> > identifier scheme
>> > in formal way. I think it would be less confusing to convey that notion.
>>
>> I'm having trouble with this confusion because every time I read the
>> section in its entirety, it's perfectly clear to me.  Even after doing
>> what I can do compensate for the usual I-wrote-this blindness.
>>
>> I'll add an example, which I hope clarifies this, but in the absence
>> of specific suggestion, I don't know how I can address these comments.
>>
>

Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

Gábor Molnár
'Implicit closing' is not handled very well in the stream state machine. Closely reading the section about 'closed' state reveals that there's currently no restriction about receiving frames on an implicitly closed stream. It says:

      An endpoint MUST NOT send frames on a closed stream.  An endpoint
      that receives a frame after receiving a RST_STREAM or a frame
      containing a END_STREAM flag on that stream MUST treat that as a
      stream error (Section 5.4.2) of type STREAM_CLOSED.


And then goes on with ignoring specific frame types. So it's an error if the peer sends something after sending RST_STREAM or END_STREAM, but it's not an error if it sends something on an implicitly closed stream.

Promised streams are also implicitly half-closed, so this paragraph does not apply to those either, but I don't know if it's intentional or not because this has a specific use case (1. an endpoint promises a push stream 2. puts the whole push stream into the output buffer including the last data frame 3. from its perspective, the stream is now in 'fully closed' state 4. but then it receives an RST_STREAM(CANCEL) from the peer 5. it is not an error because it did not receive an RST_STREAM or END_STREAM from the peer before, so everyone is happy)

I think that one of the clean solutions would be to introduce a new 'unused' (or similar) stream state which raises a connection error if it receives anything. Or staying with the original wording without mentioning implicit closing. The seconds sounds simpler and I think that the original wording was accurate enough.


2013/8/14 James M Snell <[hidden email]>
Yes, with a bit of refinement that is better. I struggled to come up
with a good alternative also...

On Wed, Aug 14, 2013 at 10:02 AM, William Chan (陈智昌)
<[hidden email]> wrote:
> Sorry Martin, I think I'm with everyone else in thinking this is confusing.
> You're being very precise with your language, and it's technically correct,
> but I believe that's because you've internalized the state diagram. The
> simplest, yet perhaps imprecise, way of describing this is to say that
> stream ids are monotonically increasing, so you once you use a new stream
> identifier X, you cannot newly use a stream identifier < X. I'm terrible at
> this editorial stuff and don't know if my new proposed wording is any
> better. I suspect it's very possibly worse :P
>
>
> On Wed, Aug 14, 2013 at 6:51 PM, Martin Thomson <[hidden email]>
> wrote:
>>
>> On 14 August 2013 16:15, Tatsuhiro Tsujikawa <[hidden email]>
>> wrote:
>> > I'm non-native English readers and if you ask me it is confusing then
>> > I'll
>> > answers yes.
>> > My understanding is that that explains the monotonically increasing
>> > stream
>> > identifier scheme
>> > in formal way. I think it would be less confusing to convey that notion.
>>
>> I'm having trouble with this confusion because every time I read the
>> section in its entirety, it's perfectly clear to me.  Even after doing
>> what I can do compensate for the usual I-wrote-this blindness.
>>
>> I'll add an example, which I hope clarifies this, but in the absence
>> of specific suggestion, I don't know how I can address these comments.
>>
>


Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

Gábor Molnár
So after reading through the state machine text a few times again, I realised that the last part handles implicitly closed streams with saying that anything that is not explicitly allowed should be declined with a connection error... But in this case, my push stream related scenario would also result in connection error, but that's not related to this issue, so opened a new one here: https://github.com/http2/http2-spec/issues/236


2013/8/20 Gábor Molnár <[hidden email]>
'Implicit closing' is not handled very well in the stream state machine. Closely reading the section about 'closed' state reveals that there's currently no restriction about receiving frames on an implicitly closed stream. It says:

      An endpoint MUST NOT send frames on a closed stream.  An endpoint
      that receives a frame after receiving a RST_STREAM or a frame
      containing a END_STREAM flag on that stream MUST treat that as a
      stream error (Section 5.4.2) of type STREAM_CLOSED.


And then goes on with ignoring specific frame types. So it's an error if the peer sends something after sending RST_STREAM or END_STREAM, but it's not an error if it sends something on an implicitly closed stream.

Promised streams are also implicitly half-closed, so this paragraph does not apply to those either, but I don't know if it's intentional or not because this has a specific use case (1. an endpoint promises a push stream 2. puts the whole push stream into the output buffer including the last data frame 3. from its perspective, the stream is now in 'fully closed' state 4. but then it receives an RST_STREAM(CANCEL) from the peer 5. it is not an error because it did not receive an RST_STREAM or END_STREAM from the peer before, so everyone is happy)

I think that one of the clean solutions would be to introduce a new 'unused' (or similar) stream state which raises a connection error if it receives anything. Or staying with the original wording without mentioning implicit closing. The seconds sounds simpler and I think that the original wording was accurate enough.


2013/8/14 James M Snell <[hidden email]>
Yes, with a bit of refinement that is better. I struggled to come up
with a good alternative also...

On Wed, Aug 14, 2013 at 10:02 AM, William Chan (陈智昌)
<[hidden email]> wrote:
> Sorry Martin, I think I'm with everyone else in thinking this is confusing.
> You're being very precise with your language, and it's technically correct,
> but I believe that's because you've internalized the state diagram. The
> simplest, yet perhaps imprecise, way of describing this is to say that
> stream ids are monotonically increasing, so you once you use a new stream
> identifier X, you cannot newly use a stream identifier < X. I'm terrible at
> this editorial stuff and don't know if my new proposed wording is any
> better. I suspect it's very possibly worse :P
>
>
> On Wed, Aug 14, 2013 at 6:51 PM, Martin Thomson <[hidden email]>
> wrote:
>>
>> On 14 August 2013 16:15, Tatsuhiro Tsujikawa <[hidden email]>
>> wrote:
>> > I'm non-native English readers and if you ask me it is confusing then
>> > I'll
>> > answers yes.
>> > My understanding is that that explains the monotonically increasing
>> > stream
>> > identifier scheme
>> > in formal way. I think it would be less confusing to convey that notion.
>>
>> I'm having trouble with this confusion because every time I read the
>> section in its entirety, it's perfectly clear to me.  Even after doing
>> what I can do compensate for the usual I-wrote-this blindness.
>>
>> I'll add an example, which I hope clarifies this, but in the absence
>> of specific suggestion, I don't know how I can address these comments.
>>
>



Reply | Threaded
Open this post in threaded view
|

Re: Implicit close of idle streams

Martin Thomson-3
On 20 August 2013 06:52, Gábor Molnár <[hidden email]> wrote:
> so opened a new one here: https://github.com/http2/http2-spec/issues/236

This is not specific to PUSH_PROMISE, it's a general race.  Thus, we
need more text (bleargh) to deal with the fact that a sender could
receive RST_STREAM after sending END_STREAM.