The question this answers
What is inside this critical section that does not belong there, and what does keeping it there cost?
A updateProfile(userId, patch) handler: load the cached profile, validate the patch against a remote policy service, apply the patch to the in-memory object, and write it through to the database.
The profiles cache — a map from user id to a mutable Profile object — reachable from every handler. The policy service and the database are external systems, not shared memory.
A profile is never observed half-patched: any reader sees either the pre-patch state or the fully applied post-patch state, and no two concurrent patches to the same profile lose a field.
Synchronization exists to preserve this sentence. If a schedule can make it false, the code is wrong no matter which primitive it uses.
The pair everyone writes and the pair everyone should write
This is the most common performance bug in synchronized code, and it is almost always introduced with good intentions: the invariant is real, the lock is real, and the region was drawn around the *function* rather than around the invariant. The result is correct code whose throughput ceiling is set by a system you do not operate.
The arithmetic is worth doing explicitly because it is more brutal than intuition suggests. The in-memory patch takes about 2 microseconds. The policy call takes 40 ms at p50 and 400 ms at p99. Holding the lock across it makes the hold time 40 ms, which caps this profile at 25 updates per second. When the policy service degrades to 400 ms — not down, just slow — the cap becomes 2.5 updates per second, and every handler in the pool blocks. Your service reports a total outage because a dependency got slow.
The better version moves both external calls out. Notice that this is not just faster: it is *more available*, because a policy-service slowdown now costs latency on the requests that need it rather than blocking every request that touches any profile. That decoupling is the real prize, and it is why this belongs in review rather than in a profiler.
1async function updateProfile(userId: string, patch: Patch) {2 return await profileLock.withLock(async () => { // <-- acquired3 const p = profiles.get(userId)!4 5 const verdict = await policyService.check(userId, patch) // 40 ms p50 / 400 ms p996 if (!verdict.allowed) throw new Forbidden(verdict.reason)7 8 Object.assign(p, patch) // ~2 microseconds9 await db.profiles.update(userId, p) // 8 ms10 11 return p12 }) // <-- released, ~48 ms later13}14 15// Hold time : ~48 ms typical, ~410 ms at the dependency's p99.16// Throughput : ~20 updates/sec for this lock, on any hardware.17// Blast radius: policy service degrades -> every handler blocks -> the whole18// service is down, and the dashboards say the CPU is idle.19// Bonus bug : if policyService.check() ever calls back into code that takes20// profileLock, this deadlocks. Holding a lock across a call you21// do not control means holding it across code you have not read.1async function updateProfile(userId: string, patch: Patch) {2 const before = profileLock.withLockSync(() => ({ ...profiles.get(userId)! }))3 // snapshot: microseconds4 5 const verdict = await policyService.check(userId, patch) // outside. Slow, and fine.6 if (!verdict.allowed) throw new Forbidden(verdict.reason)7 8 const after = profileLock.withLockSync(() => { // <-- region: ~2 microseconds9 const p = profiles.get(userId)!10 if (p.version !== before.version) throw new Conflict() // someone patched meanwhile11 Object.assign(p, patch)12 p.version += 113 return { ...p }14 }) // <-- released15 16 await db.profiles.update(userId, after) // outside. 8 ms, and fine.17 return after18}19 20// Hold time : ~2 microseconds x2. Throughput ceiling is now ~500,000/sec.21// Blast radius: a slow policy service costs latency on requests that need it,22// and blocks nobody else.23// The price : the check-then-act is now split across the gap, so the second24// region must detect a concurrent change. That is the version25// check — optimistic concurrency, paid for deliberately.Both versions preserve the invariant; the second does it with a hold time four orders of magnitude smaller. The cost is explicit and worth naming: shrinking the region reopened a gap, so the second region has to detect that someone else patched in between. You traded a guaranteed serialisation for a version check plus a retry path — which is the right trade whenever the excluded work is slow, and the wrong one if conflicts are the common case rather than the rare one. See Optimistic vs Pessimistic.
What the hold time does to everybody else
The timeline makes the difference visceral. In the top group, four handlers each need 2 ticks of real work and each waits behind 8 ticks of someone else's network call. The fourth handler's latency is 24 ticks to do 2 ticks of work — and if a fifth arrives, it waits behind all of them. The queue grows faster than it drains, which is the definition of an unbounded queue and the mechanism of a lock convoy.
In the bottom group the same four handlers make their network calls concurrently, because the calls are outside the region, and each one's lock hold is a single tick. Total wall time drops from 32 ticks to 10, and — more importantly — the *shape* changes: the system now scales with the dependency's concurrency rather than with its latency.
The metric that reveals this in production is lock hold time at p99, and almost nobody instruments it. What people do instrument is CPU, which is flat and low throughout the top group, and request latency, which is enormous. Low CPU plus high latency plus a thread dump full of threads blocked on one monitor is the complete diagnosis. See Hold Time, Wait Time, and the Ratio Between Them and Reading a Thread Dump.
A checklist for what belongs inside
The rule generalises past network calls. Anything whose duration you do not control, or that can call back into code you did not write, is a hazard inside a critical section. The matrix below is the review checklist, ordered roughly by how often each one appears in real diffs.
The two rows that surprise people are logging and callbacks. Structured logging that writes synchronously to a socket or a full pipe can block for an unbounded time, and a log line inside a hot region is genuinely a production risk rather than a style question. Callbacks are worse: invoking user code while holding a lock means the lock is now held across whatever that code does, including acquiring another lock or calling back into your own API. That is the classic library deadlock, and the standard fix is to state in the contract that callbacks are invoked without the lock held.
The last row is worth its own emphasis: on a single-threaded async runtime, holding an async lock across an await is not merely slow, because the awaiting task yields and something else runs — including, possibly, a task that wants the same lock and now waits for a network round trip. On the same runtime, holding a *blocking* lock across an await is a guaranteed deadlock, because the only thread is blocked and can never run the holder to completion.
| Inside the region | Duration | Verdict | Why |
|---|---|---|---|
| Reading and writing the protected fields | nanoseconds | REQUIRED | This is the region. Anything less and the invariant is unprotected. |
| Pure computation on those fields | nanoseconds to microseconds | OK if short | Fine if it is arithmetic. Move it out if it is a sort of a large collection. |
| Allocating the object you are about to publish | microseconds | MOVE OUT | Nothing else can reach it yet, so construct it before acquiring and publish inside. |
| A log line | microseconds — or unbounded if the sink blocks | MOVE OUT | A synchronous sink writing to a full pipe or a slow socket blocks for an unbounded time while you hold the lock. |
| A database query | milliseconds | MOVE OUT | Hold time becomes query latency; a slow query plan becomes a service-wide stall. |
| An HTTP call to another service | 10s–100s of milliseconds | NEVER | Hold time is a third party's p99. Their degradation becomes your outage, with idle CPUs. |
| A user-supplied callback | unknown | NEVER | You are now holding the lock across code you have not read, which may acquire another lock or re-enter your API. The canonical library deadlock. |
| Acquiring a second lock | depends | ONLY WITH A DOCUMENTED ORDER | This is where deadlock enters a codebase. If it must happen, the global order goes in a comment at both sites. See Lock Ordering. |
An await on a single-threaded runtime | unbounded | ASYNC LOCK ONLY, AND RARELY | A blocking lock here deadlocks the loop outright. An async lock merely serialises every waiter behind a network call. |
Key points
- A lock held across a call you do not control has a hold time set by someone else, which makes their latency your throughput ceiling and their degradation your outage.
- Throughput through a lock is 1/hold-time. Moving a 40 ms call out of a region takes the ceiling from ~25/sec to hundreds of thousands per second, on the same hardware.
- The availability argument is stronger than the performance one: with the call outside, a slow dependency costs latency only to the requests that need it.
- Shrinking a region usually reopens a gap. Pay for it deliberately with a version check or a conditional update — do not pretend the gap is not there.
- Never hold a lock across a user-supplied callback. You are holding it across code you have not read, and that is the classic library deadlock.
- On a single-threaded runtime, a blocking lock across an
awaitdeadlocks the loop; an async lock merely queues everyone behind a network call. - Logging inside a hot region is a real hazard, not a style preference, when the sink can block.
The loop, answered
Every field is required, which is why no lesson here can recommend concurrency without naming the interleaving that breaks it, the complexity it adds, and the simpler thing to consider first.
- • The region's hold time is the sum of everything between acquire and release, including time the holder spends blocked on something else.
- • Waiters are parked for the whole hold time, so the wait queue length grows as arrival rate × hold time.
- • When arrival rate exceeds 1/hold-time the queue never drains, latency grows without bound, and CPU stays low because everyone is parked.
- • Moving work outside the region shortens hold time directly and usually reopens a window, which must then be closed by detecting concurrent change rather than by excluding it.
- • The snapshot-compute-verify shape is the general pattern: take a consistent copy under the lock, do the slow work outside, re-enter and verify nothing changed before committing.
- • Wide region: A holds the lock through a 40 ms policy call; B, C and D block for 40, 80 and 120 ms. Correct, and the service is effectively serialised on one profile.
- • Wide region under dependency degradation: the policy call takes 400 ms; every handler in the pool is blocked; the health check times out and the instance is removed from the load balancer while its CPU is idle.
- • Narrow region: A and B both snapshot version 7; A commits version 8; B re-enters, sees version 8 against its snapshot of 7, and returns Conflict. Correct, and the conflict is visible rather than silent.
- • Narrow region without the version check: A and B both snapshot version 7, both apply their patches to the current object, and B's
Object.assignoverwrites the field A set. A lost update introduced by shrinking the region carelessly. - • Callback inside the region: A holds
profileLockand invokes a listener that callsgetProfile, which acquiresprofileLock. With a non-reentrant mutex this is an instant self-deadlock on one thread.
- • A narrower region guarantees the same mutual exclusion over a shorter span. It does not guarantee the same *program*: work moved out is work that can now interleave.
- • The snapshot-compute-verify pattern guarantees you detect a conflicting change; it does not guarantee your work was not wasted, because the retry recomputes it.
- • Moving I/O out guarantees your hold time is bounded by your own code. It guarantees nothing about the total latency the user sees — that is unchanged, and only the blocking of others is removed.
- • A documented lock order guarantees no cycle among the locks it covers. It guarantees nothing about a lock acquired inside a callback you invoked.
- • Contention is arrival rate × hold time. Both factors are multiplicative, but hold time is usually the one you can change by four orders of magnitude in an afternoon.
- • At 80% of the 1/hold-time ceiling, queueing theory already predicts wait times several times the service time; the curve is not linear and the experience of "suddenly slow" is that knee. See Queueing: Why Systems Get Slow Before They Get Broken in performance.
- • Moving the slow call out converts lock contention into dependency concurrency — which may reveal a *new* bottleneck at the dependency, and that one is at least visible and boundable. See Bounding Concurrency.
- • Retries introduced by optimistic verification are themselves a contention cost under high conflict rates; the narrow region wins decisively only when conflicts are rare.
- • Lock convoy — hold time exceeds the inter-arrival time and the queue diverges. Latency climbs, CPU stays flat, and the instance fails its health check. See Lock Convoys.
- • Dependency-induced outage — a slow third party blocks every handler, so a partial degradation upstream becomes a total outage in your service.
- • Deadlock through a callback — user code invoked under the lock re-enters and acquires the same or another lock.
- • Lost update from careless shrinking — the region was narrowed without adding conflict detection, so the check-then-act gap is now unguarded.
- • Event-loop deadlock — a blocking lock taken on the only thread, which then awaits the holder that can never be scheduled.
- • Silent regression — a later commit adds a metrics call or a log line inside the region and the hold time quietly returns.
- • On any hot lock: shrinking the region is the highest-yield change available and requires no new primitive, no new dependency and no architectural change.
- • Whenever a dependency is slow or unreliable, because the fix converts a shared-fate failure into a local one.
- • In review of any diff that adds a line to an existing critical section — the checklist gives a yes/no answer in seconds.
- • When shrinking crosses the invariant boundary and nobody adds the conflict check. One statement too narrow is a correctness bug traded for a latency win.
- • When conflicts are common: snapshot-compute-verify then retries constantly, and the retry work exceeds the wait you removed. Under high conflict, hold the lock. See Optimistic vs Pessimistic.
- • When the region is already microseconds and contention lives elsewhere — you are optimising a number nobody measures.
- • When shrinking is achieved by splitting one lock into several without documenting the acquisition order, trading contention for deadlock.
- • Instrument lock hold time as a histogram, not a mean. The p99 is where the accidental I/O shows up, and the mean will hide it completely.
- • Instrument lock wait time separately. Hold time is what you fix; wait time is what the user feels.
- • Take a thread dump during the incident: many threads in BLOCKED on one monitor with one thread inside a socket read is the complete diagnosis in one artefact. See Reading a Thread Dump.
- • Compare measured throughput against the predicted 1/hold-time ceiling. A plateau at that number confirms the lock, not the CPU, is the limit.
- • Add a debug assertion that fails if the region's duration exceeds a budget — a hold-time guardrail catches the regression that a code review misses.
- • The narrow version is longer and requires a version field, a conflict path and a decision about what the caller does with a conflict. That is real complexity bought for real availability.
- • The reasoning for why each excluded statement is safe outside the region lives in the author's head unless it is written down, and it must be re-verified whenever the function changes.
- • Snapshot-compute-verify introduces a retry loop, which introduces a retry budget, which introduces a decision about what happens when retries are exhausted.
- • The "no callbacks under the lock" rule must become part of a library's documented contract, or callers will assume the opposite.
- • Do the whole operation in the database with a conditional
UPDATE ... WHERE version = ?and no process-local lock at all. Works across replicas, which a mutex does not. See The Database Solves Concurrency For Its Data, Not For Your Memory. - • Own the state in a single task and send it messages; the queue provides the serialisation and no caller ever blocks on a lock. See The Actor Model.
- • Publish an immutable snapshot and swap the reference, so readers never take a lock and writers hold it for one word write. See Copy-on-Write as a Concurrency Strategy.
- • Keep the wide lock, and bound how many tasks may attempt it, so the queue is explicit and bounded rather than implicit and unbounded. See Bounding Concurrency.
How much of the task is inside the lock?
Eight threads, one lock
A mutex buys correctness with throughput
What people believe, and what is true
The lock is only held for one function call, so the scope is fine.
Scope is measured in time, not in lines. One function call that performs a network request is a 40 ms critical section.
The database call is fast, so keeping it inside is harmless.
It is fast until an index is dropped, a plan changes or a replica lags. Inside a lock, its p99 becomes your throughput ceiling on the worst possible day.
Shrinking the lock is a pure win.
Shrinking usually reopens a gap. The win is real only when you close the gap deliberately with conflict detection, and it inverts entirely when conflicts are common.
Async locks make holding across await fine.
They make it non-fatal. Every waiter is still queued behind a network round trip, so the throughput ceiling is exactly the same as the blocking case.