Go’s append is not always thread safe

Problem Example

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.

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.

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

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

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 doesn’t change. There’s no confusion here, so there is no race.

z and y get their own memory

Things are different in the new code.

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.

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

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

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

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.

append with the local variable first