Subject: Shouldn't we add a new CLOSING state in H2 ?

From: Willy Tarreau <[email-masked]>
Date: Thu, 24 Jan 2019 15:06:56 +0100

Hi,

I've recently got an interesting problem reported about haproxy's H2
implementation which could possibly affect other implementations to
some extents. At least I think it is in part related to the issue
about the dependency tree being racy here :

   https://github.com/httpwg/http-extensions/issues/751

The problem is that from time to time some people are getting some TCP
resets at the end of a transfer during an haproxy process restart. I
analysed the situation a little bit and figured what can cause this.

In short, the client sends a HEADERS frame with ES flag set. The stream
is seen as half-closed(remote). Let's say it's a GET for a large object.
Haproxy passes it to the server, receives the response and starts to
stream the response to the client. During the transfer, haproxy gets a
signal indicating that it's going to be reloaded and must terminate its
connections ASAP. It then sends a GOAWAY with ID=1 to the client along
with the data which continue to be streamed, informing the client that
it doesn't want to receive new requests on this connection. Once it
finishes to transfer the large response, the last DATA frame carries
the ES flag, and the stream switches to CLOSED.

Since the connection no longer has any stream and already sent a GOAWAY
a long time ago to perform a graceful shutdown, it can naturally close.

Except that the client continues to send WINDOW_UPDATE frames back...
These frames will eventually meet a closed TCP connection, resulting
in a TCP reset often causing some payload truncation, especially with
the data still lying in the system's socket buffers (hence why it mosly
affects large objects).

We could imagine a lot of dirty time-based workarounds for this, but
they would only steer the problem to another area.

On TCP an exactly similar situation exists and is covered by the LAST_ACK
state : the connection remains in LAST_ACK until the FIN is acknowledged.

In my opinion a good approach here would consist in implementing a CLOSING
state before the CLOSED state as long as we know we have data in flight,
and only switching to CLOSED once enough WINDOW_UPDATE were received.

The frames causing this problem are essentially DATA frames, because these
ones lead to WINDOW_UPDATE frames being sent back. These ones are almost
ACKs. I'm saying "almost" because they can also be used to bump the window
at once. But at least we know we're expected to continue to receive such
frames until the window has not grown by at least the number of bytes
sent since the last update. And in all situations where the stream window
doesn't change it would even be an exact match.

This could also help address some of the races that exist in error handling
for frames received on closed streams since in general the stream is not
available anymore to know whether it was closed with ES or RST_STREAM for
example.

A possible improvement to the approach above could be to add a new setting
mentioning that WINDOW_UPDATE=0 is supported, and we'd use this to indicate
that we've received the ES bit or RST_STREAM for the associated stream. It
could possibly even be extended to the connection itself, by indicating that
we've reached the end of the last stream advertised in GOAWAY since in this
case we know the other side is not going to send anything anymore.

Does anyone here think there would be any interest in working on such a
thing ? I'm personally interested in trying to stuff something like this
into haproxy. But while the passive monitoring of WINDOW_UPDATE might work
without cooperation from the other side, something more robust like an
extension for WU=0 would need support from other agents, so if nobody
is interested it's not worth working on it. That's why I'm interested in
gathering opinions.

Thanks,
Willy

---

From: Alcides Viamontes E <[email-masked]>
Date: Thu, 24 Jan 2019 19:31:32 +0100

> But at least we know we're expected to continue to receive such
frames until the window has not grown by at least the number of bytes
sent since the last update

I'm lost here, what's preventing *some* user-agents from reading
`content-length` headers and stop sending WINDOW_UPDATE frames right about
when the stream is approaching the expected payload length, while *other*
user-agents send WINDOW_UPDATE frames up to the last byte and beyond?
Section 8.1.2.6 seems to imply that `content-length` is as good as ever, so
some user-agents can get stingy and send just enough rope in  WINDOW_UPDATE
to cover up to the last byte on the stream.

Acknowledging reception of the ES and forbidding WINDOW_UPDATE after such
an acknowledgment would indeed provide a nice solution, though perhaps it
would be better to use a new explicit frame, e.g. STREAM_CLOSED_ACK. I
suspect this frame would even be useful in the opposite situation when the
user-agent is doing a large upload and wants to transition from
half-closed(remote) to closed as soon as it is done.

We had to go the way of the dirty time-based workarounds to implement
seamless reloads, and unless all user-agents implement a solution at once,
I don't think we can get rid of them :-( ...

-- 
Alcides Viamontes E.
ShimmerCat AB, CTO
(+46) 722294542

---

From: Willy Tarreau <[email-masked]>
Date: Thu, 24 Jan 2019 20:02:11 +0100

> to cover up to the last byte on the stream.

Excellent point, I hadn't thought about it! That's why I really needed
to expose the problem!

> half-closed(remote) to closed as soon as it is done.

Possibly indeed.

> I don't think we can get rid of them :-( ...

/me is sad to read this :-(

At least with an extension, only the connections not handled by clients
supporting the extension would stay alive, this could significantly limit
the extent of the problem once major clients support it.

Thanks,
Willy

---

From: Willy Tarreau <[email-masked]>
Date: Fri, 25 Jan 2019 05:39:44 +0100

> I don't think we can get rid of them :-( ...

So I have another idea that I'll see if I can implement for this specific
case of the seamless reload operation : the server could send a PING to
the client after the last frame of the last stream, and wait for the
PING/ACK to come back. Once it gets it, it will know that all data frames
were properly received. Some implementations might let the PING/ACK frame
leave faster than WINDOW_UPDATES (which is not really an issue). But we
could as well emit a SETTINGS frame with SETTINGS_MAX_CONCURRENT_STREAMS=0
and wait for the ACK, eventhough in my opinion it will be equivalent.

I'll see how far I can go with this. But I'm definitely interested in
trying to address the stream state synchronization between both ends
anyway.

Willy

---

From: Patrick McManus <[email-masked]>
Date: Fri, 25 Jan 2019 16:04:15 -0500

I've run into this too. On H1 as well, of course. The solution space seems
to be

1] a go-away ack to handshake the go-away

2] document a strategy about not closing the session until all the session
flow control credits for streams <= last id have been returned to the
sender. session credits would be more reliable than stream credits, but I'm
not sure the peer is actually really required to send them while closing if
they seem redundant.

3] wait for h3 where this little tcp artifact of intentionally losing data
is not an issue.

it might be reasonable to do all 3

---

From: Willy Tarreau <[email-masked]>
Date: Fri, 25 Jan 2019 23:39:56 +0100

> 1] a go-away ack to handshake the go-away

In fact I got an idea about how to do this with what we have and that
I'll try to implement into haproxy : the idea is to send a SETTINGS
frame with a 0 stream limit to the client just after the last data
frame from the last stream and to wait for its ACK to come back. Once
the ACK is back, it will mean that the client has already received and
parsed all data frames, and since the stream limit is zero, it is not
going to send any extra streams there. In the worst case there could
be one or two WINDOW_UPDATE sent immediately after depending on the
frames scheduling but that will definitely be a minor detail since
what matters is that no RST is sent before the client receives all
data.

> not sure the peer is actually really required to send them while closing if

That's what Alcides thinks as well and I must confess I never thought
about this risk, but it seems non-null. We could however document a
recommendation to ACK everything so that implementations can rely on
this to know the stream is really finished.

> 3] wait for h3 where this little tcp artifact of intentionally losing data

Sure but TCP isn't going to disappear in an eye blink ;-)

> it might be reasonable to do all 3

I think so as well. I'll report back about my experiments with SETTINGS
before closing the connection. And for the issue that affects stream
dependencies I think it's still valuable to suggest how to proceed in
the most backwards compatible way. For example if we have a signal
indicating that an implementation engages in always ACKing the exact
sum of all DATA frames, we could recommend that implementations start
to use this as an optional signal.

Cheers,
Willy

---

From: Martin Thomson <[email-masked]>
Date: Sun, 27 Jan 2019 08:43:49 +1100

> 3] wait for h3 where this little tcp artifact of intentionally losing data

h3 won't solve this issue.  At least not given its current design.  That's a fundamental limitation of the design there also.  While that design corrects some of the mistakes in h2, it doesn't provide any more surety to a server about where the client is.

The shutdown design in h3 relies on the observation that only the client knows when a shutdown is complete.  The consequence being that a server never really gets told when shutdown is complete.  The client drops the connection when it has all it needs, but there is no signal to the server when this happens, other than the CONNECTION_CLOSE that the client is obligated to send.  

The assumption implicit in that design is that the only reason that a server might initiate a close is when it simply cannot continue for any reason.  The server can initiate a shutdown by sending GOAWAY, but that's all.

A server could, if it were able to, observe the stream states and look for Data Recvd/Reset Recvd on all of them, but that's not a state that stacks will necessarily expose, just as they might not expose acknowledgements.  After all, we could watch for acknowledgments of data in TCP too.

Now, the obvious question becomes: well, what if the server does want to close the connection, but clients don't reliably do that when they are sent a GOAWAY?  I don't know the answer there.  It could be that it's a largely academic question anyway, if clients are sufficiently well-behaved.

FWIW, I don't think that a GOAWAY ACK is the right concept here.  For h2, you want a clean signal that the connection is closing, because that is the action a client takes when it is done.  RFC 7540 imagines that this signal is a TCP FIN.  But we know how reliable the TCP close is, so I guess there might be room for something else.

---

From: Patrick McManus <[email-masked]>
Date: Sat, 26 Jan 2019 17:29:16 -0500

On Sat, Jan 26, 2019 at 4:47 PM Martin Thomson <[email-masked]> wrote:

> h3 won't solve this issue.  At least not given its current design.

Perhaps you are looking at a broader issue than I am (and thought was what
wily was describing).. I was specifically referring to the pattern that
forces the RST when tcp data hits the closed connection which in return
forces the truncation and dropping of buffered valid recvd (even ack'd)
data on the client.. That should go away (ha) in h3, no?

---

From: Martin Thomson <[email-masked]>
Date: Sun, 27 Jan 2019 09:31:38 +1100

> data on the client.. That should go away (ha) in h3, no?

So if the server closes the connection, it will re-send the packet containing the CONNECTION_CLOSE, but that's all.  Whatever data the client might have received should still be available.