Skip to content

RateLimiter memory storage: unlocked eviction, full-store sort, inline cleanup scan #2971

Closed

Description

@bpamiri

Problem

The memory-storage path of vendor/wheels/middleware/RateLimiter.cfc was a held package in the 2026-06-09 review remediation (held: file occupied by PR #2913, which fixed database storage only) and ended up tracked nowhere — #2911's scope is exclusively database storage. Verified on develop:

  • handle() calls $evictOldest() outside any lock (RateLimiter.cfc:141-143) — concurrent requests race the eviction bookkeeping.
  • Emergency eviction ends in an ArraySort closure-comparator over the entire remaining store (:578-580) on the request thread.
  • $maybeCleanup runs a full key scan inline on the request thread (:435-501).

Impact

Under memory storage at high key cardinality: races on shared store state, and latency spikes on the unlucky request that triggers eviction/cleanup.

Suggested approach

Guard eviction/cleanup with the existing named-lock discipline (double-checked, lock only on trigger), replace the full sort with a cheaper eviction heuristic (e.g. sampled LRU or expiry-bucket sweep), and bound per-request cleanup work.

Acceptance criteria

  • No unguarded writes to the shared memory store.
  • Eviction cost bounded (no full-store sort on the request thread).
  • Specs covering concurrent eviction and cleanup-bound behavior.

Source

Internal framework review 2026-06-09 (held package middleware-memory-store), coverage audit 2026-06-10. Note: the duplicate-validation dead-code finding (MA1) from the same package is already fixed on develop — excluded here.

Activity

  1. wheels-bot commented on Jun 10, 2026

    @wheels-bot
    Contributor

    Wheels Bot — Triage

    Classified as bug.

    Suspected layer

    middleware — see .ai/wheels/middleware/ for canonical patterns (CLAUDE.md § "Middleware Quick Reference" and vendor/wheels/middleware/ for sibling implementations).

    Fix sketch

    All changes localize to vendor/wheels/middleware/RateLimiter.cfc. Around :141-143, wrap the eviction trigger in the existing named-lock discipline (double-checked: re-check StructCount(store) >= maxKeys inside the lock to avoid redundant evictions under contention). Replace the full-store ArraySort comparator at :578-580 with a bounded heuristic — either sampled-LRU (pick K random entries, evict the oldest) or expiry-bucket sweep — so eviction cost is O(K) not O(N log N). For $maybeCleanup at :435-501, cap per-call work (max keys scanned per request, or move to a sampled sweep) so the unlucky request doesn't pay for the full store. Reference: the database-storage path was hardened in #2913; this brings the memory path to parity.

    Confidence: medium

    The report is clear with cited line numbers and a concrete acceptance list, but the fix involves design trade-offs (sampled-LRU vs expiry-bucket sweep) and auto-downgrades from high per the vendor/wheels/middleware/** rule.

    <!-- wheels-bot:triage:2971 -->
    <!-- wheels-bot:triage-class:bug -->
    <!-- wheels-bot:triage-confidence:medium -->

  2. wheels-bot commented on Jun 10, 2026

    @wheels-bot
    Contributor

    Wheels Bot — Fix proposed

    Draft PR: #2983

    Spec: vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc — 4 new validation specs (failing → passing) plus 2 behavioural smoke tests covering the flooded-store eviction path and the bounded-cleanup path.
    Implementation: vendor/wheels/middleware/RateLimiter.cfc — locked emergency eviction with double-checked locking, bounded-sample LRU eviction (new evictionSampleSize knob, default 64), bounded per-call cleanup scan (new maxKeysScannedPerCleanup knob, default 5000), Wheels.RateLimiter.InvalidConfiguration validation on both new knobs.

    A human review is required before merge. Reviewer A and Reviewer B will weigh in shortly. Doc updates (MDX guides, .ai/wheels/, CLAUDE.md) are handled separately by bot-update-docs.yml.

  3. added a commit that references this issue on Jun 10, 2026
    4b80a9a
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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions