Skip to content

Commit 1f8204f

Browse files
drpcstream,drpcwire: wire the receive-window grant policy into the read path
Install a per-stream recvWindow and drive it from the read/recv path: - HandleFrame intercepts an incoming KindWindowUpdate before the assembler and applies it to the send window. Intercepting early keeps a grant that interleaves with an in-progress message (grants are emitted off the write lock) from disturbing reassembly. - A dispatched KindMessage frame returns credit via recvWindow, emitting a KindWindowUpdate once the accrued amount crosses the threshold and the high-water gate is open. - RawRecv/MsgRecv call recvWindow after dequeuing; consuming can reopen a gate the high-water mark had closed and flush the accrued grant. Grants are control frames, appended without blocking, so emitting them from the reader goroutine is safe. The window is opt-in: with no recvw installed no grants are emitted, and an incoming grant with no send window is ignored, so default behavior is unchanged. Stream-level only; the connection-level receive budget and the enablement path that installs the windows come in later commits. The packet assembler can silently drop an unfinished message when a higher message id supersedes it (a legitimate sequence: a send preempted mid-message, then a cancel). Those bytes were counted into the receive window at dispatch and would inflate buffered forever, wedging the high-water gate shut and deadlocking the sender once its credit ran out. The assembler now records the drop (TakeDiscarded) and HandleFrame releases the bytes as consumed, letting their credit flow back. The release runs last (deferred past the replacement frame's accounting and handlePacket), so the grant decision sees the final buffered state rather than the transient dip, and a terminal packet that supersedes a partial suppresses the grant via termination. emitGrant is also a no-op once the stream has terminated: the ring drains queued messages after close, and returning credit behind the terminal frame would invite the peer to send more doomed data. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
1 parent 5256f29 commit 1f8204f

4 files changed

Lines changed: 435 additions & 1 deletion

File tree

Lines changed: 297 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,297 @@
1+
// Copyright (C) 2026 Cockroach Labs.
2+
// See LICENSE for copying information.
3+
4+
package drpcstream
5+
6+
import (
7+
"context"
8+
"io"
9+
"testing"
10+
"time"
11+
12+
"github.com/zeebo/assert"
13+
"github.com/zeebo/errs"
14+
15+
"storj.io/drpc"
16+
"storj.io/drpc/drpcwire"
17+
)
18+
19+
// captureWriter returns a MuxWriter whose output frames are decoded and
20+
// delivered on the returned channel, so tests can observe emitted grants.
21+
func captureWriter(t *testing.T) (*drpcwire.MuxWriter, <-chan drpcwire.Frame) {
22+
t.Helper()
23+
pr, pw := io.Pipe()
24+
mw := drpcwire.NewMuxWriter(pw, func(error) {})
25+
frames := make(chan drpcwire.Frame, 64)
26+
rd := drpcwire.NewReader(pr)
27+
go func() {
28+
for {
29+
fr, err := rd.ReadFrame()
30+
if err != nil {
31+
return
32+
}
33+
frames <- fr
34+
}
35+
}()
36+
t.Cleanup(func() {
37+
mw.Stop(nil)
38+
<-mw.Done()
39+
_ = pw.Close()
40+
_ = pr.Close()
41+
})
42+
return mw, frames
43+
}
44+
45+
func waitFrame(t *testing.T, frames <-chan drpcwire.Frame) drpcwire.Frame {
46+
t.Helper()
47+
select {
48+
case fr := <-frames:
49+
return fr
50+
case <-time.After(time.Second):
51+
t.Fatal("expected a frame to be written, got none")
52+
return drpcwire.Frame{}
53+
}
54+
}
55+
56+
func assertNoFrame(t *testing.T, frames <-chan drpcwire.Frame) {
57+
t.Helper()
58+
select {
59+
case fr := <-frames:
60+
t.Fatalf("unexpected frame written: %v", fr)
61+
case <-time.After(blockShort):
62+
}
63+
}
64+
65+
func msgFrame(sid, mid uint64, data []byte, done bool) drpcwire.Frame {
66+
return drpcwire.Frame{
67+
ID: drpcwire.ID{Stream: sid, Message: mid},
68+
Kind: drpcwire.KindMessage,
69+
Data: data,
70+
Done: done,
71+
}
72+
}
73+
74+
// An incoming KindWindowUpdate is intercepted and applied to the send window.
75+
func TestStream_IncomingGrantAppliesToSendWindow(t *testing.T) {
76+
st := newGateStream(t)
77+
st.sendw = newSendWindow(0)
78+
79+
assert.NoError(t, st.HandleFrame(drpcwire.WindowUpdateFrame(st.ID(), 500)))
80+
assert.Equal(t, st.sendw.available(), int64(500))
81+
}
82+
83+
// A grant interleaved with an in-progress message is intercepted before the
84+
// assembler, so reassembly is undisturbed.
85+
func TestStream_GrantDoesNotDisturbReassembly(t *testing.T) {
86+
st := newGateStream(t)
87+
sid := st.ID()
88+
89+
assert.NoError(t, st.HandleFrame(msgFrame(sid, 1, []byte("foo"), false)))
90+
assert.NoError(t, st.HandleFrame(drpcwire.WindowUpdateFrame(sid, 100))) // interleaved
91+
assert.NoError(t, st.HandleFrame(msgFrame(sid, 1, []byte("bar"), true)))
92+
93+
got, err := st.RawRecv()
94+
assert.NoError(t, err)
95+
assert.Equal(t, string(got), "foobar")
96+
}
97+
98+
// Dispatching a data frame returns credit once the threshold is met.
99+
func TestStream_DispatchEmitsGrant(t *testing.T) {
100+
mw, frames := captureWriter(t)
101+
st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10})
102+
st.recvw = newRecvWindow(1<<20, 100)
103+
104+
assert.NoError(t, st.HandleFrame(msgFrame(st.ID(), 1, make([]byte, 150), true)))
105+
106+
fr := waitFrame(t, frames)
107+
sid, delta, ok := drpcwire.ParseWindowUpdate(fr)
108+
assert.That(t, ok)
109+
assert.Equal(t, sid, st.ID())
110+
assert.Equal(t, delta, uint64(150))
111+
}
112+
113+
// Above the high-water mark, dispatch withholds; consuming resumes and flushes
114+
// the accrued credit.
115+
func TestStream_ConsumeEmitsGrantOnResume(t *testing.T) {
116+
mw, frames := captureWriter(t)
117+
st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10})
118+
st.recvw = newRecvWindow(100, 50)
119+
120+
// 120 bytes buffered >= high-water 100 -> withheld.
121+
assert.NoError(t, st.HandleFrame(msgFrame(st.ID(), 1, make([]byte, 120), true)))
122+
assertNoFrame(t, frames)
123+
124+
// Consume -> buffered drops below high-water -> flush accrued 120.
125+
_, err := st.RawRecv()
126+
assert.NoError(t, err)
127+
128+
fr := waitFrame(t, frames)
129+
_, delta, ok := drpcwire.ParseWindowUpdate(fr)
130+
assert.That(t, ok)
131+
assert.Equal(t, delta, uint64(120))
132+
}
133+
134+
// With no receive window installed, no grants are emitted and an incoming grant
135+
// with no send window is harmlessly ignored.
136+
func TestStream_NoWindowsNoGrants(t *testing.T) {
137+
mw, frames := captureWriter(t)
138+
st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10})
139+
140+
assert.NoError(t, st.HandleFrame(msgFrame(st.ID(), 1, make([]byte, 150), true)))
141+
assertNoFrame(t, frames)
142+
143+
assert.NoError(t, st.HandleFrame(drpcwire.WindowUpdateFrame(st.ID(), 100)))
144+
}
145+
146+
// Nonconforming grant frames are dropped by the intercept without crediting.
147+
func TestStream_MalformedGrantIgnored(t *testing.T) {
148+
st := newGateStream(t)
149+
st.sendw = newSendWindow(0)
150+
151+
missing := drpcwire.WindowUpdateFrame(st.ID(), 1)
152+
missing.Data = nil // no delta payload
153+
assert.NoError(t, st.HandleFrame(missing))
154+
assert.Equal(t, st.sendw.available(), int64(0))
155+
156+
zero := drpcwire.WindowUpdateFrame(st.ID(), 1)
157+
zero.Data = drpcwire.AppendVarint(nil, 0) // zero delta
158+
assert.NoError(t, st.HandleFrame(zero))
159+
assert.Equal(t, st.sendw.available(), int64(0))
160+
}
161+
162+
// A grant arriving after termination is dropped.
163+
func TestStream_GrantAfterTerminationDropped(t *testing.T) {
164+
st := newGateStream(t)
165+
st.sendw = newSendWindow(0)
166+
167+
st.Cancel(errs.New("boom"))
168+
assert.NoError(t, st.HandleFrame(drpcwire.WindowUpdateFrame(st.ID(), 100)))
169+
assert.Equal(t, st.sendw.available(), int64(0))
170+
}
171+
172+
// An unfinished message superseded by a higher message id must release its
173+
// buffered bytes, or the high-water gate sticks shut and the withheld grant
174+
// is never emitted (sender deadlock once its credit is exhausted).
175+
func TestStream_DiscardedPartialMessageReleasesCredit(t *testing.T) {
176+
mw, frames := captureWriter(t)
177+
st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10})
178+
st.recvw = newRecvWindow(100, 50)
179+
sid := st.ID()
180+
181+
// Partial message 1 reaches the high-water mark: grants withheld.
182+
assert.NoError(t, st.HandleFrame(msgFrame(sid, 1, make([]byte, 120), false)))
183+
assertNoFrame(t, frames)
184+
185+
// Message 2 supersedes it; the discarded 120 bytes leave the buffer after
186+
// the replacement is counted, the gate reopens, and all accrued credit
187+
// (120 discarded + 10 replacement) flushes.
188+
assert.NoError(t, st.HandleFrame(msgFrame(sid, 2, make([]byte, 10), true)))
189+
190+
_, delta, ok := drpcwire.ParseWindowUpdate(waitFrame(t, frames))
191+
assert.That(t, ok)
192+
assert.Equal(t, delta, uint64(130))
193+
194+
// The completed message is intact and its own credit accounting holds.
195+
got, err := st.RawRecv()
196+
assert.NoError(t, err)
197+
assert.Equal(t, len(got), 10)
198+
assert.Equal(t, st.recvw.bufferedBytes(), int64(0))
199+
}
200+
201+
// The discard release must not decide on the transient dip before the
202+
// replacement frame is counted: a large replacement that lands above
203+
// high-water withholds everything, including the discarded bytes' credit.
204+
func TestStream_DiscardReleaseSeesReplacementFrame(t *testing.T) {
205+
mw, frames := captureWriter(t)
206+
st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10})
207+
st.recvw = newRecvWindow(100, 50)
208+
sid := st.ID()
209+
210+
// Partial message 1 pins buffered at the high-water mark.
211+
assert.NoError(t, st.HandleFrame(msgFrame(sid, 1, make([]byte, 120), false)))
212+
assertNoFrame(t, frames)
213+
214+
// Message 2 (1000 bytes) supersedes it. Final buffered = 1000 >= 100, so
215+
// no grant may be emitted -- releasing the 120 on the dip would be wrong.
216+
assert.NoError(t, st.HandleFrame(msgFrame(sid, 2, make([]byte, 1000), true)))
217+
assertNoFrame(t, frames)
218+
219+
// Consuming the completed message reopens the gate and flushes everything.
220+
_, err := st.RawRecv()
221+
assert.NoError(t, err)
222+
_, delta, ok := drpcwire.ParseWindowUpdate(waitFrame(t, frames))
223+
assert.That(t, ok)
224+
assert.Equal(t, delta, uint64(1120))
225+
}
226+
227+
// A terminal frame that supersedes a partial message must not flush the
228+
// discarded bytes' credit: termination is applied first, and emitGrant
229+
// no-ops on a terminated stream.
230+
func TestStream_TerminalSupersedeEmitsNoGrant(t *testing.T) {
231+
mw, frames := captureWriter(t)
232+
st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10})
233+
st.recvw = newRecvWindow(100, 50)
234+
sid := st.ID()
235+
236+
// Partial message 1: grants withheld above high-water.
237+
assert.NoError(t, st.HandleFrame(msgFrame(sid, 1, make([]byte, 120), false)))
238+
assertNoFrame(t, frames)
239+
240+
// A remote cancel with a higher message id discards the partial and
241+
// terminates the stream; the withheld credit must stay unemitted.
242+
assert.NoError(t, st.HandleFrame(drpcwire.Frame{
243+
ID: drpcwire.ID{Stream: sid, Message: 2}, Kind: drpcwire.KindCancel, Done: true,
244+
}))
245+
assert.That(t, st.IsTerminated())
246+
assertNoFrame(t, frames)
247+
}
248+
249+
// Draining queued messages after termination must not emit grants: credit
250+
// returned behind the terminal frame would invite more doomed data.
251+
func TestStream_NoGrantsAfterTermination(t *testing.T) {
252+
mw, frames := captureWriter(t)
253+
st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10})
254+
st.recvw = newRecvWindow(100, 50)
255+
256+
// 120 bytes buffered >= high-water 100: grant withheld while live.
257+
assert.NoError(t, st.HandleFrame(msgFrame(st.ID(), 1, make([]byte, 120), true)))
258+
assertNoFrame(t, frames)
259+
260+
st.Cancel(errs.New("boom"))
261+
262+
// The queued message still drains (ring close keeps buffered data
263+
// readable), which on a live stream would flush the accrued 120 bytes of
264+
// credit; after termination it must stay unemitted.
265+
got, err := st.RawRecv()
266+
assert.NoError(t, err)
267+
assert.Equal(t, len(got), 120)
268+
assertNoFrame(t, frames)
269+
}
270+
271+
// rawEnc round-trips raw byte slices for MsgRecv tests.
272+
type rawEnc struct{}
273+
274+
func (rawEnc) Marshal(msg drpc.Message) ([]byte, error) { return *(msg.(*[]byte)), nil }
275+
func (rawEnc) Unmarshal(buf []byte, msg drpc.Message) error {
276+
*(msg.(*[]byte)) = append([]byte(nil), buf...)
277+
return nil
278+
}
279+
280+
// The MsgRecv consume path also returns credit, like RawRecv.
281+
func TestStream_MsgRecvEmitsGrant(t *testing.T) {
282+
mw, frames := captureWriter(t)
283+
st := NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10})
284+
st.recvw = newRecvWindow(100, 50)
285+
286+
// 120 bytes buffered >= high-water 100 -> withheld on dispatch.
287+
assert.NoError(t, st.HandleFrame(msgFrame(st.ID(), 1, make([]byte, 120), true)))
288+
assertNoFrame(t, frames)
289+
290+
var msg []byte
291+
assert.NoError(t, st.MsgRecv(&msg, rawEnc{}))
292+
assert.Equal(t, len(msg), 120)
293+
294+
_, delta, ok := drpcwire.ParseWindowUpdate(waitFrame(t, frames))
295+
assert.That(t, ok)
296+
assert.Equal(t, delta, uint64(120))
297+
}

0 commit comments

Comments
 (0)