From 4d03cf849b395cdccdce9ca6514ac40c5c05418f Mon Sep 17 00:00:00 2001 From: KeelrLeek <156356974+KeelrLeek@users.noreply.github.com> Date: Thu, 3 Sep 2026 11:51:35 -0500 Subject: [PATCH] community: every ledger takes its write lock, so the inbox check can no longer land on top of a friend accept --- src-tauri/src/friends.rs | 31 +++++++++++++++++++++++-------- src-tauri/src/inbox.rs | 8 ++++---- src-tauri/src/ledger.rs | 12 ++++++++++++ src-tauri/src/proposals.rs | 10 +++++++++- src-tauri/src/shared_playlists.rs | 30 ++++++++++++++++++++---------- src-tauri/src/spaces.rs | 27 +++++++++++++++++++++------ 6 files changed, 89 insertions(+), 29 deletions(-) diff --git a/src-tauri/src/friends.rs b/src-tauri/src/friends.rs index 6a31fbe..665ca39 100644 --- a/src-tauri/src/friends.rs +++ b/src-tauri/src/friends.rs @@ -30,6 +30,7 @@ use serde::{Deserialize, Serialize}; use tauri::{Emitter, Manager}; use crate::inbox::{self, Cargo}; +use crate::ledger; use crate::persona; use crate::slsk_link; use crate::state::AppState; @@ -114,8 +115,11 @@ fn path(app: &tauri::AppHandle) -> Result { .app_data_dir() .map_err(|e| e.to_string())? .join("freenet") - .join("friends.json")) + .join(LEDGER_FILE)) } +/// Both the file the ledger lives in and the key its write lock is taken on. +const LEDGER_FILE: &str = "friends.json"; + fn load(app: &tauri::AppHandle) -> Store { path(app) .ok() @@ -294,6 +298,7 @@ async fn start_request( updated_at: now, }; deliver(app, &mut rel, Cargo::FriendRequest(req.clone()), encode(REQ_PREFIX, &req)?).await?; + let _ledger = ledger::hold(LEDGER_FILE).await; let mut store = load(app); store.relationships.insert(id.clone(), rel); save(app, &store)?; @@ -322,6 +327,7 @@ pub async fn request_by_key(app: &tauri::AppHandle, persona_key: &str) -> Result pub async fn accept(app: &tauri::AppHandle, id: &str) -> Result { let me = my_key(app).await?; + let _ledger = ledger::hold(LEDGER_FILE).await; let mut store = load(app); let rel = store.relationships.get_mut(id).ok_or("no such request")?; if rel.request_from_me { @@ -340,6 +346,7 @@ pub async fn accept(app: &tauri::AppHandle, id: &str) -> Result Result { + let _ledger = ledger::hold(LEDGER_FILE).await; let mut store = load(app); let rel = store.relationships.get_mut(id).ok_or("no such request")?; let now = now_secs(); @@ -355,6 +362,7 @@ pub async fn decline(app: &tauri::AppHandle, id: &str) -> Result Result { let me = my_key(app).await?; + let _ledger = ledger::hold(LEDGER_FILE).await; let mut store = load(app); let rel = store.relationships.get_mut(id).ok_or("no such relationship")?; let their = key_of(&rel.their_key)?; @@ -368,7 +376,8 @@ pub async fn remove(app: &tauri::AppHandle, id: &str) -> Result Result<(), String> { +pub async fn forget(app: &tauri::AppHandle, id: &str) -> Result<(), String> { + let _ledger = ledger::hold(LEDGER_FILE).await; let mut store = load(app); store.relationships.remove(id); save(app, &store) @@ -444,6 +453,7 @@ async fn store_request(app: &tauri::AppHandle, from: From, req: fr::FriendReques } }; let id = b58(&req.relationship_id); + let _ledger = ledger::hold(LEDGER_FILE).await; let mut store = load(app); if store.relationships.contains_key(&id) { return; @@ -479,6 +489,7 @@ async fn apply_assertion(app: &tauri::AppHandle, from: From, a: fr::FriendAssert return; } let id = b58(&a.relationship_id); + let _ledger = ledger::hold(LEDGER_FILE).await; let mut store = load(app); let Some(rel) = store.relationships.get_mut(&id) else { log::debug!("friends: assertion for an unknown relationship"); @@ -533,7 +544,8 @@ async fn apply_assertion(app: &tauri::AppHandle, from: From, a: fr::FriendAssert } } -fn apply_decline(app: &tauri::AppHandle, from: From, id: &str) { +async fn apply_decline(app: &tauri::AppHandle, from: From, id: &str) { + let _ledger = ledger::hold(LEDGER_FILE).await; let mut store = load(app); let Some(rel) = store.relationships.get_mut(id) else { return }; let sender_ok = match &from { @@ -555,8 +567,8 @@ pub(crate) async fn on_inbox_request(app: &tauri::AppHandle, sender: [u8; 32], r pub(crate) async fn on_inbox_assertion(app: &tauri::AppHandle, sender: [u8; 32], a: fr::FriendAssertion) { apply_assertion(app, From::Inbox(sender), a).await; } -pub(crate) fn on_inbox_decline(app: &tauri::AppHandle, sender: [u8; 32], id: [u8; 16]) { - apply_decline(app, From::Inbox(sender), &b58(&id)); +pub(crate) async fn on_inbox_decline(app: &tauri::AppHandle, sender: [u8; 32], id: [u8; 16]) { + apply_decline(app, From::Inbox(sender), &b58(&id)).await; } /// The event pump hands every private message here after the link proof. @@ -579,7 +591,10 @@ pub fn intercept_pm(app: &tauri::AppHandle, from: &str, message: &str) -> bool { return true; } if let Some(rest) = msg.strip_prefix(DECLINE_PREFIX) { - apply_decline(app, From::Pm(from.to_string()), rest.trim()); + // Its own task like the two above: this now takes the ledger, and an + // inline await here deadlocks the event pump. + let (app, from, id) = (app.clone(), from.to_string(), rest.trim().to_string()); + tauri::async_runtime::spawn(async move { apply_decline(&app, From::Pm(from), &id).await }); return true; } false @@ -612,6 +627,6 @@ pub async fn friend_remove(app: tauri::AppHandle, id: String) -> Result Result<(), String> { - forget(&app, &id) +pub async fn friend_forget(app: tauri::AppHandle, id: String) -> Result<(), String> { + forget(&app, &id).await } diff --git a/src-tauri/src/inbox.rs b/src-tauri/src/inbox.rs index e43f9df..bfef797 100644 --- a/src-tauri/src/inbox.rs +++ b/src-tauri/src/inbox.rs @@ -479,10 +479,10 @@ async fn dispatch(app: &tauri::AppHandle, sender: [u8; 32], cargo: Cargo) { match cargo { Cargo::FriendRequest(r) => crate::friends::on_inbox_request(app, sender, r).await, Cargo::FriendAssertion(a) => crate::friends::on_inbox_assertion(app, sender, a).await, - Cargo::FriendDecline(id) => crate::friends::on_inbox_decline(app, sender, id), - Cargo::Share(s) => crate::shared_playlists::on_inbox_share(app, sender, s), - Cargo::SpaceInvite(i) => crate::spaces::on_inbox_invite(app, sender, i), - Cargo::SpaceKey(k) => crate::spaces::on_inbox_space_key(app, sender, k), + Cargo::FriendDecline(id) => crate::friends::on_inbox_decline(app, sender, id).await, + Cargo::Share(s) => crate::shared_playlists::on_inbox_share(app, sender, s).await, + Cargo::SpaceInvite(i) => crate::spaces::on_inbox_invite(app, sender, i).await, + Cargo::SpaceKey(k) => crate::spaces::on_inbox_space_key(app, sender, k).await, } } diff --git a/src-tauri/src/ledger.rs b/src-tauri/src/ledger.rs index 5b52fae..5275341 100644 --- a/src-tauri/src/ledger.rs +++ b/src-tauri/src/ledger.rs @@ -63,6 +63,18 @@ mod tests { assert_eq!(CELL.load(Ordering::SeqCst), 2, "the second write clobbered the first"); } + /// Why spaces::invite, spaces::remove_member and proposals::decide all + /// `drop(ledger)` before their tail call into open(): this lock is NOT + /// reentrant, so re-taking the same file in the same task is a hang, not + /// an error. Nothing in the compiler catches that. + #[tokio::test] + async fn the_same_file_twice_would_hang() { + let _first = hold("test-reentrant.json").await; + let again = + tokio::time::timeout(std::time::Duration::from_millis(200), hold("test-reentrant.json")).await; + assert!(again.is_err(), "a second hold on an already-held file must never be granted"); + } + /// Different files must not block each other, or dispatching sealed cargo /// out of a held inbox would deadlock. #[tokio::test] diff --git a/src-tauri/src/proposals.rs b/src-tauri/src/proposals.rs index d22e85b..675bb10 100644 --- a/src-tauri/src/proposals.rs +++ b/src-tauri/src/proposals.rs @@ -30,6 +30,7 @@ use crate::persona; use crate::pipeline::LibraryState; use crate::playlists; use crate::shared_playlists::{self, Published}; +use crate::ledger; use crate::slsk_link; use crate::spaces; use crate::state::AppState; @@ -192,8 +193,11 @@ fn board_id(owner: &[u8; 32], uuid: &[u8; 16]) -> String { format!("{}/{}", b58(owner), b58(uuid)) } +/// Both the file the ledger lives in and the key its write lock is taken on. +const LEDGER_FILE: &str = "boards.json"; + fn path(app: &tauri::AppHandle) -> Result { - Ok(app.path().app_data_dir().map_err(|e| e.to_string())?.join("freenet").join("boards.json")) + Ok(app.path().app_data_dir().map_err(|e| e.to_string())?.join("freenet").join(LEDGER_FILE)) } fn load(app: &tauri::AppHandle) -> Ledger { path(app) @@ -392,6 +396,7 @@ pub async fn open(app: &tauri::AppHandle, space_owner_b58: &str, space_b58: &str let have: Vec<[u8; 32]> = board.authority.as_ref().map(|a| a.contributors.clone()).unwrap_or_default(); let same = want.iter().collect::>() == have.iter().collect::>(); if !same { + let _ledger = ledger::hold(LEDGER_FILE).await; let bid = board_id(&pin.owner, &pin.uuid); let mut l = load(app); let rec = l.boards.get(&bid).cloned().unwrap_or_default(); @@ -517,6 +522,7 @@ pub async fn decide(app: &tauri::AppHandle, space_owner_b58: &str, space_b58: &s return Err("the board has not reached this node".into()); } let p = board.proposals.iter().find(|p| p.proposal_id == pid).cloned().ok_or("that proposal is no longer pending")?; + let ledger = ledger::hold(LEDGER_FILE).await; let bid = board_id(&pin.owner, &pin.uuid); let mut l = load(app); let rec = l.boards.get(&bid).cloned().unwrap_or_default(); @@ -544,6 +550,8 @@ pub async fn decide(app: &tauri::AppHandle, space_owner_b58: &str, space_b58: &s l.boards.insert(bid, BoardRecord { last_seq: seq, ..rec }); save(app, &l)?; log::info!("proposals: decision {} ({}) on board {}", seq, if accept { "accept" } else { "reject" }, b58(&pin.uuid)); + // open() takes this same ledger; the write is done, so let go first. + drop(ledger); open(app, space_owner_b58, space_b58).await } diff --git a/src-tauri/src/shared_playlists.rs b/src-tauri/src/shared_playlists.rs index 3b37b14..803bdb3 100644 --- a/src-tauri/src/shared_playlists.rs +++ b/src-tauri/src/shared_playlists.rs @@ -29,6 +29,7 @@ use serde::{Deserialize, Serialize}; use tauri::{Emitter, Manager, State}; use crate::inbox::{self, Cargo, ShareCargo}; +use crate::ledger as ledger_lock; use crate::persona; use crate::pipeline::LibraryState; use crate::playlist_formats::{ParsedEntry, ParsedPlaylist}; @@ -170,8 +171,11 @@ fn path(app: &tauri::AppHandle) -> Result { .app_data_dir() .map_err(|e| e.to_string())? .join("freenet") - .join("playlists-ledger.json")) + .join(LEDGER_FILE)) } +/// Both the file the ledger lives in and the key its write lock is taken on. +const LEDGER_FILE: &str = "playlists-ledger.json"; + fn load(app: &tauri::AppHandle) -> Ledger { path(app) .ok() @@ -231,6 +235,7 @@ pub async fn publish(app: &tauri::AppHandle, lib: &LibraryState, playlist_id: i6 if items.len() > pl::MAX_ENTRIES { return Err(format!("{} entries is over the {} cap", items.len(), pl::MAX_ENTRIES)); } + let _guard = ledger_lock::hold(LEDGER_FILE).await; let mut ledger = load(app); let key = playlist_id.to_string(); let prev = ledger.published.get(&key).cloned(); @@ -331,6 +336,7 @@ pub async fn publish(app: &tauri::AppHandle, lib: &LibraryState, playlist_id: i6 pub async fn withdraw(app: &tauri::AppHandle, playlist_id: i64) -> Result { let adapter = app.state::().freenet.clone(); let owner = own_key(app).await?; + let _guard = ledger_lock::hold(LEDGER_FILE).await; let mut ledger = load(app); let key = playlist_id.to_string(); let prev = ledger.published.get(&key).cloned().ok_or("that playlist was never published")?; @@ -564,8 +570,9 @@ pub async fn send(app: &tauri::AppHandle, username: &str, playlist_id: i64) -> R Ok(()) } -fn keep_card(app: &tauri::AppHandle, card: ShareCard) { +async fn keep_card(app: &tauri::AppHandle, card: ShareCard) { let who = if card.from_username.is_empty() { card.from_key.clone() } else { card.from_username.clone() }; + let _guard = ledger_lock::hold(LEDGER_FILE).await; let mut ledger = load(app); ledger.inbox.retain(|c| c.id != card.id); ledger.inbox.push(card); @@ -577,7 +584,7 @@ fn keep_card(app: &tauri::AppHandle, card: ShareCard) { } /// Sealed cargo from inbox.rs: the envelope's signer is the sender. -pub(crate) fn on_inbox_share(app: &tauri::AppHandle, sender: [u8; 32], s: ShareCargo) { +pub(crate) async fn on_inbox_share(app: &tauri::AppHandle, sender: [u8; 32], s: ShareCargo) { let from_key = b58(&sender); let from_username = crate::slsk_link::username_for_key(app, &from_key).unwrap_or_default(); keep_card( @@ -593,7 +600,8 @@ pub(crate) fn on_inbox_share(app: &tauri::AppHandle, sender: [u8; 32], s: ShareC entry_count: s.entry_count, received_at: now_secs(), }, - ); + ) + .await; } /// The event pump hands private messages here after the link and friend @@ -603,9 +611,7 @@ pub fn intercept_pm(app: &tauri::AppHandle, from: &str, message: &str) -> bool { use base64::Engine; let Ok(bytes) = base64::engine::general_purpose::STANDARD.decode(rest.trim()) else { return true }; let Ok(r) = bincode::deserialize::(&bytes) else { return true }; - keep_card( - app, - ShareCard { + let card = ShareCard { id: format!("{}:{}:{}", normalize_username(from), b58(&r.owner), b58(&r.playlist_uuid)), from_username: from.to_string(), from_key: String::new(), @@ -615,8 +621,11 @@ pub fn intercept_pm(app: &tauri::AppHandle, from: &str, message: &str) -> bool { name: cap(&r.name, pl::MAX_NAME_BYTES), entry_count: r.entry_count, received_at: now_secs(), - }, - ); + }; + // Its own task: keeping a card now takes the ledger, and an inline await + // here would block the event pump. + let app = app.clone(); + tauri::async_runtime::spawn(async move { keep_card(&app, card).await }); true } @@ -664,7 +673,8 @@ pub fn shares_inbox(app: tauri::AppHandle) -> Vec { v } #[tauri::command] -pub fn share_dismiss(app: tauri::AppHandle, id: String) -> Result<(), String> { +pub async fn share_dismiss(app: tauri::AppHandle, id: String) -> Result<(), String> { + let _guard = ledger_lock::hold(LEDGER_FILE).await; let mut ledger = load(&app); ledger.inbox.retain(|c| c.id != id); save(&app, &ledger) diff --git a/src-tauri/src/spaces.rs b/src-tauri/src/spaces.rs index c420514..226617d 100644 --- a/src-tauri/src/spaces.rs +++ b/src-tauri/src/spaces.rs @@ -25,6 +25,7 @@ use serde::{Deserialize, Serialize}; use tauri::{Emitter, Manager}; use crate::inbox::{self, Cargo, SpaceInvite, SpaceKeyCargo}; +use crate::ledger; use crate::persona; use crate::slsk_link; use crate::state::AppState; @@ -159,8 +160,11 @@ fn path(app: &tauri::AppHandle) -> Result { .app_data_dir() .map_err(|e| e.to_string())? .join("freenet") - .join("spaces.json")) + .join(LEDGER_FILE)) } +/// Both the file the ledger lives in and the key its write lock is taken on. +const LEDGER_FILE: &str = "spaces.json"; + fn load(app: &tauri::AppHandle) -> Ledger { path(app) .ok() @@ -363,6 +367,7 @@ pub async fn create(app: &tauri::AppHandle, name: &str, description: &str, priva keys, last_epoch: 1, }; + let _ledger = ledger::hold(LEDGER_FILE).await; let mut l = load(app); l.spaces.insert(record_id(&rec.owner, &rec.space_id), rec.clone()); save(app, &l)?; @@ -374,6 +379,7 @@ pub async fn open(app: &tauri::AppHandle, owner_b58: &str, space_b58: &str) -> R let me = my_key(app).await?; let owner = key_of(owner_b58)?; let space_id = id_of(space_b58)?; + let _ledger = ledger::hold(LEDGER_FILE).await; let mut l = load(app); let rid = record_id(owner_b58, space_b58); let rec = l.spaces.get(&rid).cloned().ok_or("not a Space this device knows")?; @@ -475,6 +481,7 @@ pub async fn invite(app: &tauri::AppHandle, owner_b58: &str, space_b58: &str, pe }) .await?; let rid = record_id(owner_b58, space_b58); + let ledger = ledger::hold(LEDGER_FILE).await; let mut l = load(app); let rec = l.spaces.get_mut(&rid).ok_or("not a Space this device knows")?; let mut key = None; @@ -493,6 +500,8 @@ pub async fn invite(app: &tauri::AppHandle, owner_b58: &str, space_b58: &str, pe ) .await?; log::info!("spaces: invited {persona_key} to {:?} (epoch {})", c.name, c.epoch); + // open() takes this same ledger; the write is done, so let go first. + drop(ledger); open(app, owner_b58, space_b58).await } @@ -506,6 +515,7 @@ pub async fn remove_member(app: &tauri::AppHandle, owner_b58: &str, space_b58: & } let c = bump(app, me, owner, space_id, |c| c.members.retain(|m| m.key != gone)).await?; let rid = record_id(owner_b58, space_b58); + let ledger = ledger::hold(LEDGER_FILE).await; let mut l = load(app); let rec = l.spaces.get_mut(&rid).ok_or("not a Space this device knows")?; rec.last_epoch = c.epoch; @@ -523,6 +533,8 @@ pub async fn remove_member(app: &tauri::AppHandle, owner_b58: &str, space_b58: & } else { save(app, &l)?; } + // open() takes this same ledger; the write is done, so let go first. + drop(ledger); open(app, owner_b58, space_b58).await } @@ -538,7 +550,8 @@ pub async fn set_pinned(app: &tauri::AppHandle, owner_b58: &str, space_b58: &str open(app, owner_b58, space_b58).await } -pub fn leave(app: &tauri::AppHandle, owner_b58: &str, space_b58: &str) -> Result<(), String> { +pub async fn leave(app: &tauri::AppHandle, owner_b58: &str, space_b58: &str) -> Result<(), String> { + let _ledger = ledger::hold(LEDGER_FILE).await; let mut l = load(app); let rid = record_id(owner_b58, space_b58); if l.spaces.get(&rid).is_some_and(|r| r.my_role == "owner") { @@ -550,12 +563,13 @@ pub fn leave(app: &tauri::AppHandle, owner_b58: &str, space_b58: &str) -> Result // ---------------------------------------------------------------- inbox cargo -pub(crate) fn on_inbox_invite(app: &tauri::AppHandle, sender: [u8; 32], inv: SpaceInvite) { +pub(crate) async fn on_inbox_invite(app: &tauri::AppHandle, sender: [u8; 32], inv: SpaceInvite) { if sender != inv.owner { log::warn!("spaces: an invitation was sealed by someone other than the Space's owner"); return; } let rid = record_id(&b58(&inv.owner), &b58(&inv.space_id)); + let _ledger = ledger::hold(LEDGER_FILE).await; let mut l = load(app); let rec = l.spaces.entry(rid).or_insert_with(|| SpaceRecord { owner: b58(&inv.owner), @@ -579,11 +593,12 @@ pub(crate) fn on_inbox_invite(app: &tauri::AppHandle, sender: [u8; 32], inv: Spa } } -pub(crate) fn on_inbox_space_key(app: &tauri::AppHandle, sender: [u8; 32], sk: SpaceKeyCargo) { +pub(crate) async fn on_inbox_space_key(app: &tauri::AppHandle, sender: [u8; 32], sk: SpaceKeyCargo) { if sender != sk.owner { return; } let rid = record_id(&b58(&sk.owner), &b58(&sk.space_id)); + let _ledger = ledger::hold(LEDGER_FILE).await; let mut l = load(app); let Some(rec) = l.spaces.get_mut(&rid) else { return }; rec.keys.insert(sk.epoch.to_string(), b58(&sk.key)); @@ -621,8 +636,8 @@ pub async fn space_set_pinned(app: tauri::AppHandle, owner: String, space: Strin set_pinned(&app, &owner, &space, playlist_owner, playlist_uuid).await } #[tauri::command] -pub fn space_leave(app: tauri::AppHandle, owner: String, space: String) -> Result<(), String> { - leave(&app, &owner, &space) +pub async fn space_leave(app: tauri::AppHandle, owner: String, space: String) -> Result<(), String> { + leave(&app, &owner, &space).await } #[cfg(test)]