From dce0bfd1e3fd230fbeab54ee4655bdd77b52b9ff Mon Sep 17 00:00:00 2001
From: Alex Good <alex@memoryandthought.me>
Date: Thu, 28 Nov 2024 10:00:33 +0000
Subject: [PATCH] fix: don't hang when sharepolicies return immediately

Problem: when using a SharePolicy (rather than doing some asynchronous
work) it was possible for the repo to hang. The symptom of this would be
that a repo would never respond to sync messages for a document when a
SharePolicy returned a positive response for the document.

Why was this happening? When the repo first receives a sync message for
a (document, peer) combination it checks whether the SharePolicy allows
synchronization with the peer in question. To do this the repo creates a
new future (correspoinding to SharePolicy::should_sync) and then puts it
in a list of share decisions to poll. The share decisions are then
polled on the next run through the event loop. If the repo is in a very
quiet loop then no new event occurs to trigger the first poll of the
future.

Solution: at the end of the repo event loop check if there are any share
decisions to poll. If there are, then run the loop again.
---
 src/repo.rs                                  |  3 ++
 tests/network/document_create_then_change.rs | 52 ++++++++++++++++++++
 tests/network/main.rs                        |  1 +
 3 files changed, 56 insertions(+)
 create mode 100644 tests/network/document_create_then_change.rs

diff --git a/src/repo.rs b/src/repo.rs
index 50cfec4..16aff3a 100644
--- a/src/repo.rs
+++ b/src/repo.rs
@@ -1885,6 +1885,9 @@ impl Repo {
                 self.remove_unused_sync_states();
                 self.remove_unused_pending_messages();
                 self.gc_docs();
+                if !self.pending_events.is_empty() || !self.share_decisions_to_poll.is_empty() {
+                    continue;
+                }
                 select! {
                     recv(self.repo_receiver) -> repo_event => {
                         if let Ok(event) = repo_event {
diff --git a/tests/network/document_create_then_change.rs b/tests/network/document_create_then_change.rs
new file mode 100644
index 0000000..4cc3819
--- /dev/null
+++ b/tests/network/document_create_then_change.rs
@@ -0,0 +1,52 @@
+extern crate test_utils;
+
+use std::time::Duration;
+
+use automerge::transaction::Transactable;
+use automerge_repo::Repo;
+use test_log::test;
+use test_utils::storage_utils::{InMemoryStorage, SimpleStorage};
+
+use super::tincans::connect_repos;
+
+#[test(tokio::test)]
+async fn test_create_document_then_change_is_synced() {
+    let repo2_storage = InMemoryStorage::default();
+    // Create two repos.
+    let repo_1 = Repo::new(Some("repo1".to_string()), Box::new(SimpleStorage));
+    let repo_2 = Repo::new(Some("repo2".to_string()), Box::new(repo2_storage.clone()));
+
+    // Run the repos in the background.
+    let repo_handle_1 = repo_1.run();
+    let repo_handle_2 = repo_2.run();
+    connect_repos(&repo_handle_1, &repo_handle_2);
+
+    // Spawn a task that creates a document and makes a change to id
+    let doc_handle = tokio::spawn({
+        let repo_handle_1 = repo_handle_1.clone();
+        async move {
+            let document_handle_1 = repo_handle_1.new_document();
+
+            let repo_id = repo_handle_1.get_repo_id().clone();
+            let document_handle_1 = document_handle_1.clone();
+            // Edit the document.
+            document_handle_1.with_doc_mut(|doc| {
+                let mut tx = doc.transaction();
+                tx.put(automerge::ROOT, "repo_id", format!("{}", repo_id))
+                    .expect("Failed to change the document.");
+                tx.commit();
+            });
+            document_handle_1
+        }
+    })
+    .await
+    .unwrap();
+
+    tokio::time::sleep(Duration::from_millis(1000)).await;
+
+    // Now stop both repos
+    repo_handle_1.stop().unwrap();
+    repo_handle_2.stop().unwrap();
+
+    assert!(repo2_storage.contains_document(doc_handle.document_id()));
+}
diff --git a/tests/network/main.rs b/tests/network/main.rs
index dee69ca..9189c77 100644
--- a/tests/network/main.rs
+++ b/tests/network/main.rs
@@ -9,6 +9,7 @@ use test_utils::storage_utils::SimpleStorage;
 mod tincans;
 use tincans::{tincan_to_nowhere, tincans, TinCan, TinCans};
 mod document_changed;
+mod document_create_then_change;
 mod document_list;
 mod document_load;
 mod document_request;