h2client: a frame for a stream that has ended can reach the stream in its place (a late WINDOW_UPDATE credited to the next body; RST_STREAM answered by slot index) #98

Found in: the round-3 review of #91 (protocol lens), at #91's head 147edb4. Both mechanisms are on main (a89f02d) and #91 leaves them as they are, so they are not fixed there (#91 is #85, #86, #88 and #89).

Two places where readLoop credits or answers a frame for a stream by something other than that stream's own ID. When the stream has already ended, the frame reaches the stream that took its place.

1. A late stream WINDOW_UPDATE can be credited to the next request body

The writer sends one request body at a time. The send window of the stream whose body it is writing is kept in curStreamWindow, keyed by curStreamID.

• readLoop, on a stream WINDOW_UPDATE: } else if frame.StreamID == hc.curStreamID.Load() { hc.curStreamWindow.Add(incr) } (h2client.go:853-854 at a89f02d).
• The writer re-keys for each body: hc.curStreamWindow.Store(int64(hc.serverInitWindow)), then hc.curStreamID.Store(streamID) (:954-955 at a89f02d, in writeBodyFlowControlled). fix(h2client): real status and body bytes, no lifetime deadline, redial dead connections (#85, #86, #88, #89) #91 moves these two lines into processWriteReq, before the stream's HEADERS, in the same order.

The check and the add are two steps. Suppose a WINDOW_UPDATE for the previous body stream S arrives while the writer moves on to S'. readLoop can load curStreamID == S, the writer then stores S''s fresh window and S', and readLoop adds S's increment to S''s window.

The client then counts more window for S' than the server granted. It can send past the server's stream window, which the server treats as a FLOW_CONTROL_ERROR (RFC 9113 §6.9.1).

• When: a server sends stream WINDOW_UPDATEs as it consumes a body, and one of them lands in the nanoseconds between the check and the add.
• Effect: the extra credit matters only for a body that comes close to the server's stream window. probatorium's post-4k-h2 sends 4 KiB against a 65,535-byte default, so it cannot overrun. post-64k-h2 (65,536 bytes) was removed in v1.5.4.
• Possible fixes: do the re-key and the check-and-add under one small mutex (only bodies and stream WINDOW_UPDATEs take it), or pack the ID and the window into one 64-bit word updated by CAS.

2. RST_STREAM is answered by slot index, without the stream ID

readLoop answers RST_STREAM(S) by taking whatever channel slot (S>>1) % numSlots holds (h2client.go:810-815 at a89f02d; #91 adds only the curStreamReset store in front). S's slot can by then belong to a later stream S', numSlots = 2·MaxStreams streams later. In that case a late RST_STREAM for S fails S' with S's error code. S''s own response is then dropped, because its channel is gone.

A server sends RST_STREAM for a stream that is already closed when the client sends frames on it (RFC 9113 §5.1, STREAM_CLOSED); x/net's server does this for DATA on a stream it has reset.

• Before fix(h2client): real status and body bytes, no lifetime deadline, redial dead connections (#85, #86, #88, #89) #91's round 3, the request-body writer kept sending DATA after the server's reset for as long as window remained, and each such frame could draw one of these resets.
• Since round 3 it stops at the reset. Only frames already in flight can draw a late reset, about one round trip after the first.
• For S's slot to be taken in that time, 2·MaxStreams later streams must be opened. That takes about MaxStreams completions, whose responses the server normally sends after those late resets. A server whose write scheduler reorders frames across streams under backpressure can still send a late reset after them.

The HEADERS and DATA paths do not have this problem: a stream keeps its slot until its END_STREAM or reset is read.

• Possible fix: have the writer record the stream ID next to the channel when it registers the stream (an atomic in h2StreamSlot). readLoop then answers a RST_STREAM only when the IDs match.

No published cell is known to be affected by either.