From 0f84a366d94ed53af05517938d34b199cb650b5a Mon Sep 17 00:00:00 2001 From: Eric Holk Date: Sat, 14 Feb 2026 00:19:35 -0800 Subject: [PATCH] multi_buffer: Fix "cannot seek backward" crash in summaries_for_anchors (#49047) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Bug **Sentry:** [ZED-3W3](https://sentry.io/organizations/zed-dev/issues/7085300207/) (13 events, first seen 2025-12-03) `summaries_for_anchors` panics with "cannot seek backward" when iterating selection anchors whose excerpt locators are no longer in ascending order after an `update_path_excerpts` call. ### Root cause `update_path_excerpts` merges new excerpt ranges with existing ones. When an existing excerpt has no overlap with any new range (e.g. `existing_range.end < new.context.start`), it is removed from the excerpts tree but is **not** recorded in `replaced_excerpts`. Its stale locator persists in the `excerpt_ids` tree. Later, when a new excerpt is inserted for a different path whose locator falls between the stale locator and another surviving excerpt, `summaries_for_anchors` can encounter anchors in an order where it needs to seek the cursor *backward* — violating the forward-only cursor invariant. Concretely, the scenario requires: 1. A path with ≥3 excerpts (E_B1, E_B2, E_B3) with anchors in E_B2 and E_B3. 2. An `update_path_excerpts` call that keeps E_B1, **removes E_B2** (no overlap → no `replaced_excerpts` entry), and replaces E_B3 with a new excerpt N. 3. A new path D inserted between paths B and C, whose excerpt E_D gets a locator between N and E_B2's stale locator. 4. Resolving the old anchors: E_B2's stale locator seeks past E_D, then E_B3→N tries to seek backward to N — **panic**. ## Fix In the two branches of `update_path_excerpts` where existing excerpts are removed without being absorbed into a new range, map the removed excerpt to the nearest surviving neighbor (the last kept or inserted excerpt) in `replaced_excerpts`. This ensures stale anchors always remap to a valid, correctly-ordered locator. The two branches are: - `existing_range.end < new.context.start` — existing excerpt falls entirely before the next new range - `(None, Some(...))` — no more new ranges for remaining existing excerpts ## Verification - A reproduction test (`test_cannot_seek_backward_after_excerpt_replacement`) that constructs the exact crash scenario now passes. - Full `multi_buffer` test suite (48 tests) passes with no regressions. - Clippy is clean. Release Notes: - Fixed a possible crash when updating excerpts in multibuffers --- crates/multi_buffer/src/multi_buffer_tests.rs | 83 +++++++++++++++++++ crates/multi_buffer/src/path_key.rs | 19 +++++ 2 files changed, 102 insertions(+) diff --git a/crates/multi_buffer/src/multi_buffer_tests.rs b/crates/multi_buffer/src/multi_buffer_tests.rs index 8792f95e39c966fbc0af08db33cd692ed6fe5c37..455bef14ec8c07532be2c91da4e4927ef9be2ebd 100644 --- a/crates/multi_buffer/src/multi_buffer_tests.rs +++ b/crates/multi_buffer/src/multi_buffer_tests.rs @@ -5041,3 +5041,86 @@ fn test_range_to_buffer_ranges_with_range_bounds(cx: &mut App) { assert_eq!(ranges_unbounded_trailing[0].2, te_excerpt_1_id); assert_eq!(ranges_unbounded_trailing[1].2, te_excerpt_2_id); } + +#[gpui::test] +fn test_cannot_seek_backward_after_excerpt_replacement(cx: &mut TestAppContext) { + let buffer_b_text: String = (0..50).map(|i| format!("line_b {i}\n")).collect(); + let buffer_b = cx.new(|cx| Buffer::local(buffer_b_text, cx)); + + let buffer_c_text: String = (0..10).map(|i| format!("line_c {i}\n")).collect(); + let buffer_c = cx.new(|cx| Buffer::local(buffer_c_text, cx)); + + let buffer_d_text: String = (0..10).map(|i| format!("line_d {i}\n")).collect(); + let buffer_d = cx.new(|cx| Buffer::local(buffer_d_text, cx)); + + let path_b = PathKey::with_sort_prefix(0, rel_path("bbb.rs").into_arc()); + let path_c = PathKey::with_sort_prefix(0, rel_path("ddd.rs").into_arc()); + let path_d = PathKey::with_sort_prefix(0, rel_path("ccc.rs").into_arc()); + + let multibuffer = cx.new(|_| MultiBuffer::new(Capability::ReadWrite)); + + multibuffer.update(cx, |multibuffer, cx| { + multibuffer.set_excerpts_for_path( + path_b.clone(), + buffer_b.clone(), + vec![ + Point::row_range(0..3), + Point::row_range(15..18), + Point::row_range(30..33), + ], + 0, + cx, + ); + }); + + multibuffer.update(cx, |multibuffer, cx| { + multibuffer.set_excerpts_for_path( + path_c.clone(), + buffer_c.clone(), + vec![Point::row_range(0..3)], + 0, + cx, + ); + }); + + let (anchor_in_e_b2, anchor_in_e_b3) = multibuffer.read_with(cx, |multibuffer, cx| { + let snapshot = multibuffer.snapshot(cx); + let excerpt_ids: Vec = snapshot.excerpts().map(|(id, _, _)| id).collect(); + assert_eq!(excerpt_ids.len(), 4, "expected 4 excerpts (3×B + 1×C)"); + + let e_b2_id = excerpt_ids[1]; + let e_b3_id = excerpt_ids[2]; + + let e_b2 = snapshot.excerpt(e_b2_id).expect("E_B2 should exist"); + let e_b3 = snapshot.excerpt(e_b3_id).expect("E_B3 should exist"); + + let anchor_b2 = Anchor::in_buffer(e_b2_id, e_b2.range.context.start); + let anchor_b3 = Anchor::in_buffer(e_b3_id, e_b3.range.context.start); + (anchor_b2, anchor_b3) + }); + + multibuffer.update(cx, |multibuffer, cx| { + multibuffer.set_excerpts_for_path( + path_b.clone(), + buffer_b.clone(), + vec![Point::row_range(0..3), Point::row_range(28..36)], + 0, + cx, + ); + }); + + multibuffer.update(cx, |multibuffer, cx| { + multibuffer.set_excerpts_for_path( + path_d.clone(), + buffer_d.clone(), + vec![Point::row_range(0..3)], + 0, + cx, + ); + }); + + multibuffer.read_with(cx, |multibuffer, cx| { + let snapshot = multibuffer.snapshot(cx); + snapshot.summaries_for_anchors::(&[anchor_in_e_b2, anchor_in_e_b3]); + }); +} diff --git a/crates/multi_buffer/src/path_key.rs b/crates/multi_buffer/src/path_key.rs index 8c20f211f61990b3775d231dc37a80352d7b9b98..dbee196e7ea32ff294e13666f3d26499dbf1ab5d 100644 --- a/crates/multi_buffer/src/path_key.rs +++ b/crates/multi_buffer/src/path_key.rs @@ -375,6 +375,15 @@ impl MultiBuffer { (None, Some((existing_id, _))) => { existing_iter.next(); to_remove.push(existing_id); + // Map removed excerpt to nearest surviving neighbor so stale + // anchors remap to a valid locator and maintain ordering. + let replacement = to_insert.last().map(|(id, _)| *id).unwrap_or(insert_after); + if replacement != ExcerptId::min() { + self.snapshot + .get_mut() + .replaced_excerpts + .insert(existing_id, replacement); + } continue; } (Some(_), None) => { @@ -388,6 +397,16 @@ impl MultiBuffer { if existing_range.end < new.context.start { let existing_id = existing_iter.next().unwrap(); to_remove.push(existing_id); + // Map removed excerpt to nearest surviving neighbor so stale + // anchors remap to a valid locator and maintain ordering. + let replacement = + to_insert.last().map(|(id, _)| *id).unwrap_or(insert_after); + if replacement != ExcerptId::min() { + self.snapshot + .get_mut() + .replaced_excerpts + .insert(existing_id, replacement); + } continue; } else if existing_range.start > new.context.end { let new_id = next_excerpt_id();