From 85a13fa4772ee6f72db29098120f2ae12a48baa8 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 5 Jan 2022 11:28:49 -0800 Subject: [PATCH] Fix panic when resolving anchors after an excerpt id has been recycled Co-Authored-By: Nathan Sobo --- crates/editor/src/multi_buffer.rs | 57 +++++++++++++++++++++++- crates/editor/src/multi_buffer/anchor.rs | 5 +++ crates/text/src/text.rs | 4 +- 3 files changed, 62 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 7e718552366b03fb4979fc7a363176cba99078c3..4d39b921401a24432e126ca8f7d0f7e3b3959d3a 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -1291,7 +1291,7 @@ impl MultiBufferSnapshot { let mut position = D::from_text_summary(&cursor.start().text); if let Some(excerpt) = cursor.item() { - if excerpt.id == anchor.excerpt_id { + if excerpt.id == anchor.excerpt_id && excerpt.buffer_id == anchor.buffer_id { let excerpt_buffer_start = excerpt.range.start.summary::(&excerpt.buffer); let buffer_position = anchor.text_anchor.summary::(&excerpt.buffer); if buffer_position > excerpt_buffer_start { @@ -1312,6 +1312,7 @@ impl MultiBufferSnapshot { let mut summaries = Vec::new(); while let Some(anchor) = anchors.peek() { let excerpt_id = &anchor.excerpt_id; + let buffer_id = anchor.buffer_id; let excerpt_anchors = iter::from_fn(|| { let anchor = anchors.peek()?; if anchor.excerpt_id == *excerpt_id { @@ -1328,7 +1329,7 @@ impl MultiBufferSnapshot { let position = D::from_text_summary(&cursor.start().text); if let Some(excerpt) = cursor.item() { - if excerpt.id == *excerpt_id { + if excerpt.id == *excerpt_id && excerpt.buffer_id == buffer_id { let excerpt_buffer_start = excerpt.range.start.summary::(&excerpt.buffer); summaries.extend( excerpt @@ -1379,6 +1380,7 @@ impl MultiBufferSnapshot { let text_anchor = excerpt.clip_anchor(excerpt.buffer.anchor_at(buffer_start + overshoot, bias)); Anchor { + buffer_id: excerpt.buffer_id, excerpt_id: excerpt.id.clone(), text_anchor, } @@ -1397,6 +1399,7 @@ impl MultiBufferSnapshot { let text_anchor = excerpt.clip_anchor(text_anchor); drop(cursor); return Anchor { + buffer_id: excerpt.buffer_id, excerpt_id, text_anchor, }; @@ -1595,10 +1598,12 @@ impl MultiBufferSnapshot { .flat_map(move |(replica_id, selections)| { selections.map(move |selection| { let mut start = Anchor { + buffer_id: excerpt.buffer_id, excerpt_id: excerpt.id.clone(), text_anchor: selection.start.clone(), }; let mut end = Anchor { + buffer_id: excerpt.buffer_id, excerpt_id: excerpt.id.clone(), text_anchor: selection.end.clone(), }; @@ -2349,6 +2354,54 @@ mod tests { assert_eq!(old_snapshot.anchor_after(10).to_offset(&new_snapshot), 14); } + #[gpui::test] + fn test_multibuffer_anchors_after_replacing_excerpts(cx: &mut MutableAppContext) { + let buffer_1 = cx.add_model(|cx| Buffer::new(0, "abcd", cx)); + let buffer_2 = cx.add_model(|cx| Buffer::new(0, "efghi", cx)); + + // Create an insertion id in buffer 1 that doesn't exist in buffer 2 + buffer_1.update(cx, |buffer, cx| { + buffer.edit([4..4], "123", cx); + }); + + let multibuffer = cx.add_model(|_| MultiBuffer::new(0)); + let excerpt_id = multibuffer.update(cx, |multibuffer, cx| { + multibuffer.push_excerpt( + ExcerptProperties { + buffer: &buffer_1, + range: 0..7, + }, + cx, + ) + }); + + // Create an anchor in the second insertion of buffer 1 + let anchor = multibuffer.read(cx).read(cx).anchor_before(7); + + multibuffer.update(cx, |multibuffer, cx| { + multibuffer.remove_excerpts([&excerpt_id], cx); + let new_excerpt_id = multibuffer.push_excerpt( + ExcerptProperties { + buffer: &buffer_2, + range: 0..5, + }, + cx, + ); + + // The same id is reused for an excerpt in a different buffer. + assert_eq!(new_excerpt_id, excerpt_id); + + // We don't attempt to resolve the text anchor from buffer 1 + // in buffer 2. + let snapshot = multibuffer.snapshot(cx); + assert_eq!(snapshot.summary_for_anchor::(&anchor), 0); + assert_eq!( + snapshot.summaries_for_anchors::(&[anchor]), + vec![0] + ); + }); + } + #[gpui::test(iterations = 100)] fn test_random_excerpts(cx: &mut MutableAppContext, mut rng: StdRng) { let operations = env::var("OPERATIONS") diff --git a/crates/editor/src/multi_buffer/anchor.rs b/crates/editor/src/multi_buffer/anchor.rs index 758a62526bf1be8825dc08083f668fc3308e45f4..2e1e1a924634f8b1b734b539a16805fbb888fd92 100644 --- a/crates/editor/src/multi_buffer/anchor.rs +++ b/crates/editor/src/multi_buffer/anchor.rs @@ -9,6 +9,7 @@ use text::{rope::TextDimension, Point}; #[derive(Clone, Eq, PartialEq, Debug, Hash)] pub struct Anchor { + pub(crate) buffer_id: usize, pub(crate) excerpt_id: ExcerptId, pub(crate) text_anchor: text::Anchor, } @@ -16,6 +17,7 @@ pub struct Anchor { impl Anchor { pub fn min() -> Self { Self { + buffer_id: 0, excerpt_id: ExcerptId::min(), text_anchor: text::Anchor::min(), } @@ -23,6 +25,7 @@ impl Anchor { pub fn max() -> Self { Self { + buffer_id: 0, excerpt_id: ExcerptId::max(), text_anchor: text::Anchor::max(), } @@ -54,6 +57,7 @@ impl Anchor { if self.text_anchor.bias != Bias::Left { if let Some(buffer_snapshot) = snapshot.buffer_snapshot_for_excerpt(&self.excerpt_id) { return Self { + buffer_id: self.buffer_id, excerpt_id: self.excerpt_id.clone(), text_anchor: self.text_anchor.bias_left(buffer_snapshot), }; @@ -66,6 +70,7 @@ impl Anchor { if self.text_anchor.bias != Bias::Right { if let Some(buffer_snapshot) = snapshot.buffer_snapshot_for_excerpt(&self.excerpt_id) { return Self { + buffer_id: self.buffer_id, excerpt_id: self.excerpt_id.clone(), text_anchor: self.text_anchor.bias_right(buffer_snapshot), }; diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index 9db7591f22a5dcdf4a7bbfaee6883a29767bc98e..402cbfecfac34382d28b8fd9e2b01a11d46203ea 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -1495,7 +1495,7 @@ impl BufferSnapshot { insertion_cursor.prev(&()); } let insertion = insertion_cursor.item().expect("invalid insertion"); - debug_assert_eq!(insertion.timestamp, anchor.timestamp, "invalid insertion"); + assert_eq!(insertion.timestamp, anchor.timestamp, "invalid insertion"); fragment_cursor.seek_forward(&Some(&insertion.fragment_id), Bias::Left, &None); let fragment = fragment_cursor.item().unwrap(); @@ -1537,7 +1537,7 @@ impl BufferSnapshot { insertion_cursor.prev(&()); } let insertion = insertion_cursor.item().expect("invalid insertion"); - debug_assert_eq!(insertion.timestamp, anchor.timestamp, "invalid insertion"); + assert_eq!(insertion.timestamp, anchor.timestamp, "invalid insertion"); let mut fragment_cursor = self.fragments.cursor::<(Option<&Locator>, usize)>(); fragment_cursor.seek(&Some(&insertion.fragment_id), Bias::Left, &None);