From 9adbab5d99f81025eda5469ed70cab0dd577eaee Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 12 Oct 2022 10:28:17 +0200 Subject: [PATCH] Fix opening a buffer after leaving and joining the same project 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. --- 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(-) diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index f6f0f5c7f2..0809a7c61e 100644 --- a/crates/collab/src/integration_tests.rs +++ b/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); }); - // Drop client B's connection and ensure client A and client C observe client B leaving the project. + // 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); + }); + + 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, _| { diff --git a/crates/collab/src/rpc/store.rs b/crates/collab/src/rpc/store.rs index cc34094782..b7dd39cff1 100644 --- a/crates/collab/src/rpc/store.rs +++ b/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 diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 11792bcf1e..6db9ce8aca 100644 --- a/crates/project/src/project.rs +++ b/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();