Skip to content

Sweeper::reclaimable()'s carve-out is wider than the resume it protects — a legacy record failed at a structure-only position is immortal #41

Description

@TBarregren

This was generated by AI during triage.

Split out of the analysis on #25, which deliberately declined to widen itself to cover this.

The finding

Sweeper::reclaimable() and Dispatcher::is_resumable() ask different questions about the same record, and the sweep's carve-out is strictly wider than the resume it exists to protect.

  • The sweep spares a record when is_pre_adaptation_stall() is true.
  • The Dispatcher re-drives one only when is_pre_adaptation_stall() && adapted_budgets() !== null && has_free_slot().

adapted_budgets() is null whenever stalled_budget() returns null — which it does for a structure-only position and for the artifact's sealed index, since neither spends a shrinkable bound. So a legacy (pre-0.6.0) record failed at one of those positions is spared by the sweep forever and re-driven by nobody. It is immortal residue.

This predates the work on #25. It is the same root cause — one predicate serving two callers whose needs differ — which is why it surfaced while analysing that ticket rather than on its own.

What is_pre_adaptation_stall()'s own docblock says

"This is the one definition of that record shape, because two callers must agree about it exactly: the Dispatcher re-drives it, and the TTL sweep spares it while reclaiming every other failed record. A disagreement would have the sweep delete the very container the resume is for."

The docblock anticipates disagreement in one direction — the sweep deleting what the resume needs — and guards against it. The other direction is unguarded, and is the one that actually happens: the sweep sparing what the resume will never come for.

What has to be decided

Whether the sweep should spare a record it can establish is resumable, rather than one merely shaped like a pre-adaptation stall. That is the obvious answer and it is not obviously safe, because the two callers evaluate at different moments: has_free_slot() is a live condition, so a record that is not resumable right now may be minutes later, and a sweep reading is_resumable() verbatim would reclaim a record purely because the slot happened to be busy.

So the fix is probably not "make the sweep call is_resumable()" but "give the resume condition a time-invariant half that both callers can share, with the live half left to the Dispatcher alone". Whoever picks this up should establish that split rather than assume it.

Scope note

The record class involved is exactly the one ADR-0015's penultimate Consequences bullet describes as a back-compatibility cluster that must retire as one piece. Anything done here must leave that cluster reachable — this is a correction to how one predicate is consumed, not a step towards retiring it early.

Blocked by

Nothing technically. Sequence it after #25's follow-up lands, which removes one class of record from the leak (a legacy record that throws) and would otherwise be re-solved here.

Activity

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

    bugSomething isn't workingwontfixThis will not be worked on

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions