Fixed a bug where buffer saved clocks would desynchronize in rare execution paths

Mikayla Maki and Max created 2 years ago

co-authored-by: Max <[e-mail]>

Change summary

crates/clock/src/clock.rs                               |  1 
crates/collab/src/tests/randomized_integration_tests.rs | 11 +++--
crates/project/src/project.rs                           | 21 +++++-----
crates/project/src/worktree.rs                          |  8 ++--
4 files changed, 21 insertions(+), 20 deletions(-)

Detailed changes

crates/clock/src/clock.rs

@@ -66,6 +66,7 @@ impl<'a> AddAssign<&'a Local> for Local {
     }
 }
 
+/// A vector clock
 #[derive(Clone, Default, Hash, Eq, PartialEq)]
 pub struct Global(SmallVec<[u32; 8]>);
 

crates/collab/src/tests/randomized_integration_tests.rs

@@ -628,12 +628,13 @@ async fn apply_client_operation(
 
             ensure_project_shared(&project, client, cx).await;
             let requested_version = buffer.read_with(cx, |buffer, _| buffer.version());
-            let save = project.update(cx, |project, cx| project.save_buffer(buffer, cx));
-            let save = cx.background().spawn(async move {
-                let (saved_version, _, _) = save
-                    .await
+            let save = project.update(cx, |project, cx| project.save_buffer(buffer.clone(), cx));
+            let save = cx.spawn(|cx| async move {
+                save.await
                     .map_err(|err| anyhow!("save request failed: {:?}", err))?;
-                assert!(saved_version.observed_all(&requested_version));
+                assert!(buffer
+                    .read_with(&cx, |buffer, _| { buffer.saved_version().to_owned() })
+                    .observed_all(&requested_version));
                 anyhow::Ok(())
             });
             if detach {

crates/project/src/project.rs

@@ -37,8 +37,8 @@ use language::{
     range_from_lsp, range_to_lsp, Anchor, Bias, Buffer, CachedLspAdapter, CodeAction, CodeLabel,
     Completion, Diagnostic, DiagnosticEntry, DiagnosticSet, Diff, Event as BufferEvent, File as _,
     Language, LanguageRegistry, LanguageServerName, LocalFile, OffsetRangeExt, Operation, Patch,
-    PendingLanguageServer, PointUtf16, RopeFingerprint, TextBufferSnapshot, ToOffset, ToPointUtf16,
-    Transaction, Unclipped,
+    PendingLanguageServer, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction,
+    Unclipped,
 };
 use log::error;
 use lsp::{
@@ -69,7 +69,7 @@ use std::{
         atomic::{AtomicUsize, Ordering::SeqCst},
         Arc,
     },
-    time::{Duration, Instant, SystemTime},
+    time::{Duration, Instant},
 };
 use terminals::Terminals;
 use util::{
@@ -1617,7 +1617,7 @@ impl Project {
         &self,
         buffer: ModelHandle<Buffer>,
         cx: &mut ModelContext<Self>,
-    ) -> Task<Result<(clock::Global, RopeFingerprint, SystemTime)>> {
+    ) -> Task<Result<()>> {
         let Some(file) = File::from_dyn(buffer.read(cx).file()) else {
             return Task::ready(Err(anyhow!("buffer doesn't have a file")));
         };
@@ -5985,16 +5985,15 @@ impl Project {
             .await?;
         let buffer_id = buffer.read_with(&cx, |buffer, _| buffer.remote_id());
 
-        let (saved_version, fingerprint, mtime) = this
-            .update(&mut cx, |this, cx| this.save_buffer(buffer, cx))
+        this.update(&mut cx, |this, cx| this.save_buffer(buffer.clone(), cx))
             .await?;
-        Ok(proto::BufferSaved {
+        Ok(buffer.read_with(&cx, |buffer, _| proto::BufferSaved {
             project_id,
             buffer_id,
-            version: serialize_version(&saved_version),
-            mtime: Some(mtime.into()),
-            fingerprint: language::proto::serialize_fingerprint(fingerprint),
-        })
+            version: serialize_version(buffer.saved_version()),
+            mtime: Some(buffer.saved_mtime().into()),
+            fingerprint: language::proto::serialize_fingerprint(buffer.saved_version_fingerprint()),
+        }))
     }
 
     async fn handle_reload_buffers(

crates/project/src/worktree.rs

@@ -923,7 +923,7 @@ impl LocalWorktree {
         path: Arc<Path>,
         has_changed_file: bool,
         cx: &mut ModelContext<Worktree>,
-    ) -> Task<Result<(clock::Global, RopeFingerprint, SystemTime)>> {
+    ) -> Task<Result<()>> {
         let handle = cx.handle();
         let buffer = buffer_handle.read(cx);
 
@@ -979,7 +979,7 @@ impl LocalWorktree {
                 buffer.did_save(version.clone(), fingerprint, entry.mtime, cx);
             });
 
-            Ok((version, fingerprint, entry.mtime))
+            Ok(())
         })
     }
 
@@ -1290,7 +1290,7 @@ impl RemoteWorktree {
         &self,
         buffer_handle: ModelHandle<Buffer>,
         cx: &mut ModelContext<Worktree>,
-    ) -> Task<Result<(clock::Global, RopeFingerprint, SystemTime)>> {
+    ) -> Task<Result<()>> {
         let buffer = buffer_handle.read(cx);
         let buffer_id = buffer.remote_id();
         let version = buffer.version();
@@ -1315,7 +1315,7 @@ impl RemoteWorktree {
                 buffer.did_save(version.clone(), fingerprint, mtime, cx);
             });
 
-            Ok((version, fingerprint, mtime))
+            Ok(())
         })
     }