Skip to content

Commit

Permalink
Mutexes: Advise against embedding
Browse files Browse the repository at this point in the history
Our guidance for mutexes suggested that it was okay to embed mutexes in
structs if the struct was unexported. This was left as an allowance for
"if you really want to do it, this is the only time it's okay", but that
nuance doesn't translate well to the style guide.

Advise against embedding mutexes completely to resolve any confusion
here.

Resolves #127
  • Loading branch information
abhinav committed Jul 8, 2021
1 parent 98ce9a3 commit e0dafcc
Showing 1 changed file with 18 additions and 14 deletions.
32 changes: 18 additions & 14 deletions style.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,29 +332,28 @@ mu.Lock()
</td></tr>
</tbody></table>

If you use a struct by pointer, then the mutex can be a non-pointer field.

Unexported structs that use a mutex to protect fields of the struct may embed
the mutex.
If you use a struct by pointer, then the mutex should be a non-pointer field on
it. Do not embed the mutex on the struct.

<table>
<thead><tr><th>Bad</th><th>Good</th></tr></thead>
<tbody>
<tr><td>

```go
type smap struct {
sync.Mutex // only for unexported types
type SMap struct {
sync.Mutex

data map[string]string
}

func newSMap() *smap {
return &smap{
func NewSMap() *SMap {
return &SMap{
data: make(map[string]string),
}
}

func (m *smap) Get(k string) string {
func (m *SMap) Get(k string) string {
m.Lock()
defer m.Unlock()

Expand Down Expand Up @@ -387,12 +386,17 @@ func (m *SMap) Get(k string) string {

</td></tr>

</tr>
<tr>
<td>Embed for private types or types that need to implement the Mutex interface.</td>
<td>For exported types, use a private field.</td>
</tr>
<tr><td>

The `Mutex` field, and the `Lock` and `Unlock` methods are unintentionally part
of the exported API of `SMap`.

</td><td>

The mutex and its methods are implementation details of `SMap` hidden from its
callers.

</td></tr>
</tbody></table>

### Copy Slices and Maps at Boundaries
Expand Down

0 comments on commit e0dafcc

Please sign in to comment.