Reused Variables in Golang
In general it’s a bad idea to reuse/recycle variables (see: Software Design/Don’t reuse a variable). It makes code less readable, debugging more difficult, and can lead to bad designs.
The following sections will highlight some issues with reused/recycled variables in Go.
range
range is a Go built-in keyword provided to iterate over arrays and slices.
The syntax is shorter than C-style for loops ForStmt = “for” [ Condition | ForClause | RangeClause ] Block .,
but the way it works may be confusing at first.
Check out the following code snippet:
package main
import (
"fmt"
)
type A struct {
ValA int `json:"valA"`
ValB int `json:"valB"`
}
func main() {
input := []A{{ValA: 1},{ValA: 2}}
for i, a := range input {
fmt.Printf("%p\n", &a) // `a` addr
a.ValA *= 10
}
fmt.Println(input)
}
Output:
0xc00018e000
0xc00018e000
[{1 0} {2 0}]
Notice that the address of a is the same between iterations. This is because:
When ranging over a slice, two values are returned for each iteration. The first is the index, and the second is a copy of the element at that index.
Between each iteration there’s only one variable being modified – and it’s a copy, not the underlying element.
Decoding
While it might be tempting to reuse structures to decode data (e.g. JSON, XML, Database rows), I’m going to point out an issue with this approach especially when decoding partial or invalid data.
Valid JSON w/ Omitted Fields
The issue here is there’s a side-effect when handling omitted data.
The essence of the problem is demonstrated in the short code snippet below:
package main
import (
"encoding/json"
"fmt"
)
type A struct {
ValA int `json:"valA"`
ValB int `json:"valB"`
}
var input = A{}
func main() {
json.Unmarshal([]byte(`{"valA": 1, "valB": 1}`), &input)
json.Unmarshal([]byte(`{"valA": 0}`), &input)
output, _ := json.Marshal(input)
fmt.Println(string(output))
}
Output:
{"valA":0,"valB":1}
Notice that valB is omitted from the second JSON string.
There are many situations you could have omitted JSON like this – maybe it’s from a (unsafe) HTTP PATCH request where the API requires only the fields the client wants updated.
Or maybe the field was unset from a previous JSON serialization and the omitempty tag instructed the encoder to leave out unset fields.
Whatever the case is, we have the JSON so let’s see what happens when we decode it.
The first unmarshal operation results in each field being set to 1 ({"valA":1,"valB":1}),
and the second JSON string overwrites valA to 0 ({"valA":0,"valB":1}).
Notice valB was left unmodified from the first decoding step.
This probably isn’t a surprise – you’re reusing memory and overwriting some of it with new data.
Valid DB Row w/ Omitted Fields
Okay, let’s look at a database example that might be a bit more subtle:
package main
import (
"database/sql"
"encoding/json"
"fmt"
"strings"
"github.com/jmoiron/sqlx"
)
// A used for `sometable` row
type A struct {
Id int `json:"ID"`
ValA int `json:"valA"`
ValB int `json:"valB"`
}
func (a *A) Insert(sqldb *sqlx.DB) (int64, error) {
var rslt sql.Result
var err error
if rslt, err = sqldb.Exec(fmt.Sprintf("INSERT INTO sometable (valA, valB) VALUES (%d, %d) ", a.ValA, a.ValB)); err != nil {
return 0, err
} else if rslt == nil {
return 0, fmt.Errorf("no result")
}
id, err := rslt.LastInsertId()
if err != nil {
return 0, err
}
return id, nil
}
func (a *A) Query(sqldb *sqlx.DB, id int64, columns []string) (err error) {
var row *sql.Row
if len(columns) == 0 {
return nil
}
if row = sqldb.QueryRow(fmt.Sprintf("SELECT %s from sometable where ID=%d", strings.Join(columns, ","), id)); err != nil {
return err
} else if row == nil {
return sql.ErrNoRows
}
if err = row.Scan(a); err != nil {
return err
}
return nil
}
func main() {
input := A{
ValA: 1,
ValB: 1,
}
db, _ := sqlx.Open("mysql", "root:root")
id, _ := input.Insert(db)
db.Exec(`
CREATE TABLE sometable (
ID int,
valA varchar(255),
valA varchar(255),
);
`)
input.ValA = 0
input.ValB = 0
input.Query(db, id, []string{"valA"}) // query only one column causing omitted data returned
output, _ := json.Marshal(input)
fmt.Println(string(output))
}
Output:
{"valA":0,"valB":1}
This example is basically the same as the last, except instead of decoding JSON with omitted fields it’s decoding a database row with omitted values.
Failed Decoding
So far I have provided examples where incoming data (JSON/DB rows) are valid, but contain omitted data. But there are situations where incoming data may be in the wrong format or partially incomplete (e.g. streamed JSON), or maybe you’re just running a test with bad input data. Whatever the case, re-using a structure here also also produces problems.
The following example contains 2 valid JSON strings at the start and end of dataSequence array, the other contains an invalid field and are not added to the As array.
package main
import (
"encoding/json"
"fmt"
)
type A struct {
ValA int `json:"valA"`
ValB int`json:"valB"`
}
var input = A{}
func main() {
dataSequence := [][]byte{
[]byte(`{"valA": 1, "valB": 1}`),
[]byte(`{"valA": "invalid", "valB": 3}`), // valA fails to parse
[]byte(`{}`), // everything omitted
}
var As = make([]A, len(dataSequence))
for i, data := range dataSequence {
if err := json.Unmarshal(data, &input); err == nil {
As[i] = input
} else {
// skips second element with invalid `valA`
}
}
output, _ := json.Marshal(As[len(As)-2:]) // last two elements
fmt.Println(string(output))
}
Output:
[{},{"valA":1,"valB":3}]
This is an interesting situation.
The second unmarshal will (partially) fail due to the invalid type for the valA field, but the decoder continues on and successfully sets valB.
While the unmarshal operation returns an error and the decoded object isn’t put into A[1] index of the array, values still bleed between iterations of the loop, just like before.
Concurrency Problems (Race Condition)
Let’s suppose this is the general design you choose for your HTTP PATCH handler, with requests handled concurrently for a single resource, and you store this resource to be reused somewhere in memory. (I hope you already see the problem with running concurrent code in which multiple threads share a resource, and if not, review the output below the code snippet)
package main
import (
"encoding/json"
"fmt"
)
type A struct {
ValA int `json:"valA"`
ValB int`json:"valB"`
}
var input = A{}
func fetchFromMemory() (*A) {
return &input
}
func main() {
json.Unmarshal([]byte(`{"valA": 1, "valB": 1}`), fetchFromMemory())
for i := 0; i < 10; i++ {
go json.Unmarshal([]byte(`{"valA": 99}`), fetchFromMemory())
go json.Unmarshal([]byte(`{"valA": 9999999}`), fetchFromMemory())
output, _ := json.Marshal(fetchFromMemory())
fmt.Println(string(output))
}
}
Output:
{"valA":99,"valB":1}
==================
WARNING: DATA RACE
Write at 0x00000066b9d0 by goroutine 8:
reflect.Value.SetInt()
/usr/local/go/src/reflect/value.go:1642 +0xfd
encoding/json.(*decodeState).literalStore()
/usr/local/go/src/encoding/json/decode.go:1002 +0x266e
encoding/json.(*decodeState).value()
/usr/local/go/src/encoding/json/decode.go:384 +0x307
encoding/json.(*decodeState).object()
/usr/local/go/src/encoding/json/decode.go:765 +0x220d
encoding/json.(*decodeState).value()
/usr/local/go/src/encoding/json/decode.go:370 +0xb4
encoding/json.(*decodeState).unmarshal()
/usr/local/go/src/encoding/json/decode.go:180 +0x287
encoding/json.Unmarshal()
/usr/local/go/src/encoding/json/decode.go:107 +0x1e5
Previous write at 0x00000066b9d0 by goroutine 7:
reflect.Value.SetInt()
/usr/local/go/src/reflect/value.go:1642 +0xfd
encoding/json.(*decodeState).literalStore()
/usr/local/go/src/encoding/json/decode.go:1002 +0x266e
encoding/json.(*decodeState).value()
/usr/local/go/src/encoding/json/decode.go:384 +0x307
encoding/json.(*decodeState).object()
/usr/local/go/src/encoding/json/decode.go:765 +0x220d
encoding/json.(*decodeState).value()
/usr/local/go/src/encoding/json/decode.go:370 +0xb4
encoding/json.(*decodeState).unmarshal()
/usr/local/go/src/encoding/json/decode.go:180 +0x287
encoding/json.Unmarshal()
/usr/local/go/src/encoding/json/decode.go:107 +0x1e5
Goroutine 8 (running) created at:
main.main()
/home/austinm/Tresors/Projects/personal/austin-millan-blog/austinmillan.com/content/writings/prog.go:24 +0x204
Goroutine 7 (finished) created at:
main.main()
/home/austinm/Tresors/Projects/personal/austin-millan-blog/austinmillan.com/content/writings/prog.go:23 +0x164
==================
{"valA":99,"valB":1}
{"valA":99,"valB":1}
{"valA":99,"valB":1}
{"valA":99,"valB":1}
{"valA":99,"valB":1}
{"valA":99,"valB":1}
{"valA":9999999,"valB":1}
{"valA":9999999,"valB":1}
{"valA":9999999,"valB":1}
Found 1 data race(s)
exit status 66{"valA":99,"valB":1}
==================
WARNING: DATA RACE
Write at 0x00000066b9d0 by goroutine 8:
reflect.Value.SetInt()
/usr/local/go/src/reflect/value.go:1642 +0xfd
encoding/json.(*decodeState).literalStore()
/usr/local/go/src/encoding/json/decode.go:1002 +0x266e
encoding/json.(*decodeState).value()
/usr/local/go/src/encoding/json/decode.go:384 +0x307
encoding/json.(*decodeState).object()
/usr/local/go/src/encoding/json/decode.go:765 +0x220d
encoding/json.(*decodeState).value()
/usr/local/go/src/encoding/json/decode.go:370 +0xb4
encoding/json.(*decodeState).unmarshal()
/usr/local/go/src/encoding/json/decode.go:180 +0x287
encoding/json.Unmarshal()
/usr/local/go/src/encoding/json/decode.go:107 +0x1e5
Previous write at 0x00000066b9d0 by goroutine 7:
reflect.Value.SetInt()
/usr/local/go/src/reflect/value.go:1642 +0xfd
encoding/json.(*decodeState).literalStore()
/usr/local/go/src/encoding/json/decode.go:1002 +0x266e
encoding/json.(*decodeState).value()
/usr/local/go/src/encoding/json/decode.go:384 +0x307
encoding/json.(*decodeState).object()
/usr/local/go/src/encoding/json/decode.go:765 +0x220d
encoding/json.(*decodeState).value()
/usr/local/go/src/encoding/json/decode.go:370 +0xb4
encoding/json.(*decodeState).unmarshal()
/usr/local/go/src/encoding/json/decode.go:180 +0x287
encoding/json.Unmarshal()
/usr/local/go/src/encoding/json/decode.go:107 +0x1e5
Goroutine 8 (running) created at:
main.main()
/home/austinm/Tresors/Projects/personal/austin-millan-blog/austinmillan.com/content/writings/prog.go:24 +0x204
Goroutine 7 (finished) created at:
main.main()
/home/austinm/Tresors/Projects/personal/austin-millan-blog/austinmillan.com/content/writings/prog.go:23 +0x164
==================
{"valA":99,"valB":1}
{"valA":99,"valB":1}
{"valA":99,"valB":1}
{"valA":99,"valB":1}
{"valA":99,"valB":1}
{"valA":99,"valB":1}
{"valA":9999999,"valB":1}
{"valA":9999999,"valB":1}
{"valA":9999999,"valB":1}
Found 1 data race(s)
exit status 66
There are couple solutions off the top of my head to avoid this data race:
- Copy the variable into different addresses, and pass copies to each thread.
- Implement a locking mechanism (
"sync") for the shared resource. - Don’t reuse and share this structure across threads.
Conclusion
The conclusion is that in general it’s better to avoid reusing/recycling structures, and it should especially be avoided while decoding data.