Skip to content

Commit 7046b84

Browse files
committed
Stream processor drops completion marker
1 parent eeea34f commit 7046b84

3 files changed

Lines changed: 89 additions & 6 deletions

File tree

pkg/fileutils/tarxfer_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,92 @@ func TestReceiver_Receive_Success(t *testing.T) {
114114
}
115115
}
116116

117+
func TestReceiver_Receive_OverflowsDemuxChannel(t *testing.T) {
118+
// Regression: a context transfer that produces more BuildTransfer
119+
// packets than the demux channel can hold must not drop the
120+
// complete=true marker. Accept must apply backpressure rather
121+
// than drop. Without that, the receiver hung forever waiting on
122+
// a packet that would never arrive.
123+
archive, err := makeTar()
124+
if err != nil {
125+
t.Fatalf("makeTar: %v", err)
126+
}
127+
if len(archive) < 512 {
128+
t.Fatalf("tar archive too small: %d", len(archive))
129+
}
130+
hashBytes := sha256.Sum256(archive)
131+
hash := hex.EncodeToString(hashBytes[:])
132+
header := archive[:512]
133+
body := archive[512:]
134+
135+
// Split the body into enough chunks to exceed the demux channel
136+
// capacity, forcing the producer to wait on backpressure.
137+
const chunkSize = 16
138+
chunks := make([][]byte, 0, len(body)/chunkSize+1)
139+
for i := 0; i < len(body); i += chunkSize {
140+
end := i + chunkSize
141+
if end > len(body) {
142+
end = len(body)
143+
}
144+
chunks = append(chunks, body[i:end])
145+
}
146+
for len(chunks) < 64 {
147+
chunks = append(chunks, []byte{})
148+
}
149+
150+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
151+
defer cancel()
152+
demux := newDemux(ctx)
153+
154+
producerErr := make(chan error, 1)
155+
go func() {
156+
defer close(producerErr)
157+
if err := demux.Accept(btPacket(nil, false, map[string]string{"hash": hash})); err != nil {
158+
producerErr <- err
159+
return
160+
}
161+
if err := demux.Accept(btPacket(header, false, nil)); err != nil {
162+
producerErr <- err
163+
return
164+
}
165+
for i, chunk := range chunks {
166+
isLast := i == len(chunks)-1
167+
if err := demux.Accept(btPacket(chunk, isLast, nil)); err != nil {
168+
producerErr <- err
169+
return
170+
}
171+
}
172+
}()
173+
174+
tmpDir := t.TempDir()
175+
r := NewTarReceiver(tmpDir, demux)
176+
177+
var visited []string
178+
walkFn := func(p string, _ fs.DirEntry, _ error) error {
179+
visited = append(visited, p)
180+
return nil
181+
}
182+
183+
start := time.Now()
184+
checksum, err := r.Receive(ctx, []byte{}, []byte{}, walkFn)
185+
elapsed := time.Since(start)
186+
if err != nil {
187+
t.Fatalf("Receive failed after %v: %v", elapsed, err)
188+
}
189+
if err := <-producerErr; err != nil {
190+
t.Fatalf("producer Accept failed: %v", err)
191+
}
192+
if checksum != hash {
193+
t.Fatalf("checksum mismatch: want %s, got %s", hash, checksum)
194+
}
195+
if len(visited) != 1 || visited[0] != "file1" {
196+
t.Fatalf("unexpected visited paths: %v", visited)
197+
}
198+
if fi, err := os.Stat(filepath.Join(tmpDir, checksum, "file1")); err != nil || !fi.Mode().IsRegular() {
199+
t.Fatalf("extracted file missing or not regular: %v", err)
200+
}
201+
}
202+
117203
func TestReceiver_Receive_ServerError(t *testing.T) {
118204
ctx, cancel := context.WithCancel(context.Background())
119205
defer cancel()

pkg/stream/errors.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ var (
2727
ErrNoHandlerFound = errors.New("no handler found for packet")
2828
ErrNotATTY = errors.New("not a tty")
2929
ErrSendStreamBlocked = errors.New("send stream is blocked")
30-
ErrDemuxChannelFull = errors.New("demux channel full")
3130
)
3231

3332
type UninitializedStageErr string

pkg/stream/processor.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ func (d *Demultiplexer) Closed() bool {
5858
return d.ctx.Err() != nil
5959
}
6060

61-
// Accept validates & enqueues a packet.
61+
// Accept enqueues a packet for the demux consumer. The send blocks
62+
// when the channel is full to apply backpressure. Returns the demux
63+
// ctx error if cancelled before the packet is enqueued.
6264
func (d *Demultiplexer) Accept(c *api.ClientStream) error {
6365
if err := d.filter(c); err != nil {
6466
return err
@@ -70,10 +72,6 @@ func (d *Demultiplexer) Accept(c *api.ClientStream) error {
7072
return d.ctx.Err()
7173
case d.ch <- c:
7274
return nil
73-
default:
74-
// Channel is full - clean it up to prevent future packets from being sent here
75-
d.closeFn(d.id)
76-
return ErrDemuxChannelFull
7775
}
7876
}
7977

0 commit comments

Comments
 (0)