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.
This commit is contained in:
Nate Finch 2021-11-19 14:51:47 -05:00 committed by GitHub
parent f09cadcbd9
commit 516c158212
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -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},
}