Skip to content

Commit 7d03098

Browse files
drpcstream: gate data-frame sends on the per-stream send window
Wire the sendWindow credit gate into rawWriteLocked: a KindMessage frame now acquires len(frame) bytes of per-stream send credit before it is handed to the writer. Control frames (invoke/metadata) bypass the gate, and write stats are recorded only after credit is acquired, next to the WriteFrame they describe. The window is opt-in: a stream has no send window by default, so data writes stay ungated (unlimited) and behavior is unchanged until one is installed. terminate closes the window with the send-side error (sigs.send is first-wins, holding io.EOF when a cancel/error path pre-set it), so a send parked on credit returns the same error as one parked in WriteFrame or a later send. Close and SendError close the send window before touching the write lock, so a send parked on credit wakes and releases the lock instead of stalling them on a grant that may never come. SendError pre-closes with io.EOF to match the send signal it sets moments later; Close pre-closes with termClosed for the same reason. CloseSend is a graceful half-close, so it waits for a parked send to complete rather than aborting it. All three (Close, SendError, CloseSend) wait for the write lock without holding s.mu and re-check stream state after acquiring it: the parked writer is only freed by terminate, whose paths (Cancel/Close/SendError/inbound terminal frames) all need s.mu -- and a uniform write-then-mu order is also what keeps CloseSend and Close/SendError from ABBA-deadlocking against each other. Per-stream only; the connection-level window and the enablement path that installs the window come in later commits. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
1 parent deefd7b commit 7d03098

2 files changed

Lines changed: 377 additions & 0 deletions

File tree

