Why is there a race condition in this program?

Question

I'm looking at the typical data races in the Golang documentation, and I don't quite understand why there is a problem with this program:

func main() {
    var wg sync.WaitGroup
    wg.Add(5)
    for i := 0; i < 5; i++ {
        go func() {
            fmt.Println(i) // Not the 'i' you are looking for.
            wg.Done()
        }()
    }
    wg.Wait()
}

It prints 5, 5, 5, 5, 5 when I would expect it to print 0, 1, 2, 3, 4 (not necessarily in this order).

The way I see it, when the goroutine gets created inside the loop, the value of i is known (for instance, one could do a log.Println(i) at the beginning of the loop and see the expected value). So I would expect the goroutine to capture the value of i when it gets created and use that later on.

Obviously it's not what's happening but why?

Answer 1

Your function literal references the i from the outer scope. If you request the value of i, you get the value of whatever i is right now. In order to use the value of i at the time the Go routine was created, supply an argument:

func main() {
    var wg sync.WaitGroup
    wg.Add(5)
    for i := 0; i < 5; i++ {
        go func(i int) {
            fmt.Println(i)
            wg.Done()
        }(i)
    }
    wg.Wait()
}

runnable example

Answer 2

The variable i is not declared within the function literal, so it becomes part of a closure. An easy way how to understand closures is to think about how can they be implemented. The simple solution is using a pointer. You can think that the function literal is rewritten by the compiler into some

func f123(i *int) {
        fmt.Println(*i)
        wg.Done            
}

• On invocation of this function, by the go statement, the address of the i variable is passed to the called f123 (example name generated by the compiler).

• You're probably using default GOMAXPROCS==1, so the for loop executes 5 times without any scheduling as the loop does no I/O or other "schedule points", such as channel operations.

• When the loop terminates, with i == 5, the wg.Wait finally triggers execution of the five, ready to run, goroutines (for f123). All of them have of course the same pointer to the same integer variable i.

• Every goroutine now sees the same i value of 5.

You might get different output when running with GOMAXPROCS > 1, or when the loop yields control. That can be done also by, for example, runtime.Gosched.

Answer 3

As mentioned by others, your variable i is used inside the goroutines that you're created, but those goroutines could execute way in the future, once your loop is already done looping. At this point, the value of i is not 5, and all your go routines get kicked up, read the value of i (as 5) and continue on their merry way.

I believe FUZxxl mentioned the use of passing the value i as an argument to the function. I think this is a good idea for rather complicated systems, specially if the function you're kicking a go routine for is not an inline closure. However, in most cases, I think it's a lot cleaner to just create a new temporary variable for each go routine:

http://play.golang.org/p/6dnkrEGfhn

func main() {
    var wg sync.WaitGroup
    wg.Add(5)
    for i := 0; i < 5; i++ {
        myi := i
        go func() {
            fmt.Println(myi)
            wg.Done()
        }()
    }
    wg.Wait()
}

The effect is the same, and it could be argued that it's a matter of preference, and it is. This is my preference :p