AI Code Review for Race Conditions and Concurrency Bugs: The Seven Shapes a Reviewer Should Catch

Race conditions are the bugs that pass every test, ship on a Friday, and show up in a support ticket three weeks later as “sometimes the order is charged twice”. They are also the bugs a reviewer is least likely to catch, because the code that contains them is usually correct on every line. The problem is between the lines: two of them running at once. This post is about what an AI code reviewer can and cannot do about concurrency bugs, the concrete patterns worth putting in your review instructions, and where a reviewer that can read the whole repository earns its keep.

As with our posts on migrations and infrastructure as code, most of this applies to any reviewer, human or model. Where PURA does something specific, we say so.

Why concurrency bugs survive review

A diff shows you a sequence of statements. A race condition is a property of two sequences interleaving, one of which is often not in the diff at all: the other request handler, the background job, the retry, the second tab. Three things make this worse than the usual “context outside the diff” problem.

First, the failing interleaving is rare, so tests do not reproduce it and the author has never seen it. Second, the fix is frequently somewhere other than the bug: the correct change for a double-charge is a unique constraint or an idempotency key, not a change to the charging code. Third, the vocabulary is unfamiliar to many application developers. “Check-then-act”, “lost update” and “TOCTOU” describe most production races, and a reviewer who knows the names can pattern-match in seconds; one who does not has to reason from first principles every time.

That last point is the case for an AI reviewer here. Models have read an enormous amount of concurrency literature and post-mortems. They are good at recognising the shapes. What they are not good at, unless given the material, is knowing which shapes matter in your system: whether the handler runs on one process or forty, whether the database isolation level is read committed or serializable, whether the queue delivers at least once.

The seven patterns to check

1. Check-then-act on shared state

if (!exists(user.email)) create(user). Between the check and the act, another request did the same thing. The reviewer should flag any read followed by a conditional write to the same record or key, and ask whether the invariant is enforced where it can actually be enforced: a unique index, an upsert, an atomic compare-and-set, a SELECT ... FOR UPDATE, or a lock. A comment that says “this is racy” is half a review; the useful comment names the enforcement mechanism available in the stack the repository uses.

2. Read-modify-write without atomicity

balance = get(); balance += amount; set(balance). Two concurrent writers and one update is lost. In SQL this is UPDATE ... SET balance = balance + ? versus reading, adding and writing back; in Redis it is INCRBY versus GET/SET; in memory it is an atomic or a mutex versus a plain field. The reviewer should look for the three-step shape anywhere the value is shared across requests, threads or processes, and should be especially suspicious of counters, balances, inventory levels and anything called count.

3. Non-idempotent operations behind retries

Retries are a concurrency feature: a retried request can run concurrently with the slow original it assumed was dead. Any handler that creates, charges, sends or increments needs either an idempotency key or a natural one (the order id, the message id). The reviewer should trace where retries come from (the HTTP client's policy, the queue's redelivery, the user's double click) and check whether the operation at the end of that chain tolerates being run twice. This is the single most valuable check on any PR touching payments, email or outbound webhooks.

4. Time-of-check to time-of-use on files and permissions

if (canAccess(path)) open(path). The classic TOCTOU: the file, the permission or the symlink changes between the two calls. The fix is to perform the operation and handle the failure, rather than check first. Reviewers should flag permission checks that are separated from the guarded action by anything that could yield, including an await.

5. Shared mutable state in async code

Single-threaded runtimes are not race-free. In JavaScript and Python async code, every await is a point where other work runs, so a function that reads a module-level variable, awaits, and then writes it has the same lost-update problem as a multithreaded one. Module-level caches, request-scoped state stored on a singleton, and “temporary” globals are where this lives. The reviewer should ask what else can touch that variable during the await.

6. Lock ordering and lock scope

When a PR adds a lock, the questions are: which other locks can be held when this one is taken, in what order (two locks taken in different orders by two code paths is a deadlock waiting for load), is the lock released on every exit path including exceptions, and does the critical section do I/O while holding it. A reviewer with repository access can enumerate the other call sites that take the same lock; a diff-only reviewer cannot.

7. Event ordering and at-least-once delivery

Consumers that assume events arrive once and in order are correct until the broker has a bad day. The reviewer should check whether handlers tolerate duplicates (idempotency again), reordering (version numbers or timestamps compared before applying) and partial failure (what happens if the handler crashes after the side effect and before the ack). Our dependency updates post covers the related problem of a queue or client library changing its delivery semantics under a consumer.

Turning this into review instructions

Generic instructions (“look for race conditions”) produce generic comments. The version that works names the shapes and the enforcement tools available in your stack. Something like:

“For any change that reads and then writes shared state (database rows, cache keys, module-level variables, files), identify whether the read and write are atomic. If not, say which concurrent caller could interleave and recommend the specific mechanism: a unique constraint, an atomic UPDATE, INCRBY, SELECT FOR UPDATE, or an optimistic version column. For any handler that creates, charges, sends or publishes, state whether it is idempotent under retry and name the idempotency key. For any new lock, list the other places the same lock is taken and check ordering and release on error paths.”

That paragraph is short enough to live in a repository's review configuration and specific enough that the reviewer's comments reference your tables and your cache. The custom rules post explains how to scope instructions like this to the paths where they apply, so a CSS change does not get a lecture on isolation levels.

What repository context changes

Every pattern above has a question of the form “what else touches this?”. Which other handlers write this row. Which other code paths take this lock. Whether the constraint the author is relying on actually exists in the schema. Whether the queue library is configured for at-least-once. A reviewer that only sees the diff has to guess, and tends to guess “probably fine”. PURA runs its review agent against a checkout of the repository, so an instruction such as “list every writer of this table” or “find the migration that adds the unique index this code assumes” is something it can actually do, and the resulting comment either points at the enforcement or says plainly that it could not find one. We wrote about this general failure mode in why diff-only reviewers miss real bugs, and concurrency is where it costs the most.

What a reviewer cannot do

It cannot run the race. A model can tell you that two code paths can interleave; it cannot tell you how often they do in production, whether your database's isolation level already prevents it, or whether the load pattern makes it a one-in-a-million event or a daily one. It also cannot see infrastructure it is not shown: the connection pool size, the number of workers, the broker settings. Treat its findings as a list of interleavings to think about, verify the important ones with a deliberately concurrent test (two requests fired at once, in a loop, against a real database), and put the enforcement in the data layer where it survives the next refactor.

A short checklist for the PR template

• Does any read-then-write touch state shared across requests, workers or processes?
• Is every create, charge, send or publish idempotent under retry, and what is the key?
• Is the invariant enforced in the data layer or only in application code?
• Does any await, callback or yield sit between a check and the action it guards?
• For a new lock: ordering, release on error, no I/O inside.
• For a new consumer: duplicates, reordering, crash-after-side-effect.

None of this makes concurrency bugs easy. It makes them findable at review time, which is the cheapest time, by a reviewer that knows the shapes and can go and look at the rest of the code. The alternative is the support ticket.