[JohnCampionJr]

The race

A host raises TerminalBuffer.Trimmed on its PTY reader thread. BufferSearch.OnTrimmed compacted the hit list and called RemoveRange on it right there. Meanwhile the UI thread could be stepping through the hits, or drawing from a HitsOnRow span over the list's backing array. So a held span could be rewritten in place mid-draw, and a move could index past a list that was shrinking.

Fix

The buffer now keeps a running count of trimmed lines, TerminalBuffer.TrimmedLines, readable from any thread. BufferSearch no longer subscribes to Trimmed at all. Its reader thread compares the count with the mark its hits were last rebased to. That is one volatile long read per call. It shifts the hits before Count, CurrentIndex, HitsOnRow or the moves look at them.

There's no lock on the per-frame path, and a span can only change on the thread that asked for it.

Because the count lives on the buffer, trims on the alternate screen are now counted even when the search was created on the normal one. Find also searches the screen buffer now, rather than the one-row status buffer while a program writes its status line.

Off-UI-thread search: BufferScan

A host can search in slices, releasing its terminal lock between them.

// XTerm.Search.BufferSearch
public BufferScan BeginScan(string needle, SearchOptions options = default);
public void Apply(BufferScan scan);
public int Find(string needle, SearchOptions options = default);   // unchanged single-call path

// XTerm.Search.BufferScan (sealed; made only by BeginScan)
public BufferScanStatus Next(int maxRows);
public BufferScanStatus Status { get; }
public int Count { get; }
public string Needle { get; }
public SearchOptions Options { get; }

public enum BufferScanStatus { More, Done, Truncated, Invalidated }

// XTerm.Buffer.TerminalBuffer
public long TrimmedLines { get; }
public int ResizeCount { get; }

• One frame for every hit. The scan owns its hits and its resume position. Every Next first shifts both by the lines trimmed since its last call, then searches the next slice. The caller never holds a row number, so the two can't end up in different frames.
• The resume position sits just past a whole logical line, so a wrapped line whose head was trimmed to row 0 is searched exactly once.
• Next reports Invalidated on a switch between the normal and alternate screens, or a resize. The hits are discarded, and the caller begins again. Writing the status line does not invalidate a scan.
• Threading:
  • BeginScan can be called on any thread.
  • Next runs on any thread holding the terminal lock, one call at a time.
  • Apply runs only on the search's reader thread, after the last Next, and needs no lock: lines trimmed in transit are applied on the next read.
  • Apply throws unless the scan is Done or Truncated, or if it was begun on another search.

Tests

• Race test: a Barrier-lockstep test where the main thread holds a span while a writer thread trims. It fails on the old code and passes with the fix.
• Trims between slices: A_trim_between_slices_leaves_exactly_what_a_fresh_Find_finds uses trims of 1, 3, 7 and 25 lines. The same scenario, following the previous head's docs, gave rows that were off, an ArgumentException from the order check, or stale hits.
• Row 0: A_scan_resumed_on_a_wrapped_line_at_row_0_searches_it_once.
• Invalidation: screen switch and resize invalidate a scan; the status line doesn't.
• Late trims: lines trimmed after Done but before Apply are still applied.
• Refusals: Apply refuses an unfinished scan, or another search's scan.
• Results: 34 BufferSearchTests pass. The whole XTerm.NET.Tests project: 2351 passed, 1 skipped, 0 failed.
• Mutation check: five mutants, each caught by its intended test:
  • hits not shifted;
  • resume row not shifted;
  • resume one row early;
  • resize not checked;
  • HitsOnRow skipping the catch-up.

Counting trims adds one store per trim on the scroll path. It isn't benchmarked locally; the CI perf gate's scroll-ascii corpus measures it.

🤖 Generated with Claude Code

[tomlm]

Moves the hit-list rewrite off the thread that raises Trimmed: OnTrimmed now only bumps an Interlocked counter, and the reader thread rebases the hits the next time Count/CurrentIndex/HitsOnRow/the moves read them. The PR also splits Find into a static Scan (sliced, stateless) plus Apply(hits, truncated, trimMark).

The trim-counter design is sound for Find. The sliced Scan path has a coordinate problem, and that path is the reason the split exists.

High

• Sliced Scan mixes coordinate frames when a trim lands between slices. The host releases the terminal lock between slices, and with a full scrollback trims arrive continuously. Hits from earlier slices stay in pre-trim rows, later slices write post-trim rows, and no single trimMark passed to Apply can be right for both. The highlights land N rows off, or Apply throws on the row-order check. No test covers a trim between slices. (inline on TrimMark and SearchScan.NextRow)

Notes (checked, fine or low impact)

• CatchUpWithTrims on HitsOnRow is one volatile long read plus a compare. The method is small enough to inline, and Rebase is only called on a mismatch. That fits the hot-path guidance in CLAUDE.md.
• Rebase folding several trims into one shift is correct: rows are only ever decremented, so a row below zero after the first trim stays below zero. The _current - dropped adjustment is unchanged from main and still right, because dropped hits are always a prefix.
• Find reads the mark before scanning and publishes with it. Under the terminal lock no trim can land in between, so _rebasedTo is exact.
• ScanCore's row = runEnd skip gives the same result as the old loop, which reached continuation rows and skipped them on IsWrapped. NextRow = max(runEnd+1, end) handles a run that wraps past endRow. The stepped and fixed-boundary slice tests do exercise this.
• The race test fails against the old code. It asserts that the held span is unchanged and that the rebased result matches a fresh Find, so it checks the behavior itself rather than the decision to act.
• Low: the Scan remarks say rows outside the buffer are clamped. With a non-empty needle and startRow > Lines.Length, ScanCore returns NextRow = startRow unclamped. The empty-needle branch returns clamp(endRow) instead, so the two branches disagree on NextRow when startRow > endRow. A start < length loop still terminates, so there is no failure; it is only a doc mismatch.
• Low: Apply's trimMark > TrimMark check cannot catch a mark taken from a different BufferSearch instance. Each instance's counter starts at 0 when it is constructed.

[tomlm]

High: one trim mark cannot describe hits from slices scanned on either side of a trim.

ScanCore/AddRuns write absolute rows as they stand when that slice runs (into.Add(new SearchHit(row, ...))). These docs tell the host to release the lock between slices and to read the mark "when the last slice ends". SearchScan.NextRow tells it to shift only NextRow for trims. Nothing shifts the hits already in into. With a full scrollback and output flowing, which is when an off-thread find matters, trims land between slices routinely.

Trace (scrollback full):

• Slice 1 [0,50) finds hits at rows 10 and 40.
• Output trims 5 lines. The host shifts NextRow 50 -> 45, as SearchScan.NextRow says.
• Slice 2 [45,95) finds the text that was at row 51, now written as row 46.
• into = [10, 40, 46]. Rows 10 and 40 are pre-trim (correct now: 5 and 35); row 46 is post-trim.
• Mark read at the end -> Apply shifts nothing. The first two highlights draw 5 rows below their text.
• Mark read at the start instead -> Apply shifts all three by 5, and row 46 lands on 41, which is wrong the other way.
• With a 20-line trim, slice 2 starts at 30 and its first hit is row 31, after a 40 already in into. Apply throws ArgumentException from the row-order check and clears the results.

A second effect: if the shifted start clamps to 0, and row 0 is a continuation whose head was trimmed, ScanCore treats it as a run start (row > 0 &&). Text the previous slice already matched is matched again under a new id.

Fix: give the accumulated list a single frame. Two options:

• Have Scan take the mark the list is currently in and return the mark it scanned at, shifting/dropping into's existing hits (and returning a shifted NextRow) when they differ. Apply then gets one mark that is true for every hit.
• Expose a static Rebase(List<SearchHit>, long delta) and document that the caller must call it between slices.

Either way, add a test that trims between two slices and compares the result against a Find on the final buffer. The current slice tests never write between slices, so this branch is untested.

[JohnCampionJr]

Agreed, and reproduced. Against af02261, trims of 1, 3, 7 and 25 lines between slices gave rows that were off, an ArgumentException from the order check, and stale hits.

Fixed in 13be8d9 by taking the frame away from the caller:

• BeginScan returns a stateful BufferScan.
• Each Next(maxRows), under the terminal lock, first shifts its own accumulated hits and its resume row by the lines trimmed since its last call, then scans.
• Apply(scan) publishes with the scan's own buffer and mark.
• The resume row is always just past a whole logical line, so the clamped-to-0 double count can't happen. If the trim passes everything searched, row 0 is new ground and is treated as a run start, as Find does.

The static Scan, SearchScan, TrimMark and Apply(hits, ...) are removed. The trim count now lives on the buffer (TerminalBuffer.TrimmedLines), so BufferSearch doesn't subscribe to Trimmed at all.

Tests:

• A_trim_between_slices_leaves_exactly_what_a_fresh_Find_finds (1, 3, 7, 25 lines) and A_scan_resumed_on_a_wrapped_line_at_row_0_searches_it_once, both compared against a fresh Find.
• Mutants that shift only the hits, only the row, or resume one row early are each caught.

[tomlm]

High (same root cause as the comment on TrimMark): following this advice leaves the accumulated hits in the old frame.

Shifting NextRow alone moves the next slice into post-trim coordinates while every hit already in into stays pre-trim. The list then holds two frames, and Apply(..., trimMark) can shift it by only one amount. Example: trim 5 between slices -> earlier hits draw 5 rows low. Trim 20 -> slice 2's first row (31) sorts before an earlier hit (40), and Apply throws on the order check. Whatever this doc tells the caller to shift, it has to cover the hits in into as well as NextRow, or Scan should do the shift itself.

[JohnCampionJr]

Same root cause, same fix in 13be8d9: SearchScan and its NextRow advice are gone. The caller never holds a row any more. BufferScan keeps its position privately and shifts it together with its hits on every Next, so the two can't drift into different frames.

[JohnCampionJr]

Thanks. The High finding was right; fixed in 13be8d9 with a stateful BufferScan (details in the inline threads).

Both Low notes are moot now:

• The static Scan, whose NextRow clamping disagreed between branches, is removed.
• A mark from another instance can no longer reach Apply: it takes a BufferScan, and throws ArgumentException for one begun on a different search.

Also new:

• Next returns Invalidated on a screen switch or resize.
• The trim count lives on the buffer, so trims on the alternate screen are counted even for a search made on the normal one.
• Find and the scan search the screen buffer, so a program writing its status line doesn't disturb them.

XTerm.NET.Tests: 2351 passed, 1 skipped, 0 failed, and five mutants are each caught. One thing for the perf gate: counting trims adds one store per trim on the scroll path, which scroll-ascii will measure. I've updated the PR description to match.