Mutex hat #2

Opendmitshur opened this issue 3 years ago
dmitshur commented 3 years ago · edited
struct {
	...

	rateMu     sync.Mutex
	rateLimits [categories]Rate
	mostRecent rateLimitCategory
}

Here, rateMu is a mutex hat. It sits, like a hat, on top of the variables that it protects.

So, without needing to write the comment, the above is implicitly understood to be equivalent to:

struct {
	...

	// rateMu protects rateLimits and mostRecent.
	rateMu     sync.Mutex
	rateLimits [categories]Rate
	mostRecent rateLimitCategory
}

When adding a new, unrelated field that isn't protected by rateMu, do this:

 struct {
 	...

 	rateMu     sync.Mutex
 	rateLimits [categories]Rate
 	mostRecent rateLimitCategory
+
+	common service
 }

Don't do this:

 struct {
 	...

 	rateMu     sync.Mutex
 	rateLimits [categories]Rate
 	mostRecent rateLimitCategory
+	common     service
 }

See https://talks.golang.org/2014/readability.slide#21.

Write Preview Markdown
nightlyone commented 3 years ago

Even better here is to move the new field above the mutex.

Write Preview Markdown
dmitshur commented 3 years ago

@nightlyone It really depends on the context. But yeah, it can be added above too. The point is to not blindly add it to the mutex-protected group unintentionally.

Write Preview Markdown
cbarrick commented 3 years ago
struct {
    ...

    rate struct {
        sync.Mutex
        limits     [categories]Rate
        mostRecent rateLimitCategory
    }
    
    common service
}

Why not have an inner struct with an embedded mutex? It's already idiomatic to use an embedded mutex when it locks all members of the outer struct, so this solution seems like an obvious generalization to me. And the explicit struct scope is better than the scope implied by the blank line IMO.

Write Preview Markdown
dmitshur commented 2 years ago · edited

@cbarrick Sure, that can be a viable option, if it works out nicely in the context where it is. Initializing such a struct might be harder, but it'll work if you don't need to do that often.

Write Preview Markdown
to comment.