The on-call log looked normal at first. Two withdrawals, $100 each, against the same account. The second one should have failed. It didn't.
We pulled the handler. The agent had clearly been told to stop double-spend. The code looked reasonable:
func Withdraw(db *sql.DB, accountID string, amount int) error {
var balance int
err := db.QueryRowContext(ctx,
"SELECT balance FROM accounts WHERE id = $1", accountID,
).Scan(&balance)
if err != nil {
return err
}
if balance < amount {
return ErrInsufficientFunds
}
// "Lock the row before writing"
_, err = db.ExecContext(ctx,
"SELECT balance FROM accounts WHERE id = $1 FOR UPDATE", accountID,
)
if err != nil {
return err
}
newBalance := balance - amount
_, err = db.ExecContext(ctx,
"UPDATE accounts SET balance = $1 WHERE id = $2", newBalance, accountID,
)
return err
}
Why the lock did nothing
SELECT ... FOR UPDATE takes a row lock. But it only takes that lock for rows the statement actually returns. By the time the second SELECT ... FOR UPDATE ran, both requests had already read balance = 100 from the first, unlocked SELECT.
Request A: read 100. Request B: read 100. Both hit balance < amount? No. Both take the FOR UPDATE lock — but they take it sequentially, and neither of them re-reads the balance while holding it. Both compute 100 - 100 = 0. Both write 0.
The lock was there. It just didn't cover the read that mattered.
The shape of the fix
There are two ways to make this correct, and they're not interchangeable.
Option 1: hold the lock across the whole read-modify-write, in one transaction.
tx, _ := db.BeginTx(ctx, nil)
defer tx.Rollback()
var balance int
err := tx.QueryRowContext(ctx,
"SELECT balance FROM accounts WHERE id = $1 FOR UPDATE", accountID,
).Scan(&balance)
if err != nil {
return err
}
if balance < amount {
return ErrInsufficientFunds
}
newBalance := balance - amount
_, err = tx.ExecContext(ctx,
"UPDATE accounts SET balance = $1 WHERE id = $2", newBalance, accountID,
)
if err != nil {
return err
}
return tx.Commit()
Request B now blocks on the first SELECT ... FOR UPDATE until A commits. When B wakes up, it reads the new balance — 0 — and returns ErrInsufficientFunds.
Option 2: don't read the balance at all. Make the update atomic.
res, err := db.ExecContext(ctx,
"UPDATE accounts SET balance = balance - $1 WHERE id = $2 AND balance >= $1",
amount, accountID,
)
if err != nil {
return err
}
rows, _ := res.RowsAffected()
if rows == 0 {
return ErrInsufficientFunds
}
Postgres evaluates the WHERE balance >= $1 against the row as it locks it. No read-then-write window, no transaction boilerplate, no way to forget the FOR UPDATE. For simple balance math, this is the version I reach for first.
The review miss
The PR had a test. It called Withdraw twice sequentially. Both passed. The agent's mental model was that it wrote a lock, so concurrent calls are safe. The test's mental model was that two sequential calls should leave the right balance. Neither model included two calls in flight at the same time.
We started recording pairs of concurrent requests — two identical POSTs against the same account, fired within the same millisecond — and replaying them against the local service in CI before merge. Drift between what the code thinks it does and what it actually does under concurrency now shows up as a red test instead of a reconciliation report on Monday morning. That replay loop is something we built locally in Powerduck around recorded request/response pairs; the point isn't the tool, it's that you need a fixture that fires concurrently, not just sequentially.
The takeaway
Adding a row lock is not the same as holding a lock across the read-modify-write. If the lock is on a statement that runs after the value you care about has already been copied into application memory, you have written a very polite no-op.
Two questions to ask in review next time you see FOR UPDATE:
- Does that statement actually run before the read that decides the business rule?
- Is the whole thing in one transaction, so the lock is held until commit?
If either answer is no, the lock is theater.
Top comments (0)