inline switch records fixes - #1933
Merged
Merged
Conversation
A snapshot the global switch inlines has no Snapshot call yet, so the patch it queues appends one. Turn the switch off and nothing in the source says that call site was ever inline, and RetireInline only runs where inline is in play, which is what spares a codebase that never used inline a round trip to the queue owner per verification. So the entry stayed pending for good, in the tray, the viewer or staged under obj, and review tooling went on offering it. Accepting it appended a Snapshot call that turned inline back on for that test, since an explicit Snapshot wins over the switch. The run that hands an appended patch over now records its call site under the intermediate directory, before the hand over, so a snapshot is never pending without a record. The first verification of a run with the switch off retires every recorded call site and deletes the records. By the line the entry was queued under, which is the key the owner holds it by, so there is no member fallback to reach a sibling call site's entry instead. Staged copies go with it, wherever an exiting owner or a run with no owner wrote them. Asking the owner what it holds would have found these too, but every run of every codebase would pay for the asking: on Windows a connect to a port nothing listens on waits out its timeout. With nothing recorded this costs one directory check per process. Per intermediate directory, so per configuration and target framework: a switch that is on for one framework and off for another does not have the second retiring what the first just queued. A retire answers to the switches a queue does, and a verification that cannot reach the owner leaves the records for one that can. Entries queued before this change have no record, so they are not retired.
A record that could not be read was deleted as if malformed, stranding the entry it named; an unreadable one is now left for a later run. A record whose retire threw took every later verification of the process down with it, since neither the once-per-process flag nor the delete was reached; the retire is now guarded per record and the flag set regardless. The flag also latched when the intermediate directory was not yet known, which a verification through the raw api can reach; it no longer does. A record outlived its call site: nothing deleted it once an explicit Snapshot call was accepted there, and the switch-off retire, keyed by line alone, then dropped that call's own pending snapshot. A switched-on verification that finds the call site is no longer one the switch appends to now forgets the record. The remarks and docs claimed more than the mechanism gives: per-framework isolation only for a framework whose switch was never on, records surviving a Clean target but not an obj wipe, nothing recorded for a project without an intermediate directory, and a running owner that does not answer being the one send whose record is lost.
Breaking the switch records and retire paths on purpose showed most of what they promise was held by no test. A record outliving the Snapshot call its snapshot was accepted into, a call site the switch declines keeping its pending entry, a declined Snapshot call retiring the verify call's line rather than its own, a retire that throws failing the verification, and the retire running after the run's own queue all went unnoticed. So did retiring on a build server or with DiffRunner disabled, keeping malformed records, and skipping a record whose test has since been deleted. Each now has a test that fails when it breaks. The declined Snapshot call is asserted on the wire in InlineRetireTests, since it does not involve the switch.
This was referenced Sep 23, 2026
Merged
Merged
This was referenced Sep 25, 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
1 participant
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.
Follow-up to the inline switch records (#retire-inline-after-switch-off). A review of that commit turned up cases where a record was lost or misapplied, and claims in the remarks and docs that the mechanism does not back.
Behaviour
finally.InnerVerifierAPI can reach before the adapters assign the assembly. It no longer latches in that case.NotInline, or declined) now forgets the record.Wording
DiffRunner.RetireInlineto return the send result.