Skip to content

Commit bbdd0fb

Browse files
tomassrnkaclaude
authored andcommitted
fix: data races in NBD path_direct and PostProcessor test on ARM64
- path_direct.go: capture deviceIndex before goroutine closure. The outer for-loop is not a range loop, so Go 1.22+ loop variable fix doesn't apply — deviceIndex is reassigned on each retry iteration. - postprocessor_test.go: use sync.Mutex-wrapped buffer instead of bare bytes.Buffer. The PostProcessor goroutine and test goroutine write to the buffer concurrently. Both races were exposed by running tests with -race on ARM64 (weaker memory model makes races more likely to manifest). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4f66f81 commit bbdd0fb

2 files changed

Lines changed: 33 additions & 5 deletions

File tree

packages/orchestrator/pkg/sandbox/nbd/path_direct.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func (d *DirectPathMount) Open(ctx context.Context) (retDeviceIndex uint32, err
7878

7979
telemetry.ReportEvent(ctx, "got backend size")
8080

81-
deviceIndex := uint32(math.MaxUint32)
81+
var deviceIndex uint32
8282

8383
for {
8484
deviceIndex, err = d.devicePool.GetDevice(ctx)
@@ -119,13 +119,17 @@ func (d *DirectPathMount) Open(ctx context.Context) (retDeviceIndex uint32, err
119119
server.Close()
120120

121121
dispatch := NewDispatch(serverc, d.Backend)
122+
// Capture deviceIndex for the goroutine closure — it's reassigned on
123+
// each retry iteration of the outer for-loop (not a range loop, so
124+
// Go 1.22+ loop variable fix doesn't apply).
125+
devIdx := deviceIndex
122126
// Start reading commands on the socket and dispatching them to our provider
123127
d.handlersWg.Go(func() {
124128
handleErr := dispatch.Handle(ctx)
125129
// The error is expected to happen if the nbd (socket connection) is closed
126130
logger.L().Info(ctx, "closing handler for NBD commands",
127131
zap.Error(handleErr),
128-
zap.Uint32("device_index", deviceIndex),
132+
zap.Uint32("device_index", devIdx),
129133
zap.Int("socket_index", i),
130134
)
131135
})

packages/orchestrator/pkg/template/build/writer/postprocessor_test.go

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package writer
22

33
import (
44
"bytes"
5+
"sync"
56
"testing"
67
"time"
78

@@ -12,14 +13,37 @@ import (
1213
"github.com/e2b-dev/infra/packages/shared/pkg/logger"
1314
)
1415

15-
func newTestCore(buf *bytes.Buffer) zapcore.Core {
16+
// syncBuffer wraps bytes.Buffer with a mutex for concurrent writes from
17+
// the PostProcessor goroutine and the test goroutine.
18+
type syncBuffer struct {
19+
mu sync.Mutex
20+
buf bytes.Buffer
21+
}
22+
23+
func (b *syncBuffer) Write(p []byte) (int, error) {
24+
b.mu.Lock()
25+
defer b.mu.Unlock()
26+
27+
return b.buf.Write(p)
28+
}
29+
30+
func (b *syncBuffer) Sync() error { return nil }
31+
32+
func (b *syncBuffer) String() string {
33+
b.mu.Lock()
34+
defer b.mu.Unlock()
35+
36+
return b.buf.String()
37+
}
38+
39+
func newTestCore(buf *syncBuffer) zapcore.Core {
1640
encoderCfg := zap.NewDevelopmentEncoderConfig()
1741
encoderCfg.TimeKey = ""
1842
encoder := zapcore.NewConsoleEncoder(encoderCfg)
1943

2044
core := zapcore.NewCore(
2145
encoder,
22-
zapcore.AddSync(buf),
46+
buf,
2347
zapcore.DebugLevel,
2448
)
2549

@@ -29,7 +53,7 @@ func newTestCore(buf *bytes.Buffer) zapcore.Core {
2953
func TestPostProcessor_Start(t *testing.T) {
3054
t.Parallel()
3155
ctx := t.Context()
32-
var buf bytes.Buffer
56+
var buf syncBuffer
3357
core := newTestCore(&buf)
3458

3559
interval := time.Millisecond * 100

0 commit comments

Comments
 (0)