fix: bound per-client rate limiter storage with LRU eviction#4057
fix: bound per-client rate limiter storage with LRU eviction#4057strawgate wants to merge 4 commits into
Conversation
|
surely we can find a non-hand rolled ttl dict |
|
Edited to reflect the latest analysis. tl;dr: The Log excerpt — job 76372877049(Three additional identical hunks at lines 282, 296, and 331 for |
e5c8f0f to
a6dde66
Compare
a6dde66 to
30869a2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30869a2e25
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
30869a2 to
d280a50
Compare
🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.7 <[email protected]>
🤖 Generated with Claude Code Co-Authored-By: Claude Opus 4.7 <[email protected]>
RateLimitingMiddlewareandSlidingWindowRateLimitingMiddlewarestored per-client limiters in an unboundeddefaultdictkeyed by client id. Every distinct client id created a permanent entry that was never evicted, so a long-running server facing many transient clients (or attacker-supplied ids) leaked memory without bound.This replaces the
defaultdictwith anOrderedDictused as an LRU cache, capped by a configurablemax_clients(default 10,000). Accessing a client moves it to the most-recently-used end; inserting past the cap evicts the least-recently-used entry. Eviction is safe because an evicted client was idle long enough formax_clientsother clients to be more recent — by which point its token bucket would have refilled (or its sliding window expired) anyway, so it is indistinguishable from a fresh client.max_clients < 1is rejected at construction. The same fix is applied to both middlewares. Note the internal storage attribute is renamedlimiters→_client_limitersto reflect that it is no longer a public auto-vivifying dict.Refs #4053 — fixes Bug 1 (rate-limiter dicts never evicted). Bugs 2 and 3 (FilesystemProvider lazy-lock race, unbounded
_warned_files) are out of scope here.