An operation in this code takes two locks and it deadlocks every few weeks. How would you design the locking so that cannot happen?

What is required for it to happen at all

A deadlock needs a cycle in the wait-for graph: thread one holds A and wants B, thread two holds B and wants A, and neither will release what it has. That is the whole mechanism, and stating it that way is useful because it converts an untestable timing problem into a structural property. If the graph of "which lock may be acquired while holding which" has no cycle, no interleaving can deadlock, whatever the scheduler does.

The canonical shape is an operation over two objects of the same type.

// Deadlocks. Two threads transferring in opposite directions between the
// same pair of accounts each hold one lock and wait for the other.
void transfer(Account from, Account to, long amount) {
    synchronized (from) {
        synchronized (to) {
            from.debit(amount);
            to.credit(amount);
        }
    }
}

Nothing here is wrong locally. Each thread acquires locks in the order its own arguments arrive, which is the natural way to write it, and that is precisely what makes the cycle possible: the acquisition order is a property of the call, not of the program.

Prefer not holding two locks at once

Before adopting an ordering rule, ask whether the nested acquisition is necessary, because a design with no nesting cannot deadlock on these locks at all.

Sometimes the two locks are protecting one invariant, in which case they are the wrong granularity and a single lock covering both objects is simpler and correct. A striped or partitioned lock over the account table, with both accounts falling in the same stripe, expresses that without serialising the entire system. Sometimes the operation can be decomposed into two independently-locked steps plus a durable record that ties them together, which is what a ledger with a pending entry does — slower and more code, but it removes the cross-object lock entirely.

And sometimes the answer is not to lock. If the mutation can be expressed as a compare-and-set on an immutable snapshot, or if the work can be handed to a single owning thread through a queue so that only one thread ever touches the state, the lock graph shrinks to nothing. Confinement is underrated in these discussions: the cheapest concurrency bug to avoid is the one in code that is never run concurrently.

When you must nest, define one total order

Where two locks genuinely have to be held together, the fix is to make acquisition order independent of the caller by deriving it from the objects themselves.

void transfer(Account a, Account b, long amount) {
    if (a.id() == b.id()) throw new IllegalArgumentException("same account");

    // One total order over the locks, derived from a stable key. Any order
    // will do provided every site in the codebase uses this same one.
    Account first  = a.id() < b.id() ? a : b;
    Account second = a.id() < b.id() ? b : a;

    synchronized (first) {
        synchronized (second) {
            a.debit(amount);
            b.credit(amount);
        }
    }
}

The key has to be stable for the object's lifetime and it has to be a genuine total order, which rules out anything that can compare equal for two distinct objects. If there is no natural identifier, the conventional fallback is the identity hash code with an explicit tie-break: when the two hashes collide, take a third lock dedicated to ordering, and hold it for the duration of the nested acquisition. That case is vanishingly rare and the tie-break lock is contended only when it happens.

Two conditions make this a design rather than a local fix. The order must be documented as a rule about lock levels — the account lock is always taken before the audit lock, which is always taken before the notification lock — because ordering pairs consistently is not enough if a third lock joins later. And the rule has to cover locks you do not own: the framework's, a library's internal monitor, the row locks a transaction acquires. Ordering your own two locks correctly while an ORM takes a third in the opposite order gets you nothing.

Open calls are where the cycles actually come from

Textbook examples use two named locks. In real systems the cycle is usually formed by a lock you did not know was involved, because a method invoked a callback while holding one.

Publishing an event to listeners, calling a strategy supplied by a caller, invoking an overridable method, or making a network request from inside a synchronized block all hand control to code that may acquire arbitrary locks, in an order nobody has analysed. The discipline is to make the call outside the lock: compute under the lock, copy what you need, release, then call out. It costs you the guarantee that the notification reflects the current state at the instant it is delivered, which is worth discussing explicitly, but that weaker guarantee is nearly always the right trade for a lock graph you can bound.

The same reasoning forbids exposing a lock to callers. If external code can synchronise on your object, your lock graph is whatever every caller happens to do, and it is no longer analysable at all. A private final lock object is what makes the ordering argument possible.

Timeouts as a fallback, not a design

tryLock with a timeout turns a deadlock into a failed attempt, and it is a genuine tool: it makes the failure visible and recoverable rather than a hung thread. But it is a recovery mechanism rather than a correctness argument, and it comes with two conditions. The operation must be safely abortable and retryable, meaning nothing was half-applied before the second lock failed. And the retry needs randomised backoff, because two threads that both abort and both retry immediately can repeat the collision indefinitely — no deadlock, no progress, which is harder to diagnose than the deadlock was.

Use it as a backstop with a metric attached, so that repeated timeouts tell you a cycle exists that you should be removing.

Why this is a property of the design, not a rare event

In the mid-1980s, the Therac-25 radiation therapy machine delivered massive overdoses to several patients, the result of a race condition in its control software combined with the removal of hardware interlocks that had covered the same fault on earlier models. It illustrates two things this topic keeps returning to: a race is a property of the design rather than a rare event, and software that has absorbed a check previously done in hardware inherits the whole of that obligation. A concurrency review should ask what still holds when two operations interleave in the order nobody drew.

A deadlock is the same kind of claim about the same kind of artefact. The cycle either exists in your acquisition graph or it does not, and the fact that it has fired twice in a year is a statement about traffic patterns rather than about how close you were to being safe. So review the graph, not the incident rate: list which locks are held when which others are acquired, and check for a cycle on paper. When one does fire, a thread dump names the participants and the monitors they are waiting on, and most runtimes will identify the cycle outright — but that is confirmation of a design defect rather than a substitute for having looked.

Deadlock freedom is something you argue structurally, not something you test for. Remove the nesting where you can, derive one total order from stable keys where you cannot, and never hand control to unknown code while holding a lock.