Problem Example Link to heading

I commonly see bugs appending to slices in Go in a way that is not thread safe. A simple example is the unit tests below. This test has two goroutines append to the same slice. If you run this test with the -race flag, it works just fine.

package main

import (
    "sync"
    "testing"
)

func TestAppend(t *testing.T) {
    x := []string{"start"}
    
    wg := sync.WaitGroup{}
    wg.Add(2)
    go func() {
        defer wg.Done()
        y := append(x, "hello", "world")
        t.Log(cap(y), len(y))
    }()
    go func() {
        defer wg.Done()
        z := append(x, "goodbye", "bob")
        t.Log(cap(z), len(z))
    }()
    wg.Wait()
}

Now, let’s change the code just slightly to create the x slice with more spare capacity. The only thing that changed was line 9.

package main

import (
    "testing"
    "sync"
)

func TestAppend(t *testing.T) {
    x := make([]string, 0, 6)

    wg := sync.WaitGroup{}
    wg.Add(2)
    go func() {
        defer wg.Done()
        y := append(x, "hello", "world")
        t.Log(len(y))
    }()
    go func() {
        defer wg.Done()
        z := append(x, "goodbye", "bob")
        t.Log(len(z))
    }()
    wg.Wait()
}

If we run this test with the -race flag, we will notice a race condition.

< go test -race .
==================
WARNING: DATA RACE
Write at 0x00c4200be060 by goroutine 8:
_/tmp.TestAppend.func2()
/tmp/main_test.go:20 +0xcb
Previous write at 0x00c4200be060 by goroutine 7:
_/tmp.TestAppend.func1()
/tmp/main_test.go:15 +0xcb
Goroutine 8 (running) created at:
_/tmp.TestAppend()
/tmp/main_test.go:18 +0x14f
testing.tRunner()
/usr/local/Cellar/go/1.10.2/libexec/src/testing/testing.go:777 +0x16d
Goroutine 7 (running) created at:
_/tmp.TestAppend()
/tmp/main_test.go:13 +0x105
testing.tRunner()
/usr/local/Cellar/go/1.10.2/libexec/src/testing/testing.go:777 +0x16d
==================
==================
WARNING: DATA RACE
Write at 0x00c4200be070 by goroutine 8:
_/tmp.TestAppend.func2()
/tmp/main_test.go:20 +0x11a
Previous write at 0x00c4200be070 by goroutine 7:
_/tmp.TestAppend.func1()
/tmp/main_test.go:15 +0x11a
Goroutine 8 (running) created at:
_/tmp.TestAppend()
/tmp/main_test.go:18 +0x14f
testing.tRunner()
/usr/local/Cellar/go/1.10.2/libexec/src/testing/testing.go:777 +0x16d
Goroutine 7 (finished) created at:
_/tmp.TestAppend()
/tmp/main_test.go:13 +0x105
testing.tRunner()
/usr/local/Cellar/go/1.10.2/libexec/src/testing/testing.go:777 +0x16d
==================
--- FAIL: TestAppend (0.00s)
main_test.go:16: 2
main_test.go:21: 2
testing.go:730: race detected during execution of test
FAIL
FAIL _/tmp 0.901s

Explaining why this test fails Link to heading

To understand why this fails, look at the memory of x in the old example.

Starting picture of a slice in Go

x starts with no capacity to change

Go notices that there is no memory to place "hello", "world" or to place "goodbye", "bob", so it makes new memory for y and z. Data races don’t happen when multiple threads read memory, x, that does not change. There’s no confusion here, so there is no race.

What a Go slice looks like after two appends

z and y get their own memory

Things are different in the new code.

The slice with extra capacity

x has capacity for more

Here Go notices that there is memory to place “hello”, “world”. Another goroutine also notices that there is memory for “goodbye”, “bob”. The race happens because both goroutines are trying to write to the same spare memory and it’s not clear who wins.

A slice made with a data race

Who wins?

It is a feature, not a bug, that append does not force new memory allocations each time it is called. This allows users to append inside a loop without thrashing garbage collection. The downside is that you have to be aware when appends happen to the same original slice from multiple goroutines.

Cognitive root of this bug Link to heading

I believe this bug exists because Go has, for the sake of simplicity, put many concepts into slices that are usually distributed. The thought process I see in most developers is:

  1. x = append(x, ...) looks like you’re receiving a new slice.
  2. Most functions that return values don’t mutate their inputs.
  3. Often when I use append the result is a new slice.
  4. This leads one to, falsely, think append is read only.

Identifying this bug Link to heading

Pay special attention if the first variable to append is not a local variable.This bug usually manifest when append is happening to a variable stored inside a struct or a variable passed into the current function. For example, a struct could have default values that are appended to each request. Be careful when appending to shared memory, or memory the current goroutine doesn’t entirely own.

Workarounds Link to heading

The easiest workaround is to not use shared state as the first variable to append. Instead, make a new slice with the total capacity you need, and use the new slice as the first variable to append. Below is the failing example test modified to work. An alternative to append here is to use copy.

package main

import (
        "sync"
        "testing"
)

func TestAppend(t *testing.T) {
    x := make([]string, 0, 6)
    x = append(x, "start")
    
    wg := sync.WaitGroup{}
    wg.Add(2)
    go func() {
        defer wg.Done()
        y := make([]string, 0, len(x)+2)
        y = append(y, x...)
        y = append(y, "hello", "world")
        t.Log(cap(y), len(y), y[0])
    }()
    go func() {
        defer wg.Done()
        z := make([]string, 0, len(x)+2)
        z = append(z, x...)
        z = append(z, "goodbye", "bob")
        t.Log(cap(z), len(z), z[0])
    }()
    wg.Wait()
}