英文:
Why is this function not thread safe in golang?
问题
这是我要翻译的代码：
// 这段代码位于某个具有返回签名 (*Data, error) 的方法中
mapStore := make(...)
resSlice := make(...)
wg := new(sync.WaitGroup)
ec := make(chan error)
for keyString, sliceValue := range myMap {
wg.Add(1)
keyString := keyString
sliceValue := sliceValue
go func() {
err := func(keyString string, sliceValue []Value, wg *sync.WaitGroup) error {
defer wg.Done()
res, err := process(keyString, sliceValue)
if err != nil {
return errors.Wrapf(err, "wrong")
}
if res == nil {
return nil
}
if res.someData != nil {
mapStore[*res.someData] = append(mapStore[*res.someData], res)
return nil
}
resSlice := append(resSlice, res)
return nil
}(keyString, sliceValue, wg)
if err != nil {
ec <- err
return
}
}()
}
}
wg.Wait()
select {
case err := <- ec:
return nil, err
default:
return resSlice, nil
}
有人告诉我这段代码在某些情况下不是线程安全的，但我不确定具体是哪里出了问题。我认为问题可能出在处理 ec 中的 err 上，但我希望能得到一些帮助！
英文:
This is the code that I am mentioning:
// this is inside some method which has return signature like this: (*Data, error)
mapStore := make(...)
resSlice := make(...)
wg := new(sync.WaitGroup)
ec := make(chan error)
for keyString, sliceValue := range myMap {
wg.Add(1)
keyString := keyString
sliceValue := sliceValue
go func() {
err := func(keyString string, sliceValue []Value, wg *sync.WaitGroup) error {
defer wg.Done()
res, err := process(keyString, sliceValue)
if err != nil {
return errors.Wrapf(err, "wrong")
}
if res == nil {
return nil
}
if res.someData != nil {
mapStore[*res.someData] = append(mapStore[*res.someData], res)
return nil
}
resSlice := append(resSlice, res)
return nil
}(keyString, sliceValue, wg)
if err != nil {
ec <- err
return
}
}()
}
}
wg.Wait()
select {
case err := <- ec:
return nil, err
default:
return resSlice, nil
}
I am told that this is not thread safe for some reason but I am not confident as to where. I am thinking its the issue in handling err in ec but would appreciate some help!
答案1
得分: 1
我还没有完全分析所有内容，但是mapStore的多个goroutine修改是不安全的：
mapStore[*res.someData] = append(mapStore[*res.someData], res)
但是作为一个起点，你可以在这段代码下运行竞争检测器，它会为你找到很多问题。
这段代码也明显是不安全的：
resSlice := append(resSlice, res)
但它并不完全做你想要的事情。它创建了一个名为resSlice的新局部变量，遮蔽了外部的变量，但同时也修改了外部的变量（见下面的注释）。除了两个操作可能同时尝试追加并发冲突外，如果append需要重新分配内存，它还可以将整个切片移动到其他位置，因此即使你在其周围加锁，也可能导致线程安全问题。
通常情况下，你不希望每个goroutine都更新某个中央变量，而是希望每个goroutine通过通道将其结果传回。然后，主函数收集所有的值并更新变量。可以参考Go并发模式：管道和取消中的一些示例。
英文:
I haven't analyzed all of it, but definitely the modification of mapStore from multiple goroutines is unsafe:
mapStore[*res.someData] = append(mapStore[*res.someData], res)
But as a starting point, run this under the race detector. It'll find many problems for you.
This is also clearly unsafe:
resSlice := append(resSlice, res)
But it also doesn't quite do what you think. This creates a new local variable called resSlice that shadows the outer one, but it also modifies the outer one (see comments below). Beyond the fact that two things may try to append at the same time and collide, append can move the whole slice in memory if it needs to reallocate, so this can cause thread-safety issues even if you put locks around it.
Typically, rather than having each goroutine update some central variable, you want to have each goroutine pass its results back on a channel. Then have the main function collect all the values and update the variables. See Go Concurrency Patterns: Pipelines and cancellation for some examples.
通过集体智慧和协作来改善编程学习和解决问题的方式。致力于成为全球开发者共同参与的知识库，让每个人都能够通过互相帮助和分享经验来进步。
评论