Subject: HTTP/2: Race between PUSH_PROMISE and exclusive PRIORITY

From: Tom Bergan <[email-masked]>
Date: Thu, 15 Sep 2016 16:31:32 -0700

I believe the following example has a race:

1. Server receives request A.
2. Server sends PUSH_PROMISE frames for B, C, D.
3. Client sends PRIORITY to make B the exclusive parent of C
4. Server receives that PRIORITY frame.

After step 2, the server's local priority tree is A -> {B,C,D}, due to the
default priority rules in RFC 7540 Section 5.3.5. At step 4, the server
cannot know if the client has received the PUSH_PROMISE for D yet. This
makes it ambiguous whether the client intended the tree to be A -> B ->
{C,D} or A -> {B->C, D}. I believe this race can happen any time a
PUSH_PROMISE and exclusive PRIORITY can pass each other on the wire. I
think the race is impossible for non-exclusive PRIORITYs.

This is the simplest example of the race, but it's admittedly strange that
the client would send that specific PRIORITY frame. Here's slightly more
complex, but less strange example:

1. Client requests A then B, where B's HEADERS frame makes A the exclusive
parent of B
2. Server receives request A
3. Server sends PUSH_PROMISE frames for C, D.
4. Server receives request B
5. Client receives PUSH_PROMISE frames for C, D

At each step, the priority trees are updated like this:

1. Client's tree is A -> B
2. Server's tree is A
3. Server's tree is A -> {C,D}
4. Server's tree is A -> B -> {C,D}
5. Client's tree is A -> {B,C,D}

If steps 3 and 4 are swapped, the client and server will finish with the
same priority tree. As-is, they finish with different priority trees.

Does that sound right? If so, is this worth mentioning in some kind of
errata or other note? Apologies if this was brought up previously -- I
searched through the mailing list archives and did not see a mention.

-Tom

---

From: Martin Thomson <[email-masked]>
Date: Thu, 15 Sep 2016 17:03:41 -0700

Hey Tom,

This is a good bug.  I think that we built the exclusive thing without
considering that possibility.  The problem is that there is no knowing
what streams the sender of priority frame knew about when they send
the PRIORITY frame.

The same problem exists for PRIORITY sent by the server, but that's
far less of a problem because we don't see anyone using that (or if
they do, we don't agree on what that means).

A simple "fix" here is to prohibit PRIORITY+exclusive from affecting
streams that haven't been "mentioned" by the endpoint sending
PRIORITY.  That is, each endpoint maintains a watermark (for both odd
and even stream numbers) that increases every time they receive a
frame from their peer and any stream number below that is assumed to
be known to the other side.  That means tracking mentions though,
which is a little cumbersome.

--Martin

On 15 September 2016 at 16:31, Tom Bergan <[email-masked]> wrote:
> PUSH_PROMISE and exclusive PRIORITY can pass each other on the wire. I think
> the race is impossible for non-exclusive PRIORITYs.
>

---

From: Tom Bergan <[email-masked]>
Date: Fri, 16 Sep 2016 00:00:11 -0700

Your fix would work.

An even simpler (but maybe more controversial) fix is to eliminate the
notion of exclusive dependencies entirely. Say the tree is A -> {X1...XN}
and the client requests B, where the client wants to promote B above all
Xn. With exclusive dependencies, we can do this in a single HEADERS frame.
Without exclusive dependencies, we can still accomplish this by sending A
-> B in the HEADERS, followed by a PRIORITY frame for each B -> Xn. The
second version is a problem only if: (a) N is large, or (b) we are worried
about a race where the server receives the HEADERS frame and starts sending
a response before receiving the following PRIORITY frames.

(a) is an empirical question. Do any H2 clients use exclusive dependencies
on trees with large fanout? I can only speak for Chrome, which does not do
this. Chrome's priority trees are mostly linear.

(b) is a possible race, but I'm not sure how much we should care. The
server's priority tree is "wrong" between receiving HEADERS and the Nth
PRIORITY. In practice, I expect this will be a very short time period
unless there happens to be a congestion window boundary in the middle, and
since PRIORITY frames are small, I expect that to be rare, but this is only
a hunch.

On Thu, Sep 15, 2016 at 5:03 PM, Martin Thomson <[email-masked]>
wrote:

> think

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 20 Sep 2016 10:34:23 +0900

Hi,

> which is a little cumbersome.

I would like to point out that having that kind of method would also
be beneficial for server-side performance.

RFC 7540 section 5.3.4 suggests:

   an endpoint SHOULD retain stream
   prioritization state for a period after streams become closed.  The
   longer state is retained, the lower the chance that streams are
   assigned incorrect or default priority values.

This suggestion is vague and is hard to implement. Ideally, server
needs to maintain the prioritization states of closed streams at least
for one RTT so that it would not receive PRIORITY frames from client
that is relative to the state of a closed stream. However, it is hard
to find out the correct RTT.

Therefore, a cautious server implementation would try to retain the
prioritization states of most-recently-used streams up to
MAX_CONCURRENT_STREAMS. By doing so, one can get rid of the risk to
receive a PRIORITY stream relative to the state of a closed stream,
since a client would never try to open more streams at once than
MAX_CONCURRENT_STREAMS.

However, trying to retain the prioritization states of streams as much
as MAX_CONCURRENT_STREAMS may become a performance bottleneck as you
try to more precisely distribute the bandwidth between the streams.

That said, there would be other ways than using a watermark to solve the issue.

For example, we could introduce frames for indicating the server that
the client has seen a stream getting open, or closed, or the priority
being changed.

For the push case, a client would send such frame as it receives
PUSH_PROMISE. Server can use the ordering between the such frames and
PRIORITY frame to find out the client's view of the prioritization
tree at the moment the client has sent the PRIORITY frame.

For the case described in this mail, a client would send such frame at
it sees a stream being closed. Server can use the frame to discard the
prioritization state of the closed streams.

-- 
Kazuho Oku

---

From: Martin Thomson <[email-masked]>
Date: Tue, 20 Sep 2016 11:56:42 +1000

> since a client would never try to open more streams at once than

That's not going to work, since the stream limit is unidirectional,
whereas priority applies to both.  Keeping 2xMAX_CONCURRENT_STREAMS
would be closer to what you want (but again, not perfect).

---

From: Kazuho Oku <[email-masked]>
Date: Tue, 20 Sep 2016 11:59:20 +0900

2016-09-20 10:56 GMT+09:00 Martin Thomson <[email-masked]>:
> whereas priority applies to both.

Thank you for the correction. I agree.

If a server is going to push streams, the number of prioritization
states that needs to be tracked becomes the maximum number of
concurrent streams opened by the client _plus_ the maximum opened by
the server needs to be tracked.

Anyways, tracking such large amount of states is inefficient, and I
think we should better fix the issue together with the race pointed
out by Tom.

-- 
Kazuho Oku