← Bank
Code Review

Race Condition on a Balance Update

Code ReviewSenior~12m
concurrencyrace-conditiondebuggingatomicity

Prompt

Finance has a reconciliation bug: a few accounts a day end up with balances that don't match the sum of their transactions, almost always a too-high balance after a burst of concurrent withdrawals. It's rare, never reproduces in a single test, and only shows under load. This withdraw handler runs on every worker. Debug it — form a hypothesis before you change anything.

async function withdraw(accountId, amount) {
  const account = await db.query(
    "SELECT balance FROM accounts WHERE id = $1",
    [accountId]
  );
  const balance = account[0].balance;

  if (balance >= amount) {
    const newBalance = balance - amount;
    await db.query(
      "UPDATE accounts SET balance = $1 WHERE id = $2",
      [newBalance, accountId]
    );
    return { ok: true, newBalance };
  }
  return { ok: false, reason: "insufficient_funds" };
}

How this round runs

State the race precisely before touching code, then tell me how you'd reproduce and confirm it, what the atomic fix is, and what you ruled out. "Only under load, balances too high, rare" should drive your hypothesis.

Model answer

Hypothesis, stated before any change: this is a read-modify-write race — a check-then-act on a shared row. The symptom set points straight at it: rare, load-only, and balances too high after concurrent withdrawals. That "too high" is the signature of a lost update.

The race, precisely: the handler SELECTs the balance, decides in application memory, then UPDATEs with an absolute value it computed from the stale read. Two withdrawals on the same account interleave like this — start balance 100, each withdrawing 100:

  • T1 reads balance 100. T2 reads balance 100 (before T1 writes).
  • T1 checks 100 ≥ 100, computes newBalance 0, writes 0.
  • T2 checks 100 ≥ 100 (its stale read), computes newBalance 0, writes 0.

Both succeed, 200 leaves the account, but the balance is 0 instead of −100 — and in the partial-withdrawal case T2's UPDATE balance = $1 clobbers T1's write entirely (last-writer-wins on an absolute value), so a withdrawal silently "refunds" itself and the balance lands too high. That matches the report exactly.

How I'd reproduce and confirm — because it won't show in a serial test: fire many concurrent withdraw calls at one account (e.g. Promise.all of 50 withdrawals that should overdraw it, or a load tool) and assert the final balance equals start − sum(successful). It'll pass single-threaded and fail under concurrency. To confirm the mechanism rather than just see the symptom, I'd check the DB isolation level (default Read Committed permits exactly this lost update) and add temporary logging of the read balance vs the written balance per request — you'll see two requests read the same value.

The fix is to make the check-and-decrement atomic so the database, not application memory, enforces the invariant. Do the conditional decrement in one statement and make the write relative, not absolute:

async function withdraw(accountId, amount) {
  const rows = await db.query(
    `UPDATE accounts
        SET balance = balance - $1
      WHERE id = $2 AND balance >= $1
      RETURNING balance`,
    [amount, accountId]
  );
  if (rows.length === 0) return { ok: false, reason: "insufficient_funds" };
  return { ok: true, newBalance: rows[0].balance };
}

balance = balance - $1 is computed by the DB under a row lock, and the WHERE balance >= $1 guard runs in the same atomic statement, so an interleaving withdrawal either sees the already-decremented balance or fails the guard — no lost update. Equivalent alternatives: SELECT … FOR UPDATE to lock the row before the read-modify-write, an optimistic version column with a compare-and-set on update, or SERIALIZABLE isolation with retry-on-conflict. The relative single-statement update is the simplest and avoids holding a lock across app-side logic.

What I ruled out: not a math bug (the subtraction is correct in isolation), not a caching/replica-lag read (it reproduces against the primary), not a missing index or slowness (this is a correctness race, not latency), and not double-submit on the client alone (the same row racing from two workers reproduces it server-side even with distinct requests). The defect is specifically check-then-act on shared mutable state without atomicity or a lock.

Signals — what a strong answer shows
  • Stated the read-modify-write / check-then-act race before editing, and derived it from the symptom (load-only, rare, balance too high)
  • Walked a concrete interleaving showing the lost update / last-writer-wins, not just 'there's a race'
  • Gave a reproduction that's concurrent by construction and an assertion (final == start − sum) that fails only under load
  • Fixed it atomically — single-statement conditional relative decrement — and listed real alternatives (FOR UPDATE, optimistic version, SERIALIZABLE+retry)
  • Explicitly ruled out math, replica lag, indexing, and client double-submit, with reasons
Follow-ups — where it goes next
  • Why does the balance come out too high rather than too low? → Absolute-value UPDATE means the later stale write overwrites the earlier one (last-writer-wins), erasing a decrement.
  • Does wrapping it in a transaction fix it? → Not at Read Committed — both reads still see the old value. You need a lock (FOR UPDATE), an atomic conditional update, or SERIALIZABLE with retry.
  • How would you prove it's fixed? → Re-run the concurrent overdraw test; final balance must equal start − sum(successful) and over-withdrawals must be rejected.
Study this: Programming
// write your answer here