Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(iroh): do not double-close docs on drop #2383

Merged
merged 2 commits into from
Jun 20, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 33 additions & 6 deletions iroh/src/client/docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,11 @@ where
fn drop(&mut self) {
let doc_id = self.id;
let rpc = self.rpc.clone();
self.rt.spawn(async move {
rpc.rpc(DocCloseRequest { doc_id }).await.ok();
});
if !self.closed.swap(true, Ordering::Relaxed) {
self.rt.spawn(async move {
rpc.rpc(DocCloseRequest { doc_id }).await.ok();
});
}
}
}

Expand Down Expand Up @@ -176,13 +178,14 @@ where

/// Close the document.
pub async fn close(&self) -> Result<()> {
self.0.closed.store(true, Ordering::Release);
self.rpc(DocCloseRequest { doc_id: self.id() }).await??;
if !self.0.closed.swap(true, Ordering::Relaxed) {
self.rpc(DocCloseRequest { doc_id: self.id() }).await??;
}
Ok(())
}

fn ensure_open(&self) -> Result<()> {
if self.0.closed.load(Ordering::Acquire) {
if self.0.closed.load(Ordering::Relaxed) {
Err(anyhow!("document is closed"))
} else {
Ok(())
Expand Down Expand Up @@ -783,6 +786,30 @@ mod tests {
Ok(())
}

/// Test that closing a doc does not close other instances.
#[tokio::test]
async fn test_doc_close() -> Result<()> {
let _guard = iroh_test::logging::setup();

let node = crate::node::Node::memory().spawn().await?;
let author = node.authors().default().await?;
// open doc two times
let doc1 = node.docs().create().await?;
let doc2 = node.docs().open(doc1.id()).await?.expect("doc to exist");
// close doc1 instance
doc1.close().await?;
// operations on doc1 now fail.
assert!(doc1.set_bytes(author, "foo", "bar").await.is_err());
// dropping doc1 will close the doc if not already closed
// wait a bit because the close-on-drop spawns a task for which we cannot track completion.
drop(doc1);
tokio::time::sleep(tokio::time::Duration::from_millis(100)).await;

// operations on doc2 still succeed
doc2.set_bytes(author, "foo", "bar").await?;
Ok(())
}

#[tokio::test]
async fn test_doc_import_export() -> Result<()> {
let _guard = iroh_test::logging::setup();
Expand Down
Loading