Someone told me the memCacheInstance
has race condition, but go run -race
could not tell.
Codes:
type MemCache struct {
data []string
}
var memCacheInstance *MemCache
var memCacheCreateMutex sync.Mutex
func GetMemCache() *MemCache {
if memCacheInstance == nil {
memCacheCreateMutex.Lock()
defer memCacheCreateMutex.Unlock()
if memCacheInstance == nil {
memCacheInstance = &MemCache{
data: make([]string, 0),
}
}
}
return memCacheInstance
}
The go race detector does not detect every race, but when it does, it's always a positive case. You have to write code that simulates the racy behavior.
Your example has data race if GetMemCache()
is called from multiple goroutines. This simple example triggers the race detector:
func main() {
go GetMemCache()
GetMemCache()
}
Run it with go run -race .
, output is:
==================
WARNING: DATA RACE
Read at 0x000000526ac0 by goroutine 6:
main.GetMemCache()
/home/icza/gows/src/play/play.go:13 +0x64
Previous write at 0x000000526ac0 by main goroutine:
main.GetMemCache()
/home/icza/gows/src/play/play.go:18 +0x17e
main.main()
/home/icza/gows/src/play/play.go:28 +0x49
Goroutine 6 (running) created at:
main.main()
/home/icza/gows/src/play/play.go:27 +0x44
==================
Found 1 data race(s)
exit status 66
It has a race because the first read of the memCacheInstance
variable is without locking, without synchronization. All concurrent accesses to a variable must be synchronized where at least one of the accesses is a write.
A simple fix is to remove the unsynchronized read:
func GetMemCache() *MemCache {
memCacheCreateMutex.Lock()
defer memCacheCreateMutex.Unlock()
if memCacheInstance == nil {
memCacheInstance = &MemCache{
data: make([]string, 0),
}
}
return memCacheInstance
}
Also note that to execute some code once only in a concurrency-safe manner, there's sync.Once
. You may use it like this:
var (
memCacheInstance *MemCache
memCacheOnce sync.Once
)
func GetMemCache() *MemCache {
memCacheOnce.Do(func() {
memCacheInstance = &MemCache{
data: make([]string, 0),
}
})
return memCacheInstance
}
Also note that if you initialize your variable "right away" (when declared or in a package init()
function), there's no need for synchronization (because package initialization runs in a single goroutine):
var memCacheInstance = &MemCache{
data: make([]string, 0),
}
func GetMemCache() *MemCache {
return memCacheInstance
}
In which case you may also choose to export the variable and then there's no need for GetMemCache()
.