Fix negative balance from concurrent transfers with pessimistic row locks

[devin-ai-integration]

Summary

AccountService.transferAmount (and withdraw) did a check-then-write on a detached Account passed in by the controller, with no transaction and no lock. Two concurrent requests both read balance=100, both pass the Insufficient funds check, both write — sender goes negative and money is created.

Reproduction: AccountServiceConcurrencyTest (new, @SpringBootTest against MySQL) fires 10 simultaneous alice -> bob transfers of 100 from a 100 balance. Against the unmodified code:

concurrentTransfersMustNotOverdraw
  [exactly one transfer should succeed from alice's 100 balance] expected: 1  but was: 10
concurrentTransfersInOppositeDirectionsDoNotDeadlock
  [alice and bob balances must conserve the initial total]       expected: 200 but was: 220.00

Both pass after the fix (3 consecutive runs).

Fix — pessimistic locking (SELECT ... FOR UPDATE):

// AccountRepository
@Lock(LockModeType.PESSIMISTIC_WRITE)
@Query("select a from Account a where a.id = :id")
Optional<Account> findByIdForUpdate(@Param("id") Long id);

// AccountService
@Transactional
public void transferAmount(Account fromAccount, String toUsername, BigDecimal amount) {
    toId = findByUsername(toUsername).id;  reject self-transfer
    lock both rows via findByIdForUpdate in ascending id order   // deterministic order => no deadlock
    check balance on the LOCKED sender, not the detached parameter
    debit lockedFrom / credit lockedTo, record both Transactions
    fromAccount.setBalance(lockedFrom.getBalance());  // keep controller's object current for the dashboard view
}

withdraw gets the same lock-then-check treatment; deposit becomes @Transactional so its balance + transaction writes are atomic. The recipient is detached after the username lookup so the subsequent locking query actually refreshes it from the DB instead of returning the already-managed (unlocked) instance.

Why pessimistic locking instead of optimistic @Version:

• The controller hands the service a stale, detached entity. With @Version, the service would have to re-read the sender anyway to get a current version, and any concurrent write would surface as OptimisticLockException needing retry loops in every caller. Pessimistic locks make the service correct regardless of what the caller passes in.
• Contention is expected and short-lived. A transfer is a handful of row operations; blocking the second request for milliseconds on FOR UPDATE is cheaper and simpler than retry-on-conflict, and the user never sees a spurious "please try again".
• Correctness by construction on InnoDB. FOR UPDATE on both rows in a fixed order gives serialisable debit/credit with no lost-update window and no deadlock, which is the standard pattern for ledger updates.
• Optimistic versioning fits better for low-contention, read-mostly entities with user-facing edit forms — not a hot balance column.

Devin-Org: engineering

Link to Devin session: https://app.devin.ai/sessions/fb59df058fbe41d3ac939265188c108b
Open in Devin Desktop: https://app.devin.ai/desktop/session/fb59df058fbe41d3ac939265188c108b?variant=devin
Requested by: @rushcromer

---

Note

Devin errored when opening this Pull Request as rushcromer.
As a fallback, Devin opened this PR as itself.

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring