Skip to content

LLM quota reservation: check-then-insert TOCTOU not closed under cold-start concurrency (redesign) #1435

Description

@Chris0Jeky

Current disposition (2026-09-22)

The original single-database/WAL diagnosis below is superseded by PR #3280's diagnostic evidence: simultaneous first access to WebApplicationFactory.Services created multiple physical SQLite databases. Capturing one provider before concurrent reservations restores the intended single-database test. The proposed production keep-alive/redesign is therefore not justified by that failing harness.

PR #3280 restores the four quarantined boundary tests and adds fresh-file, independent-connection coverage. It is currently being requalified against main; this issue remains open until that qualification and merge complete. The evidence is same-process and SQLite-specific. It neither qualifies cross-process behavior nor explains every historical run.

The original report is retained below as historical investigation, not the active data-model contract.


Summary

The atomic LLM-quota reservation (#1313, PR #1427) does not close the check-then-insert TOCTOU under cold-start concurrency. The conditional INSERT ... SELECT ... WHERE (SUM/COUNT) < limit and every in-process serialization variant tried still allow 2–4 concurrent boundary-crossers to each read a pre-write total and all reserve. The non-concurrency behavior (recovery insert, estimate floor, settle/release paths) is correct and ships in #1427; only the atomicity guarantee is deferred here.

Mechanism

The reservation decision is a conditional insert gated on an aggregate read (SELECT SUM(tokens) / COUNT(*)). In SQLite WAL that read is evaluated against a read snapshot taken before the write lock is acquired. busy_timeout serializes lock acquisition, not snapshot content — so two writers can each hold a snapshot that predates the other's commit and both pass the predicate. SQLITE_BUSY_SNAPSHOT (the intended protection) is not raised on the paths exercised here.

What was tried and ruled out (empirically)

  • EF-pooled _context connection running ExecuteSqlInterpolatedAsync — over-admits (2+ inserts).
  • Dedicated freshly-opened SqliteConnection, Pooling=false — reduces but does not eliminate.
  • Explicit BEGIN IMMEDIATE via BeginTransaction(deferred: false) (proven to give perfect mutual exclusion in isolation: 0 overlaps / 200 bursts).
  • Dropped the redundant PRAGMA journal_mode=WAL read on the dedicated connection (it seeds a pre-BEGIN read snapshot) — no material change.
  • Process-wide static SemaphoreSlim(1,1) GLOBAL full-span lock wrapping the entire open→pragmas→BEGIN IMMEDIATE→sweep→INSERT→commit→close span, acquired before any DB touch, released only after close — still ~15% over-admit at cold start, including found=4 (all four racers admitted).

found=4 under a global full-span lock is the conclusive proof: a SemaphoreSlim(1,1) always serializes, so this is not an ordering failure — all four serialized reserves each read SUM=0, i.e. a read-visibility failure, which in-process serialization is structurally unable to fix.

Empirics

  • Warm: 0 over-admit over 6000 bursts (20 processes × 300 maximally-simultaneous 4-way bursts, thread-pool + pipeline pre-warmed) — deterministic once warm.
  • Sequential (spaced reserves, scopes disposed between): 200/200 correct — cross-connection visibility works when not cold+concurrent.
  • Cold first burst: ~15% over-admit even under the global lock (found 2/3/4), zero exceptions.
  • Background hosted services stripped (RemoveAll<IHostedService>()): still 20/40 fail ⇒ it is a same-process cold-start effect, not background-worker WAL churn.

Root cause

Cold WAL -shm read-visibility: before any persistent connection has warmed/held the shared-memory index, the four racers' concurrent EF-context/-shm initialization at cold start produces stale fresh-connection reads (racer N does not see racers 1..N-1's committed rows). Serialization downstream cannot repair a snapshot taken during that cold-init window.

Untried lead (redesign candidate)

An eager keep-alive connection opened at application startup (hosted service / DI singleton, one per DB file) that keeps the WAL/-shm warm and held before the first reserve. The warm-0/6000 result predicts this closes the cold-start window. Unproven — needs startup wiring; a lazy keep-alive (opened on first reserve) does not help because the racers establish it concurrently with the first burst (measured 12/40).

Honest note on production impact

Production almost certainly warms -shm before real traffic (startup migration Database.Migrate(), first-run bootstrapper, and pooled EF connections from the workers/first requests), so the vulnerable window is the very first concurrent same-user boundary-crossing burst immediately after a cold process start — narrow, single-host, and possibly test-concentrated (the gate tests hit iteration 0 with a freshly-booted factory and no warmup). It is a real bounded residual, not a steady-state hole.

Disposition

  • The concurrency-guarantee tests (2-way boundary + 4-way burst, token AND request) are quarantined with [Fact(Skip = "…#<this issue>…")] in PR LLM quota reservation robustness hardening (atomicity TOCTOU deferred to #1435) #1427 so CI is green for the shippable hardening.
  • Redesign options for the maintainer: eager keep-alive -shm warmer (above); or a design that does not gate the decision on a snapshot-based aggregate read (e.g. a monotonic per-user counter/row-lock the writer must take, or moving enforcement to a single-writer component).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Priority IIThe active direction's next tranche: wedge capabilities, significant defects, near-term hardening.tech-debtDebt cleanup or deferred engineering quality work with limited product-surface change.

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions