Golang race with sync.Mutex on map[string]int

312 Views Asked by At

I have a simple package I am using to log stats during a program run and I found that go run -race says there is a race condition in it. Looking at the program I'm not sure how I can have a race condition when every read and write is protected by a mutex. Can someone explain this?

package counters

import "sync"

type single struct {
    mu     sync.Mutex
    values map[string]int64
}

// Global counters object
var counters = single{
    values: make(map[string]int64),
}

// Get the value of the given counter
func Get(key string) int64 {
    counters.mu.Lock()
    defer counters.mu.Unlock()
    return counters.values[key]
}

// Incr the value of the given counter name
func Incr(key string) int64 {
    counters.mu.Lock()
    defer counters.mu.Unlock()
    counters.values[key]++        // Race condition
    return counters.values[key]
}

// All the counters
func All() map[string]int64 {
    counters.mu.Lock()
    defer counters.mu.Unlock()
    return counters.values        // running during write above
}

I use the package like so:

counters.Incr("foo")
counters.Get("foo")
2

There are 2 best solutions below

0
On BEST ANSWER

All returns the underlying map and the releases the lock, so the code using the map will have a data race.

You should return a copy of the map:

func All() map[string]int64 {
    counters.mu.Lock()
    defer counters.mu.Unlock()
    m := make(map[string]int64)
    for k, v := range counters.values {
        m[k] = v
    }
    return m    
}

Or not have an All method.

0
On

A Minimal Complete Verifiable Example would be useful here, but I think your problem is in All():

// All the counters
func All() map[string]int64 {
    counters.mu.Lock()
    defer counters.mu.Unlock()
    return counters.values        // running during write above
}

This returns a map which does not make a copy of it, so it can be accessed outside the protection of the mutex.