I’ve encountered bugs using pointers inside a range
loop twice in the past few weeks. It seems like an easy/common mistake that is worth sharing.
an example
In this example a producer tries to pass pointers across a channel to a consumer.
package main
import "fmt"
func main() {
input := [5]int{1, 2, 3, 4, 5}
c := make(chan *int, 0)
// producer
go func() {
for _, val := range input {
c <- &val
}
close(c)
}()
// consumer
for val := range c {
fmt.Println(*val)
}
}
Output:
2
2
4
4
5
That’s not right! We can see what went wrong by printing the pointers:
fmt.Println(val)
Output:
0x1043617c
0x1043617c
0x1043617c
0x1043617c
0x1043617c
the problem
D’oh - In the producer val
is allocated once at the beginning of the for
loop and we pass the same pointer across the channel for each loop iteration.
The other time I ran into this bug was a struct conversion function similar to this:
package main
import "fmt"
type Query struct {
Ids []int
}
func main() {
// initial object
q := Query{}
q.Ids = []int{1,2}
fmt.Println("object:")
for _, val := range q.Ids {
fmt.Println(val)
}
// translate object
params := make([]*int, len(q.Ids))
for i, id := range q.Ids {
params[i] = &id
}
fmt.Println("translated:")
for _,val := range params {
fmt.Println(*val)
}
}
Output:
object:
1
2
translated:
2
2
solutions
A quick workaround is to use a pointer to a new location in memory:
for i, id := range q.Ids {
temp := id
params[i] = &temp
}
A new temp
variable is allocated for each iteration of the loop, ensuring that each params
index points to a different location in memory. While this works it has two problems:
- It negates the benefits of passing pointers since it allocates and copies each
val
. - It’s leaky. The producer’s local variables are referenced within the consumer’s scope.
In C, one expects the temp
variable to be on the stack. Once the function exits you have no guarantees about the values at those locations. This isn’t a problem in Go because:
Each variable in Go exists as long as there are references to it. The storage location chosen by the implementation is irrelevant to the semantics of the language. - golang faq
This may work but it can be difficult to reason about a function whose local variables exist after the function is no longer within scope.
A better solution is to reference the struct’s memory instead of creating a new local variable.
for i, _ := range q.Ids {
params[i] = &q.Ids[i]
}
This solution does not allocate extra memory, but it can still be leaky depending on where q
is defined.
Finally, depending on your problem, it’s probably best to just pass values:
c := make(chan int, 0)
go func() {
for _, val := range results {
c <- val
}
close(c)
}()
I believe this is the clearest solution. There is no mucking with pointers and it does not leak variables outside of the function where they are defined. The extra memory allocation can be dealt once profiling shows that it is an actual problem.
Hopefully that was informative and helps you avoid this problem in your own code.