Skip to content

Commit 0e9f874

Browse files
okatu-loliclaude
andcommitted
fix(net): synchronize Buf.Close with Write/Read to prevent nil-pointer panic
internal/net.Buf had Close() nilling the underlying bytes.Buffer without taking the rw mutex, racing against concurrent Buf.Write() / Buf.Read() calls. When the consumer of MultiReadCloser closes mid-download (via downloader.interrupt()), in-flight chunk-download goroutines could dereference a nil bytes.Buffer and panic the entire process — matching the SIGSEGV stack reported in #9190 and one crash mode from #9537. Make Close acquire the lock and have Read/Write return io.ErrClosedPipe when the buffer has been nilled, instead of panicking. Add a race-detector regression test (TestBufCloseWriteRace) that panics many times without this fix and passes cleanly with it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent f4445c5 commit 0e9f874

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

internal/net/buf_race_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package net
2+
3+
import (
4+
"context"
5+
"sync"
6+
"testing"
7+
)
8+
9+
// TestBufCloseWriteRace exercises the race fixed for issue #9537/#9190:
10+
// Buf.Close() must synchronize with concurrent Buf.Write() calls so that
11+
// nilling the underlying bytes.Buffer cannot panic an in-flight writer.
12+
// Run with `go test -race ./internal/net/...` to be meaningful.
13+
func TestBufCloseWriteRace(t *testing.T) {
14+
const iters = 200
15+
const writers = 8
16+
17+
for i := 0; i < iters; i++ {
18+
buf := NewBuf(context.Background(), 1024)
19+
var wg sync.WaitGroup
20+
for w := 0; w < writers; w++ {
21+
wg.Add(1)
22+
go func() {
23+
defer wg.Done()
24+
defer func() {
25+
if r := recover(); r != nil {
26+
t.Errorf("panic in Write: %v", r)
27+
}
28+
}()
29+
for j := 0; j < 50; j++ {
30+
_, _ = buf.Write([]byte("x"))
31+
}
32+
}()
33+
}
34+
go func() {
35+
defer func() {
36+
if r := recover(); r != nil {
37+
t.Errorf("panic in Close: %v", r)
38+
}
39+
}()
40+
buf.Close()
41+
}()
42+
wg.Wait()
43+
}
44+
}

internal/net/request.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,10 @@ func (br *Buf) Read(p []byte) (n int, err error) {
643643
return 0, io.EOF
644644
}
645645
br.rw.Lock()
646+
if br.buffer == nil {
647+
br.rw.Unlock()
648+
return 0, io.ErrClosedPipe
649+
}
646650
n, err = br.buffer.Read(p)
647651
br.rw.Unlock()
648652
if err == nil {
@@ -672,10 +676,15 @@ func (br *Buf) Write(p []byte) (n int, err error) {
672676
}
673677
br.rw.Lock()
674678
defer br.rw.Unlock()
679+
if br.buffer == nil {
680+
return 0, io.ErrClosedPipe
681+
}
675682
n, err = br.buffer.Write(p)
676683
return
677684
}
678685

679686
func (br *Buf) Close() {
687+
br.rw.Lock()
688+
defer br.rw.Unlock()
680689
br.buffer = nil
681690
}

0 commit comments

Comments
 (0)