From 516c15821204b450f56df69acafdb4a13dd2c21f Mon Sep 17 00:00:00 2001 From: Nate Finch Date: Fri, 19 Nov 2021 14:51:47 -0500 Subject: [PATCH] don't embed the mutex Embedding a mutex is a really bad idea. It exposes the Lock and Unlock methods on the enclosing type, which then allows code from outside your type to mess with your mutex. There was no reason to embed the mutex in this example. The comment say it is idiomatic to do so, but that is untrue. Using a mutex called "mu" is idiomatic. --- examples/mutexes/mutexes.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/examples/mutexes/mutexes.go b/examples/mutexes/mutexes.go index fce6019..0a34010 100644 --- a/examples/mutexes/mutexes.go +++ b/examples/mutexes/mutexes.go @@ -12,29 +12,28 @@ import ( // Container holds a map of counters; since we want to // update it concurrently from multiple goroutines, we -// add a `Mutex` to synchronize access. The mutex is -// _embedded_ in this `struct`; this is idiomatic in Go. +// add a `Mutex` to synchronize access. // Note that mutexes must not be copied, so if this // `struct` is passed around, it should be done by // pointer. type Container struct { - sync.Mutex + mu sync.Mutex counters map[string]int } func (c *Container) inc(name string) { // Lock the mutex before accessing `counters`; unlock // it at the end of the function using a [defer](defer) - // statement. Since the mutex is embedded into - // `Container`, we can call the mutex's methods like - // `Lock` directly on `c`. - c.Lock() - defer c.Unlock() + // statement. + c.mu.Lock() + defer c.mu.Unlock() c.counters[name]++ } func main() { c := Container{ + // Note that the zero value of a mutex is usable as-is, so no + // initialization is required here. counters: map[string]int{"a": 0, "b": 0}, }