go vet range variable captured by func literal when using go routine inside of for each loop

huangapple go评论94阅读模式
英文:

go vet range variable captured by func literal when using go routine inside of for each loop

问题

我不太确定"func literal"是什么意思,所以这个错误有点让我困惑。我想我找到了问题所在 - 我在一个新的go协程中引用了一个范围值变量,因此该值可能随时改变,不符合我们的预期。解决这个问题的最佳方法是什么?

有问题的代码:

func (l *Loader) StartAsynchronous() []LoaderProcess {
    for _, currentProcess := range l.processes {
        cmd := exec.Command(currentProcess.Command, currentProcess.Arguments...)
        log.LogMessage("Asynchronously executing LoaderProcess: %+v", currentProcess)
        go func() {
            output, err := cmd.CombinedOutput()
            if err != nil {
                log.LogMessage("LoaderProcess exited with error status: %+v\n %v", currentProcess, err.Error())
            } else {
                log.LogMessage("LoaderProcess exited successfully: %+v", currentProcess)
                currentProcess.Log.LogMessage(string(output))
            }
            time.Sleep(time.Second * TIME_BETWEEN_SUCCESSIVE_ITERATIONS)
        }()
    }
    return l.processes
}

我提出的修复方法:

func (l *Loader) StartAsynchronous() []LoaderProcess {
    for _, currentProcess := range l.processes {
        cmd := exec.Command(currentProcess.Command, currentProcess.Arguments...)
        log.LogMessage("Asynchronously executing LoaderProcess: %+v", currentProcess)
        localProcess := currentProcess
        go func() {
            output, err := cmd.CombinedOutput()
            if err != nil {
                log.LogMessage("LoaderProcess exited with error status: %+v\n %v", localProcess, err.Error())
            } else {
                log.LogMessage("LoaderProcess exited successfully: %+v", localProcess)
                localProcess.Log.LogMessage(string(output))
            }
            time.Sleep(time.Second * TIME_BETWEEN_SUCCESSIVE_ITERATIONS)
        }()
    }
    return l.processes
}

但这样真的解决了问题吗?我只是将引用从范围变量移动到了一个不同的局部变量,其值基于我所在的for循环的迭代。

英文:

I'm not really sure what a 'func literal' is thus this error is confusing me a bit. I think I see the issue - I'm referencing a range value variable from inside a new go routine thus the value might change at anytime and not be what we expect. What's the best way to solve the issue?

The code in question:

func (l *Loader) StartAsynchronous() []LoaderProcess {
	for _, currentProcess := range l.processes {
		cmd := exec.Command(currentProcess.Command, currentProcess.Arguments...)
		log.LogMessage("Asynchronously executing LoaderProcess: %+v", currentProcess)
		go func() {
			output, err := cmd.CombinedOutput()
			if err != nil {
				log.LogMessage("LoaderProcess exited with error status: %+v\n %v", currentProcess, err.Error())
			} else {
				log.LogMessage("LoaderProcess exited successfully: %+v", currentProcess)
				currentProcess.Log.LogMessage(string(output))
			}
			time.Sleep(time.Second * TIME_BETWEEN_SUCCESSIVE_ITERATIONS)
		}()
	}
	return l.processes
}

My proposed fix:

func (l *Loader) StartAsynchronous() []LoaderProcess {
    for _, currentProcess := range l.processes {
        cmd := exec.Command(currentProcess.Command, currentProcess.Arguments...)
        log.LogMessage("Asynchronously executing LoaderProcess: %+v", currentProcess)
        localProcess := currentProcess
        go func() {
            output, err := cmd.CombinedOutput()
            if err != nil {
                log.LogMessage("LoaderProcess exited with error status: %+v\n %v", localProcess, err.Error())
            } else {
                log.LogMessage("LoaderProcess exited successfully: %+v", localProcess)
                localProcess.Log.LogMessage(string(output))
            }
            time.Sleep(time.Second * TIME_BETWEEN_SUCCESSIVE_ITERATIONS)
        }()
    }
    return l.processes
} 

But does that really solve the problem? I've just moved the reference from the range variable to a different local variable whose value is based off of the iteration of the for each loop that I'm in.

答案1

得分: 91

不要担心,对于Go的新手来说,这是一个常见的错误。是的,变量currentProcess在每次循环中都会发生变化,所以你的goroutine将使用切片_l.processes_中的最后一个进程。你只需要将变量作为参数传递给匿名函数,像这样:

func (l *Loader) StartAsynchronous() []LoaderProcess {

    for ix := range l.processes {

        go func(currentProcess *LoaderProcess) {

            cmd := exec.Command(currentProcess.Command, currentProcess.Arguments...)
            log.LogMessage("Asynchronously executing LoaderProcess: %+v", currentProcess)

            output, err := cmd.CombinedOutput()
            if err != nil {
                log.LogMessage("LoaderProcess exited with error status: %+v\n %v", currentProcess, err.Error())
            } else {
                log.LogMessage("LoaderProcess exited successfully: %+v", currentProcess)
                currentProcess.Log.LogMessage(string(output))
            }

            time.Sleep(time.Second * TIME_BETWEEN_SUCCESSIVE_ITERATIONS)

        }(&l.processes[ix]) // 使用索引传递当前进程

    }

    return l.processes
}
英文:

Don't feel bad it's a common mistake for new comers in Go, and yes the var currentProcess changes for each loop, so your goroutines will use the last process in the slice l.processes, all you have to do is pass the variable as a parameter to the anonymous function, like this:

func (l *Loader) StartAsynchronous() []LoaderProcess {

	for ix := range l.processes {

		go func(currentProcess *LoaderProcess) {

			cmd := exec.Command(currentProcess.Command, currentProcess.Arguments...)
			log.LogMessage("Asynchronously executing LoaderProcess: %+v", currentProcess)

			output, err := cmd.CombinedOutput()
			if err != nil {
				log.LogMessage("LoaderProcess exited with error status: %+v\n %v", currentProcess, err.Error())
			} else {
				log.LogMessage("LoaderProcess exited successfully: %+v", currentProcess)
				currentProcess.Log.LogMessage(string(output))
			}

			time.Sleep(time.Second * TIME_BETWEEN_SUCCESSIVE_ITERATIONS)

		}(&l.processes[ix]) // passing the current process using index

	}

	return l.processes
}

答案2

得分: 59

对于那些寻找更简单示例的人:

这是错误的:

func main() {
  for i:=0; i<10; i++{
    go func(){

        // 这里的 i 是一个“自由”变量,因为它没有被声明为 func 字面量的显式参数,
        // 所以它不会被按值复制,正如人们可能推断的那样。相反,
        // 在所有的 go 协程执行时,使用的是“当前”的 i 值
        // (在大多数情况下是循环的最后一个值)。

        processValue(i)

    }()
  }
}

func processValue(i int){
  fmt.Println(i)
}

这并不是一个错误,但可能会导致意外的行为,因为控制循环的变量 i 可能会被其他 go 协程更改。实际上是 go vet 命令 对此发出警告。Go vet 帮助准确地找到这种可疑的结构,它“使用启发式方法,不能保证所有报告都是真正的问题,但它可以找到编译器未捕获的错误”。因此,定期运行它是一个好习惯。

Go Playground 在运行代码之前会运行 go vet,你可以在这里看到它的效果 here

这是正确的:

func main() {
  for i:=0; i<10; i++{
    go func(differentI int){

        processValue(differentI)

    }(i) // 这里 i 被按值传递,因为它被声明为 func 字面量的显式参数,
         // 并且对于每个 go 协程,它都被视为一个不同的“differentI”,
         // 无论 go 协程何时执行,独立于 i 的当前值。

  }
}

func processValue(i int){
  fmt.Println(i)
}

我故意将 func 字面量的参数命名为 differentI,以明确它是一个不同的变量。以这种方式进行并发使用是安全的,go vet 不会抱怨,你也不会遇到奇怪的行为。你可以在这里看到它的效果 here。(你将看不到任何输出,因为打印是在不同的 go 协程上进行的,但程序将成功退出)

顺便说一下,func 字面量基本上是一个匿名函数 :)。

英文:

For those looking for a simpler example:

This is wrong:

func main() {
  for i:=0; i&lt;10; i++{
	go func(){

        // Here i is a &quot;free&quot; variable, since it wasn&#39;t declared
        // as an explicit parameter of the func literal, 
        // so IT&#39;S NOT copied by value as one may infer. Instead,
        // the &quot;current&quot; value of i
        // (in most cases the last value of the loop) is used
        // in all the go routines once they are executed.

		processValue(i)

	}()
  }
}

func processValue(i int){
  fmt.Println(i)
}

Is not exactly an error but could lead to unexpected behavior since the variable i, which controls your loop could be changed from other go routine. It's actually go vet command which alerts about this. Go vet helps precisely to find this kind of suspicious constructs, it uses heuristics that do not guarantee all reports are genuine problems, but it can find errors not caught by the compilers. So it's a good practice to run it from time to time.

Go Playground runs go vet before running the code, you can see that in action here.

This is correct:

func main() {
  for i:=0; i&lt;10; i++{
	go func(differentI int){

		processValue(differentI)

	}(i) // Here i is effectively passed by value since it was
         // declared as an explicit parameter of the func literal
         // and is taken as a different &quot;differentI&quot; for each
         // go routine, no matter when the go routine is executed
         // and independently of the current value of i.
  }
}

func processValue(i int){
  fmt.Println(i)
}

I intentionally named the func literal parameter differentI to make it obvious that it is a different variable. Doing it that way is safe for concurrent use, go vet won't complain and you'll get no weird behaviors. You can see that in action here. (You will see nothing since the printing is done on different go routines but the program will exit successfully)

And by the way, a func literal is basically an anonymous function go vet range variable captured by func literal when using go routine inside of for each loop

答案3

得分: 5

是的,你所做的是修复这个警告的最简单方法。

在修复之前,只有一个变量,并且所有的goroutine都引用它。这意味着它们看到的值不是它们启动时的值,而是当前的值。在大多数情况下,这是范围中的最后一个值。

英文:

Yes, what you did is the simplest way of fixing this warning properly.

Before the fix, there was only a single variable, and all goroutines were referring to it. This means they did not see the value from when they started but the current value. In most cases this is the last one from the range.

huangapple
  • 本文由 发表于 2016年10月30日 14:56:27
  • 转载请务必保留本文链接:https://go.coder-hub.com/40326723.html
匿名

发表评论

匿名网友

:?: :razz: :sad: :evil: :!: :smile: :oops: :grin: :eek: :shock: :???: :cool: :lol: :mad: :twisted: :roll: :wink: :idea: :arrow: :neutral: :cry: :mrgreen:

确定