drizzle-team / drizzle-team/drizzle-orm
[Bug]: Cache invalidation runs concurrently with DB write, causing stale-cache repopulation race
- Dominant language
- TypeScript
- Stars
- 35.8k
- Forks
- 1.6k
- Avg merge
- 2d 7h
- Merged PRs (30d)
- 4
Description
## Summary
In `PgPreparedQuery.queryWithCache`, the cache invalidation (`onMutate`) is fired **concurrently** with the database mutation via `Promise.all`. The code comment on line 86 explicitly states the intended ordering — "query the database, wait for a response, and **then** perform invalidation" — but the implementation contradicts this and runs both operations simultaneously.
## Affected Code
**File:** `drizzle-orm/src/pg-core/session.ts`, lines 86–101
```ts
// For mutate queries, we should query the database, wait for a response, and then perform invalidation
if (
(
this.queryMetadata.type === 'insert' || this.queryMetadata.type === 'update'
|| this.queryMetadata.type === 'delete'
) && this.queryMetadata.tables.length > 0
) {
try {
const [res] = await Promise.all([
query(), // DB write
this.cache.onMutate({ tables: this.queryMetadata.tables }), // cache invalidation
]);
return res;
} catch (e) {
throw new DrizzleQueryError(queryString, params, e as Error);
}
}
```
## Root Cause
`Promise.all` starts both the DB write and the cache invalidation at the same moment. The race window is:
1. `onMutate` completes (cache keys deleted) while the DB write is still in-flight.
2. A concurrent SELECT hits the now-empty cache, calls `query()` against the database, and reads the **pre-mutation row** (the write has not committed yet).
3. That stale result is written back into the cache via `cache.put(...)`.
4. The DB write later commits — but the cache now holds the old value and will serve it to all subsequent readers until TTL expiry.
With the Upstash adapter, `onMutateScript` performs a Lua `DEL` on the composite key sets. Any `cache.get()` that races between the DEL and the DB commit will repopulate the cache with pre-write data that can persist for the full configured TTL (defaulting to 1 second, but configurable to much longer values via `CacheConfig.ex`).
## Steps to Reproduce
1. Configure drizzle-orm with `UpstashCache` using a non-trivial TTL (e.g. `ex: 60`).
2. Perform a high-frequency read/write workload on the same table from multiple concurrent connections.
3. Issue an UPDATE followed immediately by a SELECT on the same key.
4. Observe the SELECT returning the pre-update value from cache, persisting for up to `ex` seconds even though the DB already reflects the new value.
The race is reliably reproducible under any concurrent load because `onMutate` (a single Redis Lua script call) typically completes faster than a round-trip DB write, leaving a window between cache clearance and DB commit.
## Impact
Any application using drizzle-orm's caching layer (Upstash or custom `Cache` implementation) is vulnerable to serving stale data after a mutation for the duration of the cache TTL. In write-heavy workloads this can result in:
- Users reading stale rows immediately after an update (e.g. an account balance, an order status, a permission flag).
- Business-logic inconsistencies when downstream code acts on cached pre-mutation state.
- The longer the configured `ex` TTL, the wider the stale-data window.
## Suggested Fix
Await `query()` before calling `onMutate`, matching the stated intent in the comment:
```ts
// For mutate queries, we should query the database, wait for a response, and then perform invalidation
if (
(
this.queryMetadata.type === 'insert' || this.queryMetadata.type === 'update'
|| this.queryMetadata.type === 'delete'
) && this.queryMetadata.tables.length > 0
) {
try {
const res = await query();
await this.cache.onMutate({ tables: this.queryMetadata.tables });
return res;
} catch (e) {
throw new DrizzleQueryError(queryString, params, e as Error);
}
}
```
This ensures the DB write has committed before any cache keys are invalidated, closing the repopulation race window entirely. The same pattern should be audited in the MySQL (`mysql-core/session.ts`) and SQLite (`sqlite-core/session.ts`) equivalents if they share the same `queryWithCache` implementation.
Contributor guide
Assessment
This issue has not been assessed yet.