One missing return statement, three years on main

There is a goroutine in Gitea that can busy-spin a CPU core forever, and it has been on main since May 2023. Here is the loop, from routers/web/admin/queue_tester.go:

for {
    select {
    case <-ctx.Done():
    case <-time.After(500 * time.Millisecond):
        ...
    }
}

The ctx.Done() case body is empty. No return. Once the context is cancelled, Done() is permanently ready, the select returns instantly, and the loop spins as fast as the scheduler allows, allocating a fresh time.After timer on every iteration for good measure. The worker function above it has the same empty-Done() shape: after shutdown, its 5-second throttle collapses to instant.

Before you reach for a pitchfork, the honest blast radius: this goroutine only starts when an admin opens the queue monitoring page in non-production mode. It is a dev-mode bug, and our own criticality scoring files it in the low tier for exactly that reason. But it is a real defect, it survived on main for more than three years through the file's creation in PR #24636 and a refinement in #24924, and at the time of the scan a GitHub search for queue_tester in go-gitea/gitea returned zero issues and zero pull requests. In three years, nobody had ever filed it.

This post is about how it was found, along with what the same scan surfaced in VS Code, Excalidraw, cal.com, and our own engine, and what it took to trust any of those findings enough to write them down.

The expensive way and the cheap way

Gitea has fixed this exact bug class before. In 2021, issue #15542, "High Idle CPU usage (fresh install)" was filed by a user who noticed their CPU was busy doing nothing. It took triage across the issue and landed as PR #15733, "Queue manager FlushAll can loop rapidly, add delay": a rapid loop in the same queue subsystem. The polling-loop family keeps recurring there; #33492 is open today.

That is the expensive way to find this bug: ship it, wait years, and hope a user is watching a CPU graph and cares enough to file. The finding above came the cheap way. The Semfora engine now emits what we call facts: deterministic claims about the code that the syntax tree proves, produced by a scan, each with a file and line attached. The fact behind the Gitea finding makes one precise claim: this loop contains no break, no return, and no throw; it exits only by panic. For this loop, that claim is the defect. The missing return in the Done() arm is the whole bug, and the fix is one line.

A true fact is not automatically a bug

The most important design decision in this whole feature is an admission: a provable claim about code structure is not the same thing as a defect. A parked thread can be intentional. So every wild finding gets a verdict from a human reading the flagged source before it goes anywhere: issue, intentional, or idiomatic.

The same scan that caught the Gitea spin also flagged a loop in Meilisearch's compaction route that sleeps in a loop forever after reporting its outcome. The fact is true; the code is deliberate, parking the thread until process restart. Verdict: intentional, worth a source comment, and no more. It flagged sixteen loops in signal-android that look exit-free but are Compose-style tickers, where coroutine cancellation provides the exit. Verdict: idiomatic, and a detector refinement is queued so they stop appearing. It flagged nine read loops in OkHttp's MockWebServer that exit via IO exceptions by design. Verdict: acceptable for a mock server.

Everything in this post that we call a bug survived that reading. Everything that did not is listed as what it is.

Proving the detectors before pointing them at strangers

Before scanning anyone else's code, we built two ground-truth corpuses: small repos with planted bugs, each paired with correct-code controls of the same shape, so a detector that fires on the bug but also on the control fails the test. The infinite-loop corpus plants seven mechanisms (self, mutual, timer, and async recursion, while(true), for(;;), event self-emit); the React rerender corpus plants seven more, from setState-in-render to a two-component ping-pong. Current score: 6 of 7 and 7 of 7 detected, with zero false positives on the controls.

The React ping-pong deserves a sentence, because it is the finding shape that convinced me this approach has legs. A child effect keyed on a function prop it calls, paired with a parent that passes a fresh function on every render whose body sets parent state: every parent render re-fires the child effect, and every fire re-renders the parent. Neither component is wrong in isolation. The loop is the pair, and you can only prove it by holding both components and the call structure between them at once.

What the scan found in the wild

The wild sweep covered 29 real open-source repositories. Beyond Gitea, the verified findings:

Excalidraw, a roughly 90k-star repo, has a popup toggle in ColorPicker.tsx whose comment promises an atomic switch: if another popup is open, close it first, then open this one next tick. The code:

