godata-race

Mixing pointer and value receivers causes a data race


I have an object I'm using to cache a bunch of data periodically to be used in a separate service:

type Cache struct {
    mut    *sync.RWMutex
    logger *log.Logger
    client *IClient
    data   map[string]*Item
}

func (pch *Cache) Start(ctx context.Context) {
    pch.update(ctx)

    for {
        select {
        case <-ctx.Done():
            return
        case <-time.After(refreshTime):
            pch.update(ctx)
        }
    }
}

func (pch Cache) Get(reference string) *Item {
    pch.mut.RLock()
    defer pch.mut.RUnlock()
    return pch.data[reference]
}

func (pch *Cache) update(ctx context.Context) {
    data, err := pch.client.GetData(ctx)
    if err != nil {
        pch.logger.Panicf("Failed to retrieve data from service: %v", err)
    }

    pch.mut.Lock()
    defer pch.mut.Unlock()
    pch.data = resp  // DATA RACE HERE
}

The intention of this object is that I call Start as a go routine and then subsequent calls to Get will use the data populated therein. This works fine when I test but it consistently fails data race checks, as in the following unit test:

func Test(t *testing.T) {
    cache := Cache{
        mut:    new(sync.RWMutex),
        logger: log.Default(),
        cache:  newMockClient(false),
        plants: make(map[string]*Item),
    }

    ctx, cancel := context.WithCancel(context.Background())
    defer cancel()
    go cache.Start(ctx)

    time.Sleep(100 * time.Millisecond)

    data, err := cache.Get("P-0001")  // DATA RACE HERE
}

The issue as I see it is that the use of a read-write mutex should prevent any race condition from occurring and that the read side of the race condition doesn't seem to be occurring on cache.data but rather on cache so I'm not really sure why this is occurring at all. I also don't see how I can avoid this data race condition outside a test environment. I could modify my test to call update instead, but that feels like a bit of a hack.

Does anyone have some suggestions on how to deal with this situation?


Solution

  • The Get method has a value receiver, and in your unit test you create cache as a Cache{} value. This causes data to be a copy of the original struct field. This means the mutex isn’t protecting the same thing and triggers the race detector.