Lines changed: 313 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,313 @@
1+
// Copyright (C) 2026 Cockroach Labs.
2+
// See LICENSE for copying information.
3+
4+
package drpcstream
5+
6+
import (
7+
"context"
8+
"errors"
9+
"io"
10+
"testing"
11+
"time"
12+
13+
"github.com/zeebo/assert"
14+
"github.com/zeebo/errs"
15+
16+
"storj.io/drpc/drpcwire"
17+
)
18+
19+
// newGateStream builds a stream writing to io.Discard with an explicit
20+
// SplitSize so small payloads are a single frame.
21+
func newGateStream(t *testing.T) *Stream {
22+
mw := testMuxWriter(t)
23+
return NewWithOptions(context.Background(), 1, mw, NewBufferPool(), Options{SplitSize: 64 << 10})
24+
}
25+
26+
// By default no send window is installed, so data writes are ungated
27+
// (unlimited) and behavior is unchanged.
28+
func TestStream_SendWindowDefaultUngated(t *testing.T) {
29+
st := newGateStream(t)
30+
assert.That(t, st.sendw == nil)
31+
assert.NoError(t, st.RawWrite(drpcwire.KindMessage, []byte("hello")))
32+
}
33+
34+
// With a finite send window, a data write blocks until enough credit is
35+
// granted.
36+
func TestStream_SendWindowGatesDataWrite(t *testing.T) {
37+
st := newGateStream(t)
38+
st.sendw = newSendWindow(4) // 4 bytes of credit
39+
40+
done := make(chan error, 1)
41+
go func() { done <- st.RawWrite(drpcwire.KindMessage, []byte("hello")) }() // 5 bytes > 4
42+
43+
select {
44+
case <-done:
45+
t.Fatal("data write returned before sufficient credit")
46+
case <-time.After(blockShort):
47+
}
48+
49+
st.sendw.grant(1) // 4 + 1 = 5 >= 5
50+
51+
select {
52+
case err := <-done:
53+
assert.NoError(t, err)
54+
case <-time.After(time.Second):
55+
t.Fatal("data write did not complete after grant")
56+
}
57+
}
58+
59+
// Control kinds (here, invoke) are not flow-controlled: they proceed even with
60+
// zero send credit.
61+
func TestStream_SendWindowControlKindsBypassGate(t *testing.T) {
62+
st := newGateStream(t)
63+
st.sendw = newSendWindow(0) // no credit at all
64+
65+
assert.NoError(t, st.WriteInvoke("service.Method", nil))
66+
}
67+
68+
// SendCancel preempts a send parked on credit: terminate (which closes the
69+
// window) runs before SendCancel takes the write lock, so the parked write
70+
// wakes, releases the lock, and the cancel frame goes out.
71+
func TestStream_SendWindowSendCancelPreemptsParkedWrite(t *testing.T) {
72+
st := newGateStream(t)
73+
st.sendw = newSendWindow(0) // send will park immediately
74+
75+
done := make(chan error, 1)
76+
go func() { done <- st.RawWrite(drpcwire.KindMessage, []byte("data")) }()
77+
78+
select {
79+
case <-done:
80+
t.Fatal("data write returned before cancel")
81+
case <-time.After(blockShort):
82+
}
83+
84+
assert.NoError(t, st.SendCancel(context.Canceled))
85+
86+
select {
87+
case err := <-done:
88+
// Same error as a send parked in WriteFrame or a later send would see.
89+
assert.That(t, errors.Is(err, io.EOF))
90+
case <-time.After(time.Second):
91+
t.Fatal("parked data write was not preempted by SendCancel")
92+
}
93+
94+
// A subsequent send observes the same error as the parked one.
95+
assert.That(t, errors.Is(st.RawWrite(drpcwire.KindMessage, []byte("more")), io.EOF))
96+
}
97+
98+
// Close preempts a send parked on credit the same way: it closes the send
99+
// window before taking the write lock rather than waiting on a grant that
100+
// may never come.
101+
func TestStream_SendWindowClosePreemptsParkedWrite(t *testing.T) {
102+
st := newGateStream(t)
103+
st.sendw = newSendWindow(0) // send will park immediately
104+
105+
done := make(chan error, 1)
106+
go func() { done <- st.RawWrite(drpcwire.KindMessage, []byte("data")) }()
107+
108+
select {
109+
case <-done:
110+
t.Fatal("data write returned before close")
111+
case <-time.After(blockShort):
112+
}
113+
114+
assert.NoError(t, st.Close())
115+
116+
select {
117+
case err := <-done:
118+
// Same error later sends see: terminate sets sigs.send to termClosed.
119+
assert.That(t, errors.Is(err, termClosed))
120+
case <-time.After(time.Second):
121+
t.Fatal("parked data write was not preempted by Close")
122+
}
123+
}
124+
125+
// SendError preempts a send parked on credit, like Close: it closes the send
126+
// window before taking the write lock so reporting an error is never stuck
127+
// behind a slow consumer.
128+
func TestStream_SendWindowSendErrorPreemptsParkedWrite(t *testing.T) {
129+
st := newGateStream(t)
130+
st.sendw = newSendWindow(0) // send will park immediately
131+
132+
done := make(chan error, 1)
133+
go func() { done <- st.RawWrite(drpcwire.KindMessage, []byte("data")) }()
134+
135+
select {
136+
case <-done:
137+
t.Fatal("data write returned before error")
138+
case <-time.After(blockShort):
139+
}
140+
141+
assert.NoError(t, st.SendError(errs.New("boom")))
142+
143+
select {
144+
case err := <-done:
145+
// io.EOF, matching sigs.send: parked and later sends agree.
146+
assert.That(t, errors.Is(err, io.EOF))
147+
case <-time.After(time.Second):
148+
t.Fatal("parked data write was not preempted by SendError")
149+
}
150+
}
151+
152+
// CloseSend is a graceful half-close, not a termination: it must NOT preempt
153+
// a send parked on credit. It waits for the write lock; once credit arrives
154+
// the parked write completes successfully and CloseSend follows it out.
155+
func TestStream_SendWindowCloseSendWaitsForParkedWrite(t *testing.T) {
156+
st := newGateStream(t)
157+
st.sendw = newSendWindow(0) // send will park immediately
158+
159+
write := make(chan error, 1)
160+
go func() { write <- st.RawWrite(drpcwire.KindMessage, []byte("data")) }()
161+
162+
select {
163+
case <-write:
164+
t.Fatal("data write returned before credit")
165+
case <-time.After(blockShort):
166+
}
167+
168+
closeSend := make(chan error, 1)
169+
go func() { closeSend <- st.CloseSend() }()
170+
171+
// Neither may make progress yet: the write is parked on credit and
172+
// CloseSend is parked behind it on the write lock.
173+
select {
174+
case <-write:
175+
t.Fatal("data write returned without credit")
176+
case <-closeSend:
177+
t.Fatal("CloseSend preempted a parked data write")
178+
case <-time.After(blockShort):
179+
}
180+
181+
st.sendw.grant(uint64(len("data")))
182+
183+
select {
184+
case err := <-write:
185+
assert.NoError(t, err) // the parked write completed, not aborted
186+
case <-time.After(time.Second):
187+
t.Fatal("parked data write did not complete after grant")
188+
}
189+
select {
190+
case err := <-closeSend:
191+
assert.NoError(t, err)
192+
case <-time.After(time.Second):
193+
t.Fatal("CloseSend did not complete after the parked write finished")
194+
}
195+
}
196+
197+
// While CloseSend waits behind a credit-parked send, termination must still
198+
// be able to proceed: Cancel needs s.mu to terminate, so CloseSend must not
199+
// hold s.mu while waiting for the write lock.
200+
func TestStream_SendWindowCancelUnwedgesCloseSendBehindParkedWrite(t *testing.T) {
201+
st := newGateStream(t)
202+
st.sendw = newSendWindow(0) // send will park immediately
203+
204+
write := make(chan error, 1)
205+
go func() { write <- st.RawWrite(drpcwire.KindMessage, []byte("data")) }()
206+
207+
select {
208+
case <-write:
209+
t.Fatal("data write returned before credit")
210+
case <-time.After(blockShort):
211+
}
212+
213+
closeSend := make(chan error, 1)
214+
go func() { closeSend <- st.CloseSend() }()
215+
216+
select {
217+
case <-closeSend:
218+
t.Fatal("CloseSend preempted a parked data write")
219+
case <-time.After(blockShort):
220+
}
221+
222+
// Cancel terminates the stream, closing the send window: the parked write
223+
// wakes with an error and CloseSend unblocks as a no-op.
224+
canceled := make(chan struct{})
225+
go func() { st.Cancel(errs.New("boom")); close(canceled) }()
226+
227+
select {
228+
case <-canceled:
229+
case <-time.After(time.Second):
230+
t.Fatal("Cancel blocked behind CloseSend waiting for the write lock")
231+
}
232+
select {
233+
case err := <-write:
234+
assert.That(t, errors.Is(err, io.EOF))
235+
case <-time.After(time.Second):
236+
t.Fatal("parked data write did not wake on termination")
237+
}
238+
select {
239+
case err := <-closeSend:
240+
assert.NoError(t, err)
241+
case <-time.After(time.Second):
242+
t.Fatal("CloseSend did not unblock after termination")
243+
}
244+
}
245+
246+
// CloseSend acquires the write lock before s.mu; Close and SendError must
247+
// use the same order (never holding s.mu while waiting for the write lock),
248+
// or the two paths ABBA-deadlock racing behind a credit-parked send.
249+
func TestStream_SendWindowCloseSendVsTerminatorNoDeadlock(t *testing.T) {
250+
for _, tc := range []struct {
251+
name string
252+
call func(*Stream) error
253+
}{
254+
{"Close", func(st *Stream) error { return st.Close() }},
255+
{"SendError", func(st *Stream) error { return st.SendError(errs.New("boom")) }},
256+
} {
257+
t.Run(tc.name, func(t *testing.T) {
258+
st := newGateStream(t)
259+
st.sendw = newSendWindow(0) // send will park immediately
260+
261+
write := make(chan error, 1)
262+
go func() { write <- st.RawWrite(drpcwire.KindMessage, []byte("data")) }()
263+
264+
select {
265+
case <-write:
266+
t.Fatal("data write returned before credit")
267+
case <-time.After(blockShort):
268+
}
269+
270+
closeSend := make(chan error, 1)
271+
terminator := make(chan error, 1)
272+
go func() { closeSend <- st.CloseSend() }()
273+
go func() { terminator <- tc.call(st) }()
274+
275+
// All three must resolve; the interesting interleaving is CloseSend
276+
// winning the write lock while the terminator wins s.mu.
277+
for name, ch := range map[string]chan error{
278+
"parked write": write, "CloseSend": closeSend, tc.name: terminator,
279+
} {
280+
select {
281+
case <-ch:
282+
case <-time.After(2 * time.Second):
283+
t.Fatalf("%s deadlocked", name)
284+
}
285+
}
286+
})
287+
}
288+
}
289+
290+
// Terminating the stream wakes a send parked on credit.
291+
func TestStream_SendWindowTerminateWakesParkedWrite(t *testing.T) {
292+
st := newGateStream(t)
293+
st.sendw = newSendWindow(0) // send will park immediately
294+
295+
done := make(chan error, 1)
296+
go func() { done <- st.RawWrite(drpcwire.KindMessage, []byte("data")) }()
297+
298+
select {
299+
case <-done:
300+
t.Fatal("data write returned before termination")
301+
case <-time.After(blockShort):
302+
}
303+
304+
st.Cancel(errs.New("boom"))
305+
306+
select {
307+
case err := <-done:
308+
// Cancel pre-sets sigs.send to io.EOF; the window closes with it.
309+
assert.That(t, errors.Is(err, io.EOF))
310+
case <-time.After(time.Second):
311+
t.Fatal("parked data write did not wake on termination")
312+
}
313+
}

0 commit comments

Comments
 (0)