Common data races in Go

I am working with Milind to fix the data race in Uber's Go database. Here is the common data races concluded from his paper: "A Study of Real-World Data Races in Golang".

Pattern 1: Loop variable capture by reference.

In Go, the loop iterator variable in for range is a single variable that takes different values in each loop iteration. (Loop variable capture by reference.)

It's better to make a local copy when using goroutines to process the loop variables.

for val := range container {
+    val := val
    go func() {
       // processing val 
    }()
}

Pattern 2: Error variable capture

Mixed use of error variable. In the following example, the concurrent writes on the return error variable from foo2() and foo3() cause a data race.

_, err := foo1()
if err != nil {
	// do something
}
go func() {
-	_, err = foo2()
+	_, err := foo2()
	if err != nil {
		// do something
	}
}
_, err = foo3()
if err != nil {
	// do something
}

Pattern 3: Normal return in a function with a named return

In Go, there is a feature called "named result parameters". The return variable can be given names and used as regular variables, just like the incoming parameters. When named, they are initialized to the zero values for their types when the function begins; if the function executes a return statement without arguments, the current values of the result parameters are used as the returned values.

func namedReturnCallee(x int) (res int) {
	res = 10
	if x > 0 {
		return // "return 10"
	}

+	newRes := res
	go func() {
-		_ = res // read res
+		_ = newRes
	}()

	return 20 // "res = 20"
}

Pattern 4: Deferred functions in a named return

Go offers a keyword defer to schedule a function call (the deferred function) to be run immediately before the function executing the defer returns.

func namedReturnWithDefer() (res string, err error) {
	defer func() {
		res, err = foo1()
	}()

	_, err = foo2()
	// do something

+    newErr := err
	go func() {
-		process_err(err)
+		process_err(newErr)
	}()

	return  // defer function runs immediately before here
}

Pattern 5: Meta fields of Slice change

The append operation appends the elements to the slice's end and returns the result. The slice may change as the number of elements reaches the slice's length, which returns a new slice.

uuids := []string{"a", "b", "c", "d"}

var res []string
var mutex sync.Mutex
safeAppend := func(str string) {
	mutex.Lock()
	res = append(res, str)
	mutex.Unlock()
}

for _, uuid := range uuids {
	uuid := uuid
	go func(r []string) {
		safeAppend(uuid)
	}(res) // slice read without holding lock
}

Pattern 6: Data Races on Thread-Unsafe Map

In Go, a map (hash table) is a sparse data structure, and accessing one element might result in accessing another element; if during the same process another insertion/deletion happens, it will modify the sparse data structure and cause a data race.

uuids := []string{"a", "b", "c", "d", "e"}
var errMap = make(map[string]error)
for _, uuid := range uuids {
	uuid := uuid
	go func() {
        // errMap keeps inserting elements
		errMap[uuid] = errors.New(uuid)
	}()
}

Pattern 7: Incorrect use of synchronization constructs

Incorrect use of synchronization constructs such as sync.Mutex and sync.RWMutex, which are value types (structures) in Go, often causes data races.

In the following example, the mutex passed to the two goroutines is captured by value. The two goroutines possess different mutex objects sharing no internal state. A correct implementation should pass the address of mutex (&mutex).

mutex := sync.Mutex{}

safeUpdate := func(m sync.Mutex) {
	m.Lock()
	globalVal++
	m.Unlock()
}

go safeUpdate(mutex)
go safeUpdate(mutex)

Pattern 8: Mixing Shared Memory with Message Passing

func (f *responseFutureImpl) Start() {
	go func() {
		resp, err := f.f()
		f.response = resp
		f.err = err
		time.Sleep(time.Second * 15)
		f.ch <- 1  // may be dead lock
	}()
}

// Waits for future
func (f *responseFutureImpl) Wait(ctx context.Context) error {
	select {
	case <-f.ch:
		return nil
	case <-ctx.Done():
		f.err = errCancelled  // data race with read in GetError
		return errCancelled
	}
}

Pattern 9: Incorrect Use of Flexible Group Synchronization

Go offers more leeway in its group synchronization construct sync.WaitGroup. The number of participants is dynamic. Incorrect placement of Add and Done methods of a sync.WaitGroup leads to data races.

In the following example, the incorrect placement of wg.Add(1) fails to guarantee the racyAccess is accessed before reaching wg.Wait().

	var wg sync.WaitGroup
	var racyAccess int
+	wg.Add(1)
	go func() {
-		wg.Add(1)
		racyAccess++
		wg.Done()
	}()
	wg.Wait()
	racyAccess++

Also, there is an example of incorrect placement of wg.Done().

func work(wg *sync.WaitGroup) int {
-	defer wg.Done()
	return 42
}

func main() {
	var wg sync.WaitGroup
	wg.Add(1)
	go func() {
		racyAccess = work(&wg)
+		wg.Done()
	}()
	wg.Wait()
	racyAccess++
}

Pattern 10: Mutating Shared Data in a Reader-Lock-Protected Critical Section

Go offers a reader-writer lock RWMutex, which allows concurrent readers to execute a critical section simultaneously. The RLock/RUnlock methods on an RWMutex hold the lock in a read-only mode. Sometimes developers accidentally put statements that may modify shared data in critical sections protected by RWMutex, while using RLock/RUnlock methods.

type Test struct {
	mutex sync.RWMutex
	ready bool
}

func cond() bool {
	return true
}

func (t *Test) updateTest() {
	t.mutex.RLock()
	defer t.mutex.RUnlock()
	_ = t.ready
	if cond() {
		t.ready = true
	}
}

func main() {
	var t Test
	var wg sync.WaitGroup
	wg.Add(2)
	go func() {
		t.updateTest()
		wg.Done()
	}()
	go func() {
		t.updateTest()
		wg.Done()
	}()
	wg.Wait()
}

Reference:

Chabbi, Milind, and Murali Krishna Ramanathan. "A study of real-world data races in Golang." Proceedings of the 43rd ACM SIGPLAN International Conference on Programming Language Design and Implementation. 2022.