3 ms·
One of them has to be the for loop reusing value thing. func processItem(item *string) { if item == nil { fmt.Println("nil item") ret
by TheDong 6y ago
One of them has to be the for loop reusing value thing.
func processItem(item *string) {
if item == nil {
fmt.Println("nil item")
return
}
fmt.Println(*item)
}
stuffToProcess := []string{"one", "two"}
wg := sync.WaitGroup{}
for _, item := range stuffToProcess {
wg.Add(1)
go func(s *string) {
time.Sleep(1 * time.Millisecond)
processItem(s)
wg.Done()
}(&item)
}
wg.Wait()
// What do I print?
Playground here: https://play.golang.org/p/mouvT1BkpNJ https://play.golang.org/p/mouvT1BkpNJ
- mimischi 6y agoCan‘t this be fixed by changing bits to be passed by value? https://play.golang.org/p/srAyKZN2NTH https://play.golang.org/p/srAyKZN2NTH
- thwarted 6y agoYes, it is odd to pass the address of a local variable from the enclosing scope like this to a go routine.
- masklinn 6y agoWell that's true in the sense that usually you'd just close over it, which is the classic broken case of closing over a mutable binding: https://play.golang.org/p/40FRmvhifBl https://play.golang.org/p/40FRmvhifBl I assume GGP's contrived example is to show the underlying mechanics of the issue.
- zimpenfish 6y agogosec[1] will warn you about this, at least. [reuse.go:26] - G601 (CWE-118): Implicit memory aliasing in for loop. (Confidence: MEDIUM, Severity: MEDIUM) 25: wg.Done() > 26: }(&item) 27: } [1] https://github.com/securego/gosec https://github.com/securego/gosec
- JulianMorrison 6y agoEasily enough fixed by item := item (that is, shadow the variable with something new that won't be overwritten)