I think deferring unlocks should be more openly described as an anti-pattern. I do appreciate the convenience of defer generally speaking but it has two problems that specifically hinder effective use of mutexes:

1. it allows you to roll up additional code that doesn't need to be inside the mutex, thus keeping other threads waiting longer for an unlock

2. Benchmarks have shown[1][2] that using defer is actually more expensive than not using defer and simply having the unlock sit just before your return call(s). I get defer produces more readable and safer code, however when you have a locking function, your are concerned with unlocking quickly. So defer directly causes complications here that, in my opinion, outweighs any benefits you get with regards to readability.

Sadly a lot of mutex examples specifically use defer without addressing any of the risks of doing so. such as GoByExample.com[3]

[1] https://gist.github.com/janisz/ce19a7fa94cbc99e2835a4421ccfc...

[2] https://medium.com/i0exception/runtime-overhead-of-using-def...

[3] https://gobyexample.com/mutexes

There are several patterns possible which resolve this problem, your third example actually shows that, as it uses a function which only does the container access in a separate function which uses defer - this guarantees that no other code is executed while the lock is held. Alternatively, you could use an anonymous function inside your function which immediately gets called and performs the locking, lookup and deferred unlocking.

As a Lisper, this situation of course screams for having a with-lock macro which allows to add an elegant, but safe primitive which mostly takes care of this. This would usually be implemented in terms of a function which performs the locking/unlocking and calls a function which executes the body. Fortunately, Go allows doing exactly that, so this pattern could be used, here an untested sketch:

  func withLock (l Lockable, f func()) {
     l.Lock()
     defer l.Unlock()
     f()
   }

  ...
  withLock(c, func() { c.Counters[]++})
  ...

Of course, this doesn't protect you doing some potentially blocking actions in the body of the function executed, but the leaner the syntax is and as the defer is guaranteed to be executed at the end of the body, the risk is greatly reduced.

You could simply place `l.unlock()` underneath the function call to remove the overhead of the defer.

That doesn't address the overhead of defer though (point 2) which can be a problem if your locks are in a well travelled path.

One might argue that if you have a hot path that depends on mutexes then perhaps you need to restructure your code to avoid sharing data (and thus the need for a mutex). And that point would be valid, generally speaking. But there's always that one edge case where it is unavoidable (or even still the cleaner and faster code).

In recent versions defer is basically zero overhead if no loops are involved. It's just a function call at every function exit. Basically what you'd do yourself manually.

Yes, the overhead isn't avoided. This example was thought as the "safe" choice. If you want to be "fast", you have to very carefully analyze what you are doing and then deciding accordingly. If you have code which cannot panic, you have no need to use defer and can unlock directly after the access. And the biggest gain would be avoiding acquiring and releasing the lock in the hot path in the first place.

I think the best practice is to approach a problem from the safe direction as long as performance isn't a total disaster and then try to find the spots which carefully have to be optimized relaxing some of the safeties. But as you are dealing with small hot spots, you can manually analyze the code in question. Similar to the "unsafe" package - if you use it only punctually, the safety loss is usually not a problem.

> So defer directly causes complications here that, in my opinion, outweighs any benefits you get with regards to readability.

The primary advantage of defer is not readability, it’s safety, in the presence of both panics and code evolving.

Yes, I agree and covered your point regarding safety in literally the sentence before that which you quoted :)

You didn't, really. You mentioned it in passing then completely ignored the issue without actually addressing how to handle the issue of locking in face of go's lack of panic safety, or human errors.

> You didn't, really. You mentioned it in passing

But it was still mentioned ;) The benefits of defer wasn't the point of my post so I'm obviously not going to write a paragraph covering all the safety concerns that defer addresses.

> then completely ignored the issue without actually addressing how to handle the issue of locking in face of go's lack of panic safety, or human errors.

Because that is a separate point. But I'm happy to discuss that if you wish:

Realistically you should put as little code inside the lock as possible and accept the risk that a panic might be generated inside the lock. However if you keep your code minimal and write thorough unit tests (not just testing expected behaviour but also hammering invalid values into the function too) then you can reduce the risk enough that it is lower than the risk of bad UX due to inefficient use of mutexes.

It's not a particularly great answer and relies heavily on developers writing good code and excellent tests, but this falls back to a point I made elsewhere that Go's tooling for managing goroutines is rather poor considering how much Go is credited for making "multi-threaded" code easy. So you're left with having to trust developers to write good code.

Does Go still run defers at function exit rather than scope exit? Changing that would be a big improvement, although I guess they’d have to use a different keyword to avoid breaking compatibility.

At function exit. You can of course always create an anonymous function to emulate a block like:

  func() {
    defer something()
    ...
  }()

This basically has the same behavior as having a block there, but with the defer being called at the exit of that functions body.

Exactly. Mutexes should be held for the shortest time possible (and you should avoid calling any function you don't know or the implementation details), yet RW mutexes only make sense if the critical section is large enough. Ergo RW mutexes very rarely make sense.

> RW mutexes only make sense if the critical section is large enough. Ergo RW mutexes very rarely make sense.

I disagree :) Exclusive locks have unavoidable cache-line ping-ponging on the state, which can make a contended lock very expensive. In a write-infrequently read-frequently scenario, mutexes can do clever things to avoid that, for example have core-local reader counts.

Also there's a middle-ground between "pointer swap" and "so large that that it will block the writers enough to notice" critical sections, and often readers are in that sweet spot.

Well, yes you enter the realm of asymmetric synchronization (I like seqlocks myself), but I feel these are no longer general purpose synchronisation primitives but have often application specific trade-offs. Generic RWlocks are usually not great and do not come with these optimization

Also, yes my critical sections are often closer to the pointer swap case that might color my expectations.

My expectations may be colored by having a pretty good default implementation of a shared mutex in our codebase :) (folly::SharedMutex)

I've had to resort to seqlocks + async reclamation (either RCU or hazard pointers, depending on the case) only in a handful of very very specific cases.

Thanks, I'll take a look at folly::SharedMutex!