Skip to content

Commit da6a600

Browse files
committed
fn: fix loop var capture and clarify naming in ParSliceErrCollect
Ensure each goroutine captures its own index/value in ParSliceErrCollect, and rename variables for clearer intent. Avoids closure capture bug and makes concurrency guards explicit.
1 parent 4ffe39f commit da6a600

File tree

1 file changed

+34
-9
lines changed

1 file changed

+34
-9
lines changed

fn/concurrency.go

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,12 @@ func ParSlice[V any](ctx context.Context, s []V, f ErrFunc[V]) error {
2626
errGroup.SetLimit(runtime.GOMAXPROCS(0))
2727

2828
for _, v := range s {
29+
// Snapshot v now so the goroutine sees the intended value
30+
// even if the caller mutates the slice s later. This is a
31+
// shallow copy only, if V contains pointers, their contents can
32+
// still change.
2933
v := v
34+
3035
errGroup.Go(func() error {
3136
return f(ctx, v)
3237
})
@@ -43,25 +48,46 @@ func ParSlice[V any](ctx context.Context, s []V, f ErrFunc[V]) error {
4348
func ParSliceErrCollect[V any](ctx context.Context, s []V,
4449
f ErrFunc[V]) (map[int]error, error) {
4550

46-
errGroup, ctx := errgroup.WithContext(ctx)
51+
errGroup, groupCtx := errgroup.WithContext(ctx)
4752
errGroup.SetLimit(runtime.GOMAXPROCS(0))
4853

49-
var instanceErrorsMutex sync.Mutex
50-
instanceErrors := make(map[int]error, len(s))
54+
var (
55+
instanceErrorsMu sync.Mutex
56+
instanceErrors = make(map[int]error, len(s))
57+
)
5158

5259
for idx := range s {
60+
// Rebind idx inside the loop so that each goroutine captures
61+
// a distinct copy of the index. Without this, all goroutines
62+
// may observe the same final idx value due to closure capture.
63+
idx := idx
64+
65+
// Snapshot s[idx] now so the goroutine sees the intended value
66+
// even if the caller mutates the slice later. This is a shallow
67+
// copy only, if V contains pointers, their contents can still
68+
// change.
69+
v := s[idx]
70+
5371
errGroup.Go(func() error {
54-
err := f(ctx, s[idx])
72+
// If already canceled, skip work without signaling a
73+
// error.
74+
select {
75+
case <-groupCtx.Done():
76+
return nil
77+
default:
78+
}
79+
80+
err := f(groupCtx, v)
5581
if err != nil {
56-
instanceErrorsMutex.Lock()
82+
instanceErrorsMu.Lock()
5783
instanceErrors[idx] = err
58-
instanceErrorsMutex.Unlock()
84+
instanceErrorsMu.Unlock()
5985
}
6086

6187
// Avoid returning an error here, as that would cancel
6288
// the errGroup and terminate all slice element
6389
// processing instances. Instead, collect the error and
64-
// return it later.
90+
// return them later.
6591
return nil
6692
})
6793
}
@@ -71,8 +97,7 @@ func ParSliceErrCollect[V any](ctx context.Context, s []V,
7197
// The goroutines that are executing in parallel should not return an
7298
// error, but the Wait call may return an error if the context is
7399
// canceled or timed out.
74-
err := errGroup.Wait()
75-
if err != nil {
100+
if err := errGroup.Wait(); err != nil {
76101
return nil, fmt.Errorf("failed to wait on error group in "+
77102
"ParSliceErrorCollect: %w", err)
78103
}

0 commit comments

Comments
 (0)