Go’s append is not always thread safe

Jack Lindamood

Follow

Jun 12, 2018 · 3 min read

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 RACEWrite at 0x00c4200be060 by goroutine 8:_/tmp.TestAppend.func2()/tmp/main_test.go:20 +0xcbPrevious write at 0x00c4200be060 by goroutine 7:_/tmp.TestAppend.func1()/tmp/main_test.go:15 +0xcbGoroutine 8 (running) created at:_/tmp.TestAppend()/tmp/main_test.go:18 +0x14ftesting.tRunner()/usr/local/Cellar/go/1.10.2/libexec/src/testing/testing.go:777 +0x16dGoroutine 7 (running) created at:_/tmp.TestAppend()/tmp/main_test.go:13 +0x105testing.tRunner()/usr/local/Cellar/go/1.10.2/libexec/src/testing/testing.go:777 +0x16d====================================WARNING: DATA RACEWrite at 0x00c4200be070 by goroutine 8:_/tmp.TestAppend.func2()/tmp/main_test.go:20 +0x11aPrevious write at 0x00c4200be070 by goroutine 7:_/tmp.TestAppend.func1()/tmp/main_test.go:15 +0x11aGoroutine 8 (running) created at:_/tmp.TestAppend()/tmp/main_test.go:18 +0x14ftesting.tRunner()/usr/local/Cellar/go/1.10.2/libexec/src/testing/testing.go:777 +0x16dGoroutine 7 (finished) created at:_/tmp.TestAppend()/tmp/main_test.go:13 +0x105testing.tRunner()/usr/local/Cellar/go/1.10.2/libexec/src/testing/testing.go:777 +0x16d==================--- FAIL: TestAppend (0.00s)main_test.go:16: 2main_test.go:21: 2testing.go:730: race detected during execution of testFAILFAIL _/tmp 0.901s

Explaining why this test fails

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

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.

Things are different in the new code.

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.

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.