Fix opening a buffer after leaving and joining the same project

Antonio Scandurra created 3 years ago

This bug existed prior to #1700 and was caused by not clearing the
buffers that were already shared with a peer that left and opened
a project using the same connection. When such peer would re-join
the project and open a buffer that it had opened previously, the
host assumed the peer had already seen that buffer and wouldn't bother
sending it again.

Change summary

crates/collab/src/integration_tests.rs | 52 +++++++++++++++++++++++++--
crates/collab/src/rpc/store.rs         |  2 
crates/project/src/project.rs          |  1 
3 files changed, 49 insertions(+), 6 deletions(-)

Detailed changes

crates/collab/src/integration_tests.rs

@@ -2017,7 +2017,7 @@ async fn test_leaving_project(
         .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx))
         .await
         .unwrap();
-    let project_b = client_b.build_remote_project(project_id, cx_b).await;
+    let project_b1 = client_b.build_remote_project(project_id, cx_b).await;
     let project_c = client_c.build_remote_project(project_id, cx_c).await;
 
     // Client A sees that a guest has joined.
@@ -2025,20 +2025,62 @@ async fn test_leaving_project(
     project_a.read_with(cx_a, |project, _| {
         assert_eq!(project.collaborators().len(), 2);
     });
-    project_b.read_with(cx_b, |project, _| {
+    project_b1.read_with(cx_b, |project, _| {
+        assert_eq!(project.collaborators().len(), 2);
+    });
+    project_c.read_with(cx_c, |project, _| {
+        assert_eq!(project.collaborators().len(), 2);
+    });
+
+    // Client B opens a buffer.
+    let buffer_b1 = project_b1
+        .update(cx_b, |project, cx| {
+            let worktree_id = project.worktrees(cx).next().unwrap().read(cx).id();
+            project.open_buffer((worktree_id, "a.txt"), cx)
+        })
+        .await
+        .unwrap();
+    buffer_b1.read_with(cx_b, |buffer, _| assert_eq!(buffer.text(), "a-contents"));
+
+    // Drop client B's project and ensure client A and client C observe client B leaving.
+    cx_b.update(|_| drop(project_b1));
+    deterministic.run_until_parked();
+    project_a.read_with(cx_a, |project, _| {
+        assert_eq!(project.collaborators().len(), 1);
+    });
+    project_c.read_with(cx_c, |project, _| {
+        assert_eq!(project.collaborators().len(), 1);
+    });
+
+    // Client B re-joins the project and can open buffers as before.
+    let project_b2 = client_b.build_remote_project(project_id, cx_b).await;
+    deterministic.run_until_parked();
+    project_a.read_with(cx_a, |project, _| {
+        assert_eq!(project.collaborators().len(), 2);
+    });
+    project_b2.read_with(cx_b, |project, _| {
         assert_eq!(project.collaborators().len(), 2);
     });
     project_c.read_with(cx_c, |project, _| {
         assert_eq!(project.collaborators().len(), 2);
     });
 
-    // Drop client B's connection and ensure client A and client C observe client B leaving the project.
+    let buffer_b2 = project_b2
+        .update(cx_b, |project, cx| {
+            let worktree_id = project.worktrees(cx).next().unwrap().read(cx).id();
+            project.open_buffer((worktree_id, "a.txt"), cx)
+        })
+        .await
+        .unwrap();
+    buffer_b2.read_with(cx_b, |buffer, _| assert_eq!(buffer.text(), "a-contents"));
+
+    // Drop client B's connection and ensure client A and client C observe client B leaving.
     client_b.disconnect(&cx_b.to_async()).unwrap();
     deterministic.run_until_parked();
     project_a.read_with(cx_a, |project, _| {
         assert_eq!(project.collaborators().len(), 1);
     });
-    project_b.read_with(cx_b, |project, _| {
+    project_b2.read_with(cx_b, |project, _| {
         assert!(project.is_read_only());
     });
     project_c.read_with(cx_c, |project, _| {
@@ -2068,7 +2110,7 @@ async fn test_leaving_project(
     project_a.read_with(cx_a, |project, _| {
         assert_eq!(project.collaborators().len(), 0);
     });
-    project_b.read_with(cx_b, |project, _| {
+    project_b2.read_with(cx_b, |project, _| {
         assert!(project.is_read_only());
     });
     project_c.read_with(cx_c, |project, _| {

crates/collab/src/rpc/store.rs

@@ -1205,7 +1205,7 @@ impl Store {
                 let guest_connection = self.connections.get(guest_connection_id).unwrap();
                 assert!(guest_connection.projects.contains(project_id));
             }
-            assert_eq!(project.active_replica_ids.len(), project.guests.len(),);
+            assert_eq!(project.active_replica_ids.len(), project.guests.len());
             assert_eq!(
                 project.active_replica_ids,
                 project

crates/project/src/project.rs

@@ -4561,6 +4561,7 @@ impl Project {
                     buffer.update(cx, |buffer, cx| buffer.remove_peer(replica_id, cx));
                 }
             }
+            this.shared_buffers.remove(&peer_id);
 
             cx.emit(Event::CollaboratorLeft(peer_id));
             cx.notify();