Bound the regex and glob caches - #105
Merged
Merged
Conversation
Both caches are keyed by pattern text and had no eviction, expiry or size cap, so every distinct pattern ever passed in stayed for the lifetime of the process. In the keystroke-driven filter box this library exists for, every prefix a user types is a distinct key, and the case-sensitivity prefix on the key doubled the space again - so the caches grew with everything ever typed rather than with the data being filtered. Route both inserts through AddBounded, which clears at a 4096-entry cap. Clear-and-restart rather than LRU because ConcurrentDictionary keeps no eviction order and tracking one would put a write on every cache hit, the path the cache exists to keep cheap. The check runs only on a miss, where a compile is about to happen anyway. Fixes #103 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012Pup1E96GmvMiRTNMdABat
|
This was referenced Sep 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #103
The defect
RegexCacheandGlobCacheare process-lifetime statics keyed by pattern text, with no eviction, expiry, size cap orClear/TryRemoveanywhere in the file. Every distinct pattern ever passed in stayed cached for the life of the process.That is unbounded growth in the library's intended use. For a keystroke-driven filter box,
"h","he","hel","hell","hello"are five distinct keys, none of which is ever revisited once the user types the next character — andCacheKey's"i:"/"s:"prefix doubles the key space again. The cache grows with everything ever typed, not with the data being filtered, in tools that run indefinitely.The change
Both inserts go through one helper instead of
TryAdddirectly:Clear-and-restart rather than LRU, which is the second of the two shapes the issue offers, and the right one here.
ConcurrentDictionarykeeps no eviction order, so an LRU would mean recording a timestamp or bumping a list node on every cache hit — a write on the exact path the cache exists to keep cheap, and one that would need its own synchronisation to stay correct under the concurrent access the type was chosen for. The bound check instead runs only on a miss, where a regex compile is about to happen anyway, soCount(which does take the dictionary's internal locks) never touches the hot path.4096 entries, the "generously-sized fixed cap" the issue suggests. Large enough that a realistic session never trips it, small enough to bound the growth; a reset costs one recompile per pattern still in use, on the next miss.
MaxCacheEntriesand the two count accessors areinternal, which the test project already sees via the existingInternalsVisibleToinAssemblyInfo.cs. No public API changes.Tests
TheRegexCacheStaysBoundedAsDistinctPatternsArriveTheGlobCacheStaysBoundedAsDistinctPatternsArriveACachedPatternStillMatchesAfterTheCacheHasResetThe third is the one that stops this passing for the wrong reason. A "fix" that never cached at all, or that cleared and then failed to re-add, would satisfy both bound assertions; it has to keep answering the same for a pattern that gets evicted and recompiled.
The assertions are
<=against the cap rather than an exact count, deliberately: these statics are shared across a suite that runs test methods in parallel, so an exact count would be flaky for reasons unrelated to the bound.<=is the actual invariant and holds regardless of what else is running.Proved failing without the fix. Removing only the bound from
AddBounded— leaving the helper, the cap and the accessors in place so the test project still compiles, since it references them:Both fail on cache size — the defect itself.
ACachedPatternStillMatchesAfterTheCacheHasResetpasses in both configurations, which is correct for a no-regression guard. The 84 pre-existing tests are unaffected in both directions.Verification
dotnet build TextFilter.sln -c Release— succeeded, 0 warnings, 0 errors acrossnet10.0,net9.0,net8.0,netstandard2.1,netstandard2.0(analyzers run as errors here)dotnet test TextFilter.sln -c Release— 87 total, 87 passed, 0 failed, 0 skippedRun on .NET SDK 10.0.401, Linux.
Interaction with #104
Heads-up for whoever merges second: open PR #104 (
Fold case invariantly in regex matching) also touchesTextFilter.cs, inDoesMatchRegex— it addsRegexOptions.CultureInvariantto the options a few lines above the insert this change rewrites. Both branch from the samemainand neither depends on the other, but they are close enough in the file that a textual conflict on the second merge is likely. The resolution is mechanical: keep #104's three-flagregexOptionsand this change'sAddBounded(RegexCache, cacheKey, regex);.That PR explicitly deferred this issue — "it is an eviction-policy decision rather than a correctness fix, and folding it in here would put a bounded-cache design in a one-option bug fix" — which is why this is separate.
Worth noting the two compound in the right direction: #104 makes a cached regex answer the same for every caller, and this bounds how many of them accumulate.
🤖 Generated with Claude Code
https://claude.ai/code/session_012Pup1E96GmvMiRTNMdABat
Generated by Claude Code