I'm going over some existing code and see this repeated several times
defer mtx.Unlock()
mtx.Lock()
This looks wrong to me, I much prefer the idiomatic way of deferring the Unlock after performing the Lock but the documentation for Mutex.Lock doesn't specify a situation where the Lock will fail. Therefor the behaviour of the early defer pattern should be identical to the idiomatic way.
My question is: is there a compelling case to say this pattern is inferior? (e.g. the Lock may fail and then the deferred Unlock will panic) and thus the code should be changed or should I leave it as is?
Short answer:
Yes, it's OK. The defer call is made after the function returns (well, sort of).
The longer, more nuanced answer:
It's risky, and should be avoided. In your 2 line snippet, it's not going to be a problem, but consider the following code:
func (o *Obj) foo() error {
defer o.mu.Unlock()
if err := o.CheckSomething(); err != nil {
return err
}
o.mu.Lock()
// do stuff
}
In this case, the mutex may not be locked at all, or worse still: it may be locked on another routine, and you end up unlocking it. Meanwhile yet another routine obtains a lock that it really shouldn't have, and you either get data races, or by the time that routine returns, the unlock call will panic. Debugging this kind of mess is a nightmare you can, and should avoid at all cost.
In addition to the code looking intuitive and being more error prone, depending on what you're trying to achieve, a defer does come at a cost. In most cases the cost is fairly marginal, but if you're dealing with something that is absolutely time critical, it's often better to manually add the unlock calls where needed. A good example where I'd not use defer is if you're caching stuff in a map[string]interface{}: I'd create a struct with the cached values and an sync.RWMutext field for concurrent use. If I use this cache a lot, the defer calls could start adding up. It can look a bit messy, but it's on a case by case basis. Either performance is what you aim for, or shorter, more readable code.
Other things to note about defers: