Why does `defer mu.Unlock()` at the top of a Go method hold the mutex longer than the data needs?

## What `defer` actually promises `defer` in Go registers a call to be run when the **surrounding function** returns — after the return values are set, in last-in-first-out order, and on every exit path including a panic that is unwinding the stack. It has no block scope. Writing ```go if cond { s.mu.Lock() defer s.mu.Unlock() } ``` does **not** release the lock at the closing brace of the `if`; it releases it when the enclosing function returns. This is the single most important difference between Go's lock idiom and a scoped guard in a language with destructors or a `synchronized` block, and it is where the critical section quietly grows. ## How the critical section widens A method usually starts life small: ```go func (s *Store) Save(id string, sess Session) error { s.mu.Lock() defer s.mu.Unlock() s.entries[id] = sess return nil } ``` That is correct and cheap: the lock is held for one map assignment. Then a later change adds a durability step — marshal the session and write it to a file — and a still later change adds a metrics push. Nobody moves the `defer`, because it is at the top where the idiom says to put it. Now the lock is held across an allocation-heavy encode and a syscall that can block for milliseconds. Only the map assignment needed protection; the rest of the body is riding along inside it. Every other goroutine calling any method that takes `s.mu` waits out the file write. This matters most for a structure on the request path — a session store consulted by every authenticated request — because the time under the lock is multiplied by the arrival rate. A microsecond map write serialises fine at high request rates; a two-millisecond file write does not, and adding CPU cores does not help, because the added cores just queue behind the same lock. ## The three narrowings **1. Unlock explicitly before the slow part.** ```go s.mu.Lock() s.entries[id] = sess s.mu.Unlock() b, err := json.Marshal(sess) ``` Simple and obvious, but you give up the panic-safety and early-return safety `defer` buys: if code between `Lock` and `Unlock` panics or returns, the mutex stays locked forever and the next caller deadlocks. Use it only when the guarded region is a few straight-line statements with no `return` inside. **2. Wrap the guarded lines in an inner function.** ```go func() { s.mu.Lock() defer s.mu.Unlock() s.entries[id] = sess }() ``` The deferred unlock now belongs to the literal, so it runs at the literal's return — the closing `}()`. You keep the panic safety and get block scope. This is the standard Go answer to "I want a scoped lock". **3. Extract a small locked method.** Best of all when the guarded work has a name: `s.put(id, sess)` takes the lock, does the map write, and returns; the caller does the encoding and the I/O outside it. Locked methods that are three lines long are easy to review, and the compiler often inlines them anyway. ## Copy out, don't compute in A related habit: when the guarded data is only read, take the lock, copy what you need into a local, unlock, and compute on the local. For a map you may need to copy the entries you care about rather than the whole map — but even copying a slice of ten values is far cheaper than holding a lock across formatting or serialisation. ## What not to conclude - **Do not read this as "defer is slow".** Deferred calls are cheap; the cost here is the *scope*, not the mechanism. - **Do not drop `defer` from methods that are already tight.** If the whole body is the critical section, `defer` at the top is exactly right and is the more robust code. - **Do not "help" by unlocking twice.** Unlocking a `sync.Mutex` that is not locked panics with "sync: unlock of unlocked mutex", and an early `Unlock()` plus the deferred one does precisely that. If you unlock explicitly, remove the `defer`. - **Do not hold a lock while sending on or receiving from a channel** unless you have proved the peer cannot block, since that turns a lock hold into an unbounded wait. ## Reviewing for it When reading a diff, find the `Lock()` and ask what the last line before the function's return is. Everything between them is the critical section. If that span contains a network call, a file operation, a `time.Sleep`, a channel operation, a log write, or a call into code you do not control, the section is too wide — and the shape of the fix is almost always "move it out", not "use a different lock type".