From 9e02f103f3472055cb3293e64a43804f0e49195f Mon Sep 17 00:00:00 2001 From: tv Date: Mon, 9 Nov 2026 15:33:27 +0000 Subject: sync: a write between signing and making a message signed too Review 4, F10: a sync message signed the node's latest change under the document's write lock (`sign_sent`), then was made under its read lock: a write committed in between went in the message unsigned, and under a policy requiring signatures waited at the peer until the next signed change came with it, or a resend. Now the message is made under a read lock taken after signing, and if a write came in between (`AppState::sign_due` again under that lock), signed again: a few times at most, past them it goes as before, its signature with the next message. --- src/state/mod.rs | 18 ++++++++++++------ src/swarm/sync.rs | 16 +++++++++++++--- 2 files changed, 25 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/state/mod.rs b/src/state/mod.rs index 2fbfd31..601febc 100644 --- a/src/state/mod.rs +++ b/src/state/mod.rs @@ -734,12 +734,7 @@ impl AppState { if self.sign_every == SignEvery::Change { return; } - let due = { - let replica = doc.blocking_read(); - replica.signatures.pending() - && (self.sign_every == SignEvery::Sent - || crate::access::policy::of(&replica.doc).required()) - }; + let due = self.sign_due(&doc.blocking_read()); if due { let mut replica = doc.blocking_write(); let replica = &mut *replica; @@ -747,6 +742,17 @@ impl AppState { } } + /// Whether `replica`'s latest change of this node should be signed before a message + /// takes it (`sign_sent`): no signature vouches for it, and `--sign-every sent` or a + /// policy requiring signatures says so. + pub(crate) fn sign_due(&self, replica: &crate::document::replica::Replica) -> bool { + self.node_key.is_some() + && self.sign_every != SignEvery::Change + && replica.signatures.pending() + && (self.sign_every == SignEvery::Sent + || crate::access::policy::of(&replica.doc).required()) + } + pub(crate) fn publish_change(&self) { self.notify_store_changed(); if let Err(error) = self.sync_events.send(SyncEvent::DocumentChanged) { diff --git a/src/swarm/sync.rs b/src/swarm/sync.rs index dd3d804..e3f6055 100644 --- a/src/swarm/sync.rs +++ b/src/swarm/sync.rs @@ -171,12 +171,22 @@ async fn deliver_sync_to_peer(state: &AppState, peer: &str) -> Result<()> { // The session the message comes from, for its send's outcome. let (session, generated) = tokio::task::spawn_blocking( move || -> (u64, Result>) { - // This node's latest change, if this message may take it, leaves signed. - signer.sign_sent(&doc); // Label the message with the epoch of the document it was generated // from, read under the same lock. let (session, epoch, epoch_seq, body, writer, delivered, signatures) = { - let replica = doc.blocking_read(); + // This node's latest change, if this message may take it, leaves + // signed: signed, then looked at again under the lock the message is + // made under, signed again if a write came in between (a few times + // at most; past them it goes as it is, its signature with the next). + let mut tries = 0; + let replica = loop { + signer.sign_sent(&doc); + let replica = doc.blocking_read(); + tries += 1; + if tries >= 3 || !signer.sign_due(&replica) { + break replica; + } + }; let mut peer_state = psm.blocking_lock(); let (epoch, epoch_seq) = (replica.epoch, replica.epoch_seq); // Whose change it passes on, if any: this message carries it. -- cgit v1.3.1