Fixing a race condition in the grant approval workflow
Two admins approving the same grant within milliseconds of each other triggered two divestment flows. The ticket was about to be closed as unreproducible. A concurrency script changed that.
- Date
- Q4 2024
- Role
- Individual contributor
- Stack
- Node.js · TypeORM · PostgreSQL · NestJS
- Scale
- Production fintech platform · multi-admin
- Status
- Shipped Q4 2024
Ops found a duplicate in the divestment ledger. Two records for the same grant, seconds apart, same amount. Compliance flagged it the same day.
The approval flow seemed simple enough. An admin reviews a pending grant and clicks approve. The system checks whether it has already been processed, and if not, triggers a divestment and marks the grant approved. The check was there. It was passing twice.
What had happened was that two admins opened the same grant in separate tabs and clicked approve within a short window. Both requests hit the server before either had committed. Both saw the grant as pending. Both proceeded.
The ticket had been open for two weeks. The team had tried to reproduce it manually. You cannot reproduce a 50ms race window by clicking twice.
The approval had to stay synchronous. Compliance required a direct acknowledgment on the approve action. Moving it to a queue was a process change that would have taken months.
The frontend was owned by a separate team and out of scope. Disabling the button after one click was discussed and deferred. The fix had to live on the server.
The divestment flow touched four tables inside a single transaction. The lock had to hold for the full duration, not just the initial state check.
No schema changes. A migration on the grants table at this volume needed a window we did not have. The fix had to be code only.
First question: was this a concurrency issue or a logic bug? Both look identical in a ledger with no per-step timestamps.
I added logging to the approval handler and went back through
the application logs for the incident window. Two requests with
the same grant ID had entered the handler 13ms apart and both
exited without error. The state check had returned
pending for both.
That confirmed it. The handler was reading state, doing work, and then writing the result. Between the read and the write, the second request was doing the same thing. The read was not protected.
To reproduce it, I wrote a script that fired two concurrent HTTP requests to the approval endpoint for the same grant ID, 1ms apart. Running it against staging produced a duplicate on every other run. The ticket had been open for two weeks. The reproduction took thirty seconds.
You cannot reproduce a 50ms race window by clicking twice. The script that found the bug became part of the test suite.
A TypeORM pessimistic write lock on the grant row. Before doing the state check, the handler now acquires an exclusive lock on the record:
manager.findOne(Grant, { where: { id }, lock: { mode: 'pessimistic_write' } })
This issues a SELECT ... FOR UPDATE. The second
concurrent request blocks at that line until the first
transaction commits. When it unblocks, it reads the updated
state, sees the grant is already approved, and exits without
triggering a second divestment.
The lock held for the full transaction because the divestment flow itself touches four tables. Releasing it after just the state check would have reintroduced the race at a later point.
- Duplicate divestments
- zero post-deploy
- Concurrent approvals handled
- one completes, one exits cleanly
- Schema changes required
- none
- Reproduction script
- added to test suite
The script ran against every subsequent PR that touched the approval handler. It caught one regression in a refactor three weeks later.
Any handler that reads state and then mutates based on it is a TOCTOU race in a multi-user system. The grant handler was not unusual. I found the same pattern in two other flows while fixing this one. The right default for a read-then-write on a shared resource is a pessimistic lock, not an optimistic check.
I would write the concurrency script first, before filing the PR, not as a debugging tool for the bug report. It took thirty seconds to reproduce something the team had spent two weeks trying to reproduce by hand. That is the more transferable lesson from this ticket.
I would also push harder for the frontend button-disable in the same sprint. The lock prevents the data corruption, but it does not prevent a confusing error message if two admins approve concurrently. Defense in depth here is cheap.