Golang Don’t pass a value to a callback in a loop with range

range keyword is often used when a variable needs to be iterated over the values. It’s easy to use and we don’t have to care about anything in most cases. However, it could introduce a bug if the value is used in a callback function which is triggered later.

Emitter struct to emulate subscription

There are some modules that has Subscribe method. It requires a callback function that will be triggered later when a condition is fulfilled.

Assume that our module connects to another system and watches a value. The value in the system changes periodically and we need to get the new value. It’s ok to poll the value but it’s better if the system notifies the value change. Many systems implement such a mechanism and provide a method to subscribe to the data change.

I defined the following struct to pretend such a system.

type Emitter struct {
    callbacks map[string]func()
}

func (e *Emitter) Subscribe(key string, cb func()) {
    if len(e.callbacks) == 0 {
        e.callbacks = map[string]func(){key: cb}
    } else {
        e.callbacks[key] = cb
    }
}

func (e *Emitter) Fire(key string) {
    action, prs := e.callbacks[key]
    if prs {
        action()
    }
}

When Subscribe method is called, it just stores the specified callback to the map with key. The callback is triggered when Fire method is called.

The pointer passed by range holds the last pointer

The following code contains a bug. This is the main topic of this post.

It’s simple code. The index and value are passed to a callback function. We expect that a different combination of index and value is shown on each Fire method call.

func loopIntArray() {
    fmt.Println("--- loopIntArray ---")
    emitter := Emitter{}
    one := 1
    two := 2
    three := 3
    intArray := []int{one, two, three}

    fmt.Println("--- range --- ")
    for index, value := range intArray {
        fmt.Printf("%d: %v\n", index, value)
        emitter.Subscribe(fmt.Sprint(index), func() {
            fmt.Printf("index: %d, value: %d\n", index, value)
        })
    }

    fmt.Println("--- fire --- ")
    emitter.Fire("0")
    emitter.Fire("1")
    emitter.Fire("2")
}

This is the result of the code above.

--- loopIntArray ---
--- range --- 
0: 1
1: 2
2: 3
--- fire --- 
index: 2, value: 3
index: 2, value: 3
index: 2, value: 3

Wow. The combination of index and value in the for-loop is as expected but not on Fire method call. The index and value are the same for all Fire method call!!

It seems that range keyword takes the next value pointer and holds it. So, the last pointer is always used when the value is accessed.

Use a new variable to hold the target pointer

We must copy the pointer of the value if the pointer is moving. I added indexCopy and valueCopy. The index and value are not pointers, so it’s actually a data copy. Then, the copied value is correctly used in the callback.

func loopIntArray2() {
    fmt.Println("--- loopIntArray ---")
    emitter := Emitter{}
    one := 1
    two := 2
    three := 3
    intArray := []int{one, two, three}

    fmt.Println("--- range --- ")
    for index, value := range intArray {
        fmt.Printf("%d: %v\n", index, value)
        // 0: 1
        // 1: 2
        // 2: 3
        indexCopy := index
        valueCopy := value
        emitter.Subscribe(fmt.Sprint(index), func() {
            fmt.Printf("index: %d, value: %d\n", indexCopy, valueCopy)
        })
    }

    fmt.Println("--- fire --- ")
    emitter.Fire("0") // index: 0, value: 1
    emitter.Fire("1") // index: 1, value: 2
    emitter.Fire("2") // index: 2, value: 3
}

It becomes the expected result.

What if using struct instead of literal value?

func loopStructArray() {
    fmt.Println("--- loopIntPointerArray ---")
    emitter := Emitter{}
    book1 := BaseProductInfo{Name: "name-1", Price: 11}
    book2 := BaseProductInfo{Name: "name-2", Price: 22}
    book3 := BaseProductInfo{Name: "name-3", Price: 33}

    structArray := []BaseProductInfo{book1, book2, book3}

    fmt.Println("--- range --- ")
    for index, value := range structArray {
        fmt.Printf("%d: %+v\n", index, value)
        // 0: {Name:name-1 Price:11}
        // 1: {Name:name-2 Price:22}
        // 2: {Name:name-3 Price:33}

        indexCopy := index
        valueCopy := value
        emitter.Subscribe(fmt.Sprint(index), func() {
            fmt.Printf("index: %d, value: %+v\n", indexCopy, valueCopy)
        })
    }

    fmt.Println("--- fire --- ")
    emitter.Fire("0") // index: 0, value: {Name:name-1 Price:11}
    emitter.Fire("1") // index: 1, value: {Name:name-2 Price:22}
    emitter.Fire("2") // index: 2, value: {Name:name-3 Price:33}
}

The proper struct is used as expected.

Be careful when the value is directly passed to a callback with range keyword. It can introduce a bug if the callback is not triggered immediately.