How to make sure code has no data races in Go?

Question

I'm writing a microservice that calls other microservices, for data that rarely updates (once in a day or once in a month). So I decided to create cache, and implemented this interface:

type StringCache interface {
    Get(string) (string, bool)
    Put(string, string)
}

internally it's just map[string]cacheItem, where

type cacheItem struct {
    data      string
    expire_at time.Time
}

My coworker says that it's unsafe, and I need add mutex locks in my methods, because it will be used in parallel by different instances of http handler functions. I have a test for it, but it detects no data races, because it uses cache in one goroutine:

func TestStringCache(t *testing.T) {
    testDuration := time.Millisecond * 10
    cache := NewStringCache(testDuration / 2)

    cache.Put("here", "this")

    // Value put in cache should be in cache
    res, ok := cache.Get("here")
    assert.Equal(t, res, "this")
    assert.True(t, ok)

    // Values put in cache will eventually expire
    time.Sleep(testDuration)

    res, ok = cache.Get("here")
    assert.Equal(t, res, "")
    assert.False(t, ok)
}

So, my question is: how to rewrite this test that it detects data race (if it is present) when running with go test -race?

Answer 1

First thing first, the data race detector in Go is not some sort of formal prover which uses static code analysis but is rather a dynamic tool which instruments the compiled code in a special way to try to detect data races at runtime.
What this means is that if the race detector is lucky and it spots a data race, you ought to be sure there is a data race at the reported spot. But this also means that if the actual program flow did not make certain existing data race condition happen, the race detector won't spot and report it.
In oher words, the race detector does not have false positives but it is merely a best-effort tool.

So, in order to write race-free code you really have to rethink your approach.
It's best to start with this classic essay on the topic written by the author of the Go race detector, and once you have absorbed that there is no benign data races, you basically just train yourself to think about concurrently running theads of execution accessing your data each time you're architecting the data and the algorithms to manipulate it.

For instance, you know (at least you should know if you have read the docs) that each incoming request to an HTTP server implemented using net/http is handled by a separate goroutine.
This means, that if you have a central (shared) data structure such as a cache which is to be accessed by the code which processes client requests, you do have multiple goroutines potentially accessing that shared data concurrently.

Now if you have another goroutine which updates that data, you do have a potential for a classic data race: while one goroutine is updating the data, another may read it.

As to the question at hand, two things:

First, Never ever use timers to test stuff. This does not work.

Second, for such a simple case as yours, using merely two goroutines completely suffices:

package main

import (
    "testing"
    "time"
)

type cacheItem struct {
    data      string
    expire_at time.Time
}

type stringCache struct {
    m   map[string]cacheItem
    exp time.Duration
}

func (sc *stringCache) Get(key string) (string, bool) {
    if item, ok := sc.m[key]; !ok {
        return "", false
    } else {
        return item.data, true
    }
}

func (sc *stringCache) Put(key, data string) {
    sc.m[key] = cacheItem{
        data:      data,
        expire_at: time.Now().Add(sc.exp),
    }
}

func NewStringCache(d time.Duration) *stringCache {
    return &stringCache{
        m:   make(map[string]cacheItem),
        exp: d,
    }
}

func TestStringCache(t *testing.T) {
    cache := NewStringCache(time.Minute)

    ch := make(chan struct{})

    go func() {
        cache.Put("here", "this")
        close(ch)
    }()

    _, _ = cache.Get("here")

    <-ch
}

Save this as sc_test.go and then

tmp$ go test -race -c -o sc_test ./sc_test.go 
tmp$ ./sc_test 
==================
WARNING: DATA RACE
Write at 0x00c00009e270 by goroutine 8:
  runtime.mapassign_faststr()
      /home/kostix/devel/golang-1.13.6/src/runtime/map_faststr.go:202 +0x0
  command-line-arguments.(*stringCache).Put()
      /home/kostix/tmp/sc_test.go:27 +0x144
  command-line-arguments.TestStringCache.func1()
      /home/kostix/tmp/sc_test.go:46 +0x62

Previous read at 0x00c00009e270 by goroutine 7:
  runtime.mapaccess2_faststr()
      /home/kostix/devel/golang-1.13.6/src/runtime/map_faststr.go:107 +0x0
  command-line-arguments.TestStringCache()
      /home/kostix/tmp/sc_test.go:19 +0x125
  testing.tRunner()
      /home/kostix/devel/golang-1.13.6/src/testing/testing.go:909 +0x199

Goroutine 8 (running) created at:
  command-line-arguments.TestStringCache()
      /home/kostix/tmp/sc_test.go:45 +0xe4
  testing.tRunner()
      /home/kostix/devel/golang-1.13.6/src/testing/testing.go:909 +0x199

Goroutine 7 (running) created at:
  testing.(*T).Run()
      /home/kostix/devel/golang-1.13.6/src/testing/testing.go:960 +0x651
  testing.runTests.func1()
      /home/kostix/devel/golang-1.13.6/src/testing/testing.go:1202 +0xa6
  testing.tRunner()
      /home/kostix/devel/golang-1.13.6/src/testing/testing.go:909 +0x199
  testing.runTests()
      /home/kostix/devel/golang-1.13.6/src/testing/testing.go:1200 +0x521
  testing.(*M).Run()
      /home/kostix/devel/golang-1.13.6/src/testing/testing.go:1117 +0x2ff
  main.main()
      _testmain.go:44 +0x223
==================
--- FAIL: TestStringCache (0.00s)
    testing.go:853: race detected during execution of test
FAIL