Embedding a sync.Mutex is equally bad whether or not the type is exported

[peterbourgon]

Unexported structs that use a mutex to protect fields of the struct may embed the mutex.

What makes unexported types special? 🙃

A mutex in a struct is almost always an implementation detail, used to provide concurrency-safety to the capabilities (methods) of the type. Embedding it allows callers to manipulate it directly, but that's an abstraction leak, breaks encapsulation, and is just an enormous maintenance risk, for what I hope are obvious reasons. Whatever callers are doing when they take and release the lock can equally well be expressed as a method, right?

Is there some other use case you have that motivates this exception?

[abhinav]

Hey, @peterbourgon, thanks for calling this out! We're in agreement that
unexported types aren't special here.

We left it there as an allowance for cases where engineers were embedding the
mutex solely to avoid typing foo.mu.Lock() and preferred typing
foo.Lock(). We felt that if they wanted to avoid those keystrokes, it was
okay only if the type is unexported, so that the Mutex field or the Lock
methods do not become part of the advertised public API of the type.

That said, we agree that maybe that nuance is difficult to communicate. We're
okay with switching the recommendation to advise against embedding mutexes.

[prashantv]

This was also inline with mentions in official Go talks, e.g.,
https://talks.golang.org/2015/tricks.slide#15
https://talks.golang.org/2012/10things.slide#3 (see "Embedded lock")

The export modified was added to avoid the sync.Mutex methods from being part of the exported type's API (almost always the wrong thing).

The current guideline also clashes with #118 -- using exported methods as a hint that it's part of the API vs an internal method. An embedded mutex will export the Lock and Unlock methods, and typically the lock is encapsulated by the type's methods.

[peterbourgon]

Nice! 😎

We left it there as an allowance for cases where engineers were embedding the mutex solely to avoid typing foo.mu.Lock() and preferred typing foo.Lock()

😧