if (appState.openPopup === type) {
  updateData({ openPopup: null });
} else if (appState.openPopup) {
  updateData({ openPopup: type });
} else {
  updateData({ openPopup: type });
}

The else if arm and the final else are byte-identical. Either the close-first behavior was never written, or the branch is vestigial and the comment describes code that does not exist. Caught purely structurally, by a detector for conditionals with identical branches.

VS Code has the same disease twice. In uriTemplate.ts, the reserved-expansion arm of an if/else chain is identical to its fallback (RFC 6570 reserved expansion differs in encoding, which is handled elsewhere, so the branch is vestigial at best). In the notebook chat-editing comparator, an inner ternary reads a.type === 'insert' ? a.modifiedCellIndex : a.modifiedCellIndex, an expression whose condition cannot matter, pasted twice, once for a and once for b. Harmless if intended. But a dead conditional is exactly the shape a rushed edit or an LLM completion leaves behind, and it now sits in the sort logic for AI chat edits.

cal.com produced the strongest performance candidate of the sweep: a deleteMany database write executing once per loop iteration in webhook trigger cleanup, the classic N+1 shape, plus a second transitive case in the same file proven through the call graph. It also produced two instances of the state-mirroring antipattern, where a derived query value is copied into React state through an effect. Those converge, so they are inefficiency and duplication rather than infinite loops, and they are labeled that way.

Most of what a scanner believes is wrong, at first

Here is the part I care most about, and the reason this feature took as long as it did. The first versions of these detectors were confidently wrong in ways that only show up on real code. Signal-android initially produced 580 loop findings; 564 of them were one Kotlin idiom the detector did not understand. Nextcloud produced 4,277 findings from a single PHP resolution mistake; the real number was 4. A first pass at flagging IO inside loops read RegExp.exec in JavaScript string-parsing loops as spawning a process per iteration, and a database cursor draining its own result set as a network round-trip per row.

Every one of those false-positive classes was found the same way: by refusing to publish a claim nobody had verified. Each verified failure became a permanent test or a documented gate. Signal went from 580 findings to 16 true ones. Nextcloud went from 4,277 to 4. A class of Rust macro loops the engine cannot see inside went from 18 false findings in VS Code to 0, by adopting the rule that an unprovable loop is never flagged. When the divergence logic could not prove that a value changes across iterations, the finding was demoted to the weaker claim instead of shipped as the stronger one.

The result is a distribution I trust: on the wild sweep, hundreds of true-but-boring findings stay in the low tier, and the escalation tier is a dozen rows a human can actually read in an afternoon.

The engine flagged itself

The scan also ran on the Semfora engine's own source, which is the fairest test available to me. It found a real one: our coupling analysis spawns one git subprocess per changed file, plus one file read per file, inside the per-file loop. True, and a genuine batching opportunity for large diffs. It also flagged a per-manifest file read that is correct exactly as written, because each manifest is a distinct file read once. One repo, both verdicts, which is the point: the fact was true in both places, and the human verdict is what separated the bug from the design.

Upstreaming

So, the pull requests. Every verified finding above went upstream this week:

• Gitea: PR #38451, the one-line fix for the queue tester spin. Approved by maintainers within a day of filing, and heading for merge as this post goes up.
• Excalidraw: PR #11663, removing the identical conditional branches in the ColorPicker toggle.
• VS Code: PR #325832 for the identical branches in uriTemplate.ts, and PR #325831 for the dead ternary in the notebook comparator.
• cal.com: PR #29776, batching the per-iteration task deletion in the webhook cleanup path.

The four behind Gitea are open and waiting on maintainer review; the projects' automated review has flagged nothing on any of them. The rule for all five is the one this post is built on: nothing gets filed that a human has not verified by reading the source, the report leads with its honest blast radius, and low-severity findings get labeled low-severity by us before a maintainer has to say it.

Gitea's 2021 version of this bug cost a user a CPU graph, a maintainer an investigation, and the project an issue and two pull requests. The 2023 version cost a scan, a source read, and a one-line pull request that was approved the next day.

One more thing, since Semfora is built on top of these projects and dozens like them. Open-source teams deserve more than hand-written fixes for bugs nobody complained about. When Semfora gets big, we commit to giving back to the tools we rely on.