fix: unhang index builds on pathological tokens, and name the file when one hangs - #165
Merged
Merged
Conversation
…en one hangs The shape gate in content-token extraction used a catastrophic-backtracking regex (a quantified group inside a trailing `+`). One long mixed-case run that fails the match sends it exponential: 2x per 2 characters, so ~90 characters never finishes. Only a run containing an underscore can fail this way, since the token scanner accepts `_` and the camelCase pattern does not, which is why base64url and JWT fixtures triggered it. The keeper span at 100% CPU holding its lock with the log frozen at "Parsing files...", and killing it was the only recovery before the next build hit the same file. Rewrite the four patterns without nested quantifiers, plus a length cap as defence in depth. Verified equivalent across 400,018 generated tokens plus the existing corpus, with zero disagreements. Because that failure blocks the event loop, no timer, signal handler or async write can report it. Add an in-flight record written synchronously before each unit of work and cleared on clean completion, so a record outliving its process is the diagnosis. Surfaced by `status`, by a warning at the next keeper start, and durably in repair.jsonl. The mark is invoked inside parseFile rather than at the call site: the caller runs a batch through Promise.all, and parseFile's awaits let every file get past the call site before any reaches the CPU-bound work. Reproduced against a real hang, where the call-site version named an innocent file. Also gate the `inProgress` stamp in `status` on the recorded pid being alive. KeeperWork's contract says a stamp outliving its keeper is harmless because readers check first; waitForCacheHead did, this renderer did not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AkashGoenka
force-pushed
the
fix/index-hang-and-inflight-diagnostics
branch
from
August 31, 2026 18:41
4901f69 to
29fd9e3
Compare
Owner
Author
|
Arun Francis M - thanks for reporting this high severity issue. This has been fixed in v2.3.2 |
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.
What
Reported by a user whose index build hung indefinitely. Root-caused, fixed, and then made the same class of failure diagnosable.
The hang
The shape gate in content-token extraction used a catastrophic-backtracking regex — a quantified group inside a trailing
+:One long mixed-case run that ultimately fails the match sends it exponential. Measured growth is 2× per 2 characters:
The trigger is narrower than it appears, which is why it showed up in real fixtures: the token scanner accepts
_but the camelCase pattern does not, so only a mixed-case run containing an underscore can fail this way — base64url blobs and JWT signatures exactly. A pure alphanumeric camelCase run matches fast and is harmless.Extraction runs on every file in every language, so the repo's language was never the point.
Symptoms, all reproduced: keeper at 100% CPU holding its lock, log frozen at
Parsing files...,initblocked for its full 180s, and killing the process the only recovery — after which the next build hit the same file and hung identically.The four patterns are rewritten without nested quantifiers, plus a token-length cap as defence in depth. Verified equivalent to the old patterns across 400,018 randomly generated tokens plus the existing corpus: zero disagreements.
The diagnosability gap
The bug was one regex; the hours went to not being able to see it. The log named no file, and
statusactively misled.That failure blocks the event loop, which rules out every usual approach — watchdog timer, progress interval, signal handler, async write are all structurally unable to run once the spin starts. So the indexer now writes what it is about to do before doing it, synchronously, and clears it on clean completion. A record that outlives its own process is therefore the diagnosis.
Repeated as a warning at the next keeper start and appended to
repair.jsonlasdied-in-progress, since startup is the last moment that evidence exists before the next build overwrites it.Also fixed
coldstart statusrendered theinProgressstamp without checking the recorded pid was alive.KeeperWork's own contract says a stamp outliving its keeper is harmless because readers check first —waitForCacheHeaddid, this renderer did not. Since killing the keeper was the only escape from the hang, the one command you would reach for while diagnosing it was guaranteed to lie.Notable
The first version of the in-flight record was wrong, and only an end-to-end test caught it. Marking at the call site reported an innocent file while a different one span at 100% CPU:
parseFileawaits a grammar load and a file read, and the caller runs a batch of 100 throughPromise.all, so every file gets past the call site before any reaches the CPU-bound section. The mark now lives insideparseFile, immediately before the synchronous work with noawaitbetween. Pinned by a test and by a comment at the signature, because a diagnostic that confidently blames the wrong file is worse than none.Testing
statusfix was verified to fail without the change, so it isn't a no-op assertion.statusbranches render.Notes
Notebook notes ride in a separate commit (
kb commit), per repo convention.