multi_buffer: Use stable storage for buffer IDs (#46884)

Piotr Osiewicz and Smit Barmase created 1 week ago

Long story short:
1. Whenever we pick a buffer for mutation in

https://github.com/zed-industries/zed/blob/b8c5f672cdf8ed3c1f4c8016f2b8d6d84b7a7a9c/crates/multi_buffer/src/multi_buffer.rs#L3831,
we seed from a hashmap<BufferId, _>.
2. That HashMap is not RNG-seeded by
itself (good), but the BufferIds are: they are derived from entity ID in

https://github.com/zed-industries/zed/blob/b8c5f672cdf8ed3c1f4c8016f2b8d6d84b7a7a9c/crates/project/src/buffer_store.rs#L641

3.^ was introduced in https://github.com/zed-industries/zed/pull/10347
4.The HashMap is not randomized, but the order of it's values could
shift
when the keys (buffer ids) change.
5. Thus, we'll end up mutating a
different buffer in the multi-buffer when BufferIds shift around.

Our "fix" is that we're using a btreemap for buffer storage, which
should ensure that the order of iteration (and seeding) the candidates
matches up with the order of buffer creation.

Co-authored-by: Smit Barmase <[e-mail]>

Closes #ISSUE

Release Notes:

- N/A

Co-authored-by: Smit Barmase <[e-mail]>

Change summary

crates/multi_buffer/src/multi_buffer.rs | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

Detailed changes

crates/multi_buffer/src/multi_buffer.rs

@@ -70,7 +70,7 @@ pub struct MultiBuffer {
     /// Use [`MultiBuffer::snapshot`] to get a up-to-date snapshot.
     snapshot: RefCell<MultiBufferSnapshot>,
     /// Contains the state of the buffers being edited
-    buffers: HashMap<BufferId, BufferState>,
+    buffers: BTreeMap<BufferId, BufferState>,
     /// Mapping from path keys to their excerpts.
     excerpts_by_path: BTreeMap<PathKey, Vec<ExcerptId>>,
     /// Mapping from excerpt IDs to their path key.
@@ -1144,7 +1144,7 @@ impl MultiBuffer {
     }
 
     pub fn clone(&self, new_cx: &mut Context<Self>) -> Self {
-        let mut buffers = HashMap::default();
+        let mut buffers = BTreeMap::default();
         let buffer_changed_since_sync = Rc::new(Cell::new(false));
         for (buffer_id, buffer_state) in self.buffers.iter() {
             buffer_state.buffer.update(new_cx, |buffer, _| {
@@ -1884,7 +1884,7 @@ impl MultiBuffer {
     pub fn clear(&mut self, cx: &mut Context<Self>) {
         self.sync_mut(cx);
         let ids = self.excerpt_ids();
-        let removed_buffer_ids = self.buffers.drain().map(|(id, _)| id).collect();
+        let removed_buffer_ids = std::mem::take(&mut self.buffers).into_keys().collect();
         self.excerpts_by_path.clear();
         self.paths_by_excerpt.clear();
         let MultiBufferSnapshot {
@@ -2968,7 +2968,7 @@ impl MultiBuffer {
 
     fn sync_from_buffer_changes(
         snapshot: &mut MultiBufferSnapshot,
-        buffers: &HashMap<BufferId, BufferState>,
+        buffers: &BTreeMap<BufferId, BufferState>,
         diffs: &HashMap<BufferId, DiffState>,
         cx: &App,
     ) -> Vec<Edit<MultiBufferOffset>> {