ddidderr/lanspread
1
0
Fork 0
Code Issues 7 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(peer): repair update lifecycle regressions

Browse Source 
FINDINGS.md identified three merge blockers in the post-plan install/update
flow.

Updates now use FetchLatestFromPeers so the Tauri update command bypasses
local manifest serving and asks peers that advertise the latest version for
fresh file metadata. PeerGameDB now aggregates and validates file descriptions
from latest-version peers, keeping stale cached metadata for older versions
from poisoning chunk planning when filenames stay the same but sizes change.

Download-to-install handoff now performs explicit async state transitions.
The download task mutates Downloading to Installing or Updating under the
active-operation write lock, clears the cancellation token, and then runs the
install transaction. OperationGuard remains armed only as crash or abort
cleanup and is disarmed after normal explicit cleanup, so final refreshes no
longer race a deferred Drop cleanup.

Local library index writers now serialize the load/mutate/save window with one
async mutex. The index fingerprint also includes the root version.ini contents
so a same-length version rewrite in the same mtime second still updates the
reported local version.

The tradeoff is that local index mutations are serialized in-process instead
of moved into a dedicated actor. That keeps the fix small and scoped to the
merge blockers while preserving the existing scanner API.

Test Plan:
- just fmt
- just test
- just clippy
- just build
- git diff --check

Refs:
- FINDINGS.md
This commit is contained in:
ddidderr
2026-05-16 14:19:10 +02:00
parent 8890d78642
commit 6242d64583
9 changed files with 722 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/lanspread-peer/src/handlers.rs
+470 -55
View File
@@ -2,6 +2,7 @@
use std::{
collections::{HashSet, hash_map::Entry},
future::Future,
net::SocketAddr,
path::{Path, PathBuf},
sync::Arc,
@@ -80,17 +81,21 @@ async fn try_serve_local_game(
}
/// Handles the `GetGame` command.
pub async fn handle_get_game_command(
pub(crate) async fn handle_get_game_command(
ctx: &Ctx,
tx_notify_ui: &UnboundedSender<PeerEvent>,
id: String,
source: GameDetailSource,
) {
if try_serve_local_game(ctx, tx_notify_ui, &id).await {
if source.allows_local() && try_serve_local_game(ctx, tx_notify_ui, &id).await {
return;
}
log::info!("Requesting game from peers: {id}");
let peers = { ctx.peer_game_db.read().await.peers_with_game(&id) };
let peers = {
let peer_game_db = ctx.peer_game_db.read().await;
source.select_peers(&peer_game_db, &id)
};
if peers.is_empty() {
log::warn!("No peers have game {id}");
if let Err(e) = tx_notify_ui.send(PeerEvent::NoPeersHaveGame { id: id.clone() }) {
@@ -101,33 +106,34 @@ pub async fn handle_get_game_command(
let peer_game_db = ctx.peer_game_db.clone();
let tx_notify_ui = tx_notify_ui.clone();
ctx.task_tracker.spawn(async move {
let mut fetched_any = false;
for peer_addr in peers {
match request_game_details_and_update(peer_addr, &id, peer_game_db.clone()).await {
Ok(_) => {
log::info!("Fetched game file list for {id} from peer {peer_addr}");
fetched_any = true;
}
Err(e) => {
log::error!("Failed to fetch game files for {id} from {peer_addr}: {e}");
}
}
}
ctx.task_tracker.spawn(fetch_game_details_from_peers(
peers,
id,
peer_game_db,
tx_notify_ui,
|peer_addr, game_id, peer_game_db| async move {
request_game_details_and_update(peer_addr, &game_id, peer_game_db).await
},
));
}
if fetched_any {
let aggregated_files = { peer_game_db.read().await.aggregated_game_files(&id) };
#[derive(Clone, Copy, Debug)]
pub(crate) enum GameDetailSource {
LocalOrPeers,
LatestPeersOnly,
}
if let Err(e) = tx_notify_ui.send(PeerEvent::GotGameFiles {
id: id.clone(),
file_descriptions: aggregated_files,
}) {
log::error!("Failed to send GotGameFiles event: {e}");
}
} else {
log::warn!("Failed to retrieve game files for {id} from any peer");
impl GameDetailSource {
fn allows_local(self) -> bool {
matches!(self, Self::LocalOrPeers)
}
fn select_peers(self, peer_game_db: &PeerGameDB, id: &str) -> Vec<SocketAddr> {
match self {
Self::LocalOrPeers => peer_game_db.peers_with_game(id),
Self::LatestPeersOnly => peer_game_db.peers_with_latest_version(id),
}
});
}
}
/// Requests game details from a peer and updates the peer game database.
@@ -147,6 +153,43 @@ async fn request_game_details_and_update(
Ok(file_descriptions)
}
async fn fetch_game_details_from_peers<F, Fut>(
peers: Vec<SocketAddr>,
id: String,
peer_game_db: Arc<RwLock<PeerGameDB>>,
tx_notify_ui: UnboundedSender<PeerEvent>,
mut fetch_details: F,
) where
F: FnMut(SocketAddr, String, Arc<RwLock<PeerGameDB>>) -> Fut + Send + 'static,
Fut: Future<Output = eyre::Result<Vec<GameFileDescription>>> + Send,
{
let mut fetched_any = false;
for peer_addr in peers {
match fetch_details(peer_addr, id.clone(), peer_game_db.clone()).await {
Ok(_) => {
log::info!("Fetched game file list for {id} from peer {peer_addr}");
fetched_any = true;
}
Err(e) => {
log::error!("Failed to fetch game files for {id} from {peer_addr}: {e}");
}
}
}
if fetched_any {
let aggregated_files = { peer_game_db.read().await.aggregated_game_files(&id) };
if let Err(e) = tx_notify_ui.send(PeerEvent::GotGameFiles {
id: id.clone(),
file_descriptions: aggregated_files,
}) {
log::error!("Failed to send GotGameFiles event: {e}");
}
} else {
log::warn!("Failed to retrieve game files for {id} from any peer");
}
}
/// Handles the `DownloadGameFiles` command.
#[allow(clippy::too_many_lines)]
pub async fn handle_download_game_files_command(
@@ -252,27 +295,49 @@ pub async fn handle_download_game_files_command(
.insert(id, cancel_token.clone());
ctx.task_tracker.spawn(async move {
let result = {
let _download_state_guard =
OperationGuard::download(download_id.clone(), active_operations, active_downloads);
let download_state_guard =
OperationGuard::download(download_id.clone(), active_operations, active_downloads);
download_game_files(
&download_id,
resolved_descriptions,
games_folder,
peer_whitelist,
file_peer_map,
tx_notify_ui_clone.clone(),
cancel_token,
)
.await
};
let result = download_game_files(
&download_id,
resolved_descriptions,
games_folder,
peer_whitelist,
file_peer_map,
tx_notify_ui_clone.clone(),
cancel_token,
)
.await;
match result {
Ok(()) => {
run_install_operation(&ctx_clone, &tx_notify_ui_clone, download_id).await;
let Some(prepared) =
prepare_install_operation(&ctx_clone, &tx_notify_ui_clone, &download_id).await
else {
end_download_operation(&ctx_clone, &download_id).await;
download_state_guard.disarm();
return;
};
if transition_download_to_install(&ctx_clone, &download_id, prepared.operation_kind)
.await
{
clear_active_download(&ctx_clone, &download_id).await;
run_started_install_operation(
&ctx_clone,
&tx_notify_ui_clone,
download_id,
prepared,
)
.await;
} else {
clear_active_download(&ctx_clone, &download_id).await;
}
download_state_guard.disarm();
}
Err(e) => {
end_download_operation(&ctx_clone, &download_id).await;
download_state_guard.disarm();
log::error!("Download failed for {download_id}: {e}");
}
}
@@ -310,16 +375,42 @@ fn spawn_install_operation(ctx: &Ctx, tx_notify_ui: &UnboundedSender<PeerEvent>,
}
async fn run_install_operation(ctx: &Ctx, tx_notify_ui: &UnboundedSender<PeerEvent>, id: String) {
if !catalog_contains(ctx, &id).await {
log::warn!("Ignoring install command for non-catalog game {id}");
let Some(prepared) = prepare_install_operation(ctx, tx_notify_ui, &id).await else {
return;
};
if !begin_operation(ctx, &id, prepared.operation_kind).await {
log::warn!("Operation for {id} already in progress; ignoring install command");
return;
}
let game_root = { ctx.game_dir.read().await.join(&id) };
run_started_install_operation(ctx, tx_notify_ui, id, prepared).await;
}
struct PreparedInstallOperation {
game_root: PathBuf,
operation: InstallOperation,
operation_kind: OperationKind,
}
async fn prepare_install_operation(
ctx: &Ctx,
tx_notify_ui: &UnboundedSender<PeerEvent>,
id: &str,
) -> Option<PreparedInstallOperation> {
if !catalog_contains(ctx, id).await {
log::warn!("Ignoring install command for non-catalog game {id}");
return None;
}
let game_root = { ctx.game_dir.read().await.join(id) };
if !version_ini_is_regular_file(&game_root).await {
log::warn!("Ignoring install command for {id}: version.ini sentinel is absent");
events::send(tx_notify_ui, PeerEvent::InstallGameFailed { id });
return;
events::send(
tx_notify_ui,
PeerEvent::InstallGameFailed { id: id.to_string() },
);
return None;
}
let local_present = local_dir_is_directory(&game_root).await;
@@ -333,13 +424,27 @@ async fn run_install_operation(ctx: &Ctx, tx_notify_ui: &UnboundedSender<PeerEve
InstallOperation::Updating => OperationKind::Updating,
};
if !begin_operation(ctx, &id, operation_kind).await {
log::warn!("Operation for {id} already in progress; ignoring install command");
return;
}
Some(PreparedInstallOperation {
game_root,
operation,
operation_kind,
})
}
async fn run_started_install_operation(
ctx: &Ctx,
tx_notify_ui: &UnboundedSender<PeerEvent>,
id: String,
prepared: PreparedInstallOperation,
) {
let PreparedInstallOperation {
game_root,
operation,
..
} = prepared;
let operation_guard = OperationGuard::new(id.clone(), ctx.active_operations.clone());
let result = {
let _operation_guard = OperationGuard::new(id.clone(), ctx.active_operations.clone());
events::send(
tx_notify_ui,
PeerEvent::InstallGameBegin {
@@ -357,6 +462,8 @@ async fn run_install_operation(ctx: &Ctx, tx_notify_ui: &UnboundedSender<PeerEve
}
}
};
end_operation(ctx, &id).await;
operation_guard.disarm();
match result {
Ok(()) => {
@@ -393,8 +500,8 @@ async fn run_uninstall_operation(ctx: &Ctx, tx_notify_ui: &UnboundedSender<PeerE
}
let game_root = { ctx.game_dir.read().await.join(&id) };
let operation_guard = OperationGuard::new(id.clone(), ctx.active_operations.clone());
let result = {
let _operation_guard = OperationGuard::new(id.clone(), ctx.active_operations.clone());
events::send(
tx_notify_ui,
PeerEvent::UninstallGameBegin { id: id.clone() },
@@ -402,6 +509,8 @@ async fn run_uninstall_operation(ctx: &Ctx, tx_notify_ui: &UnboundedSender<PeerE
install::uninstall(&game_root, &id).await
};
end_operation(ctx, &id).await;
operation_guard.disarm();
match result {
Ok(()) => {
@@ -435,6 +544,39 @@ async fn begin_operation(ctx: &Ctx, id: &str, operation: OperationKind) -> bool
}
}
async fn transition_download_to_install(ctx: &Ctx, id: &str, operation: OperationKind) -> bool {
let mut active_operations = ctx.active_operations.write().await;
match active_operations.get_mut(id) {
Some(current) if *current == OperationKind::Downloading => {
*current = operation;
true
}
Some(current) => {
log::warn!(
"Cannot transition {id} from download to install; current operation is {current:?}"
);
false
}
None => {
log::warn!("Cannot transition {id} from download to install; operation is not active");
false
}
}
}
async fn end_operation(ctx: &Ctx, id: &str) {
ctx.active_operations.write().await.remove(id);
}
async fn clear_active_download(ctx: &Ctx, id: &str) {
ctx.active_downloads.write().await.remove(id);
}
async fn end_download_operation(ctx: &Ctx, id: &str) {
end_operation(ctx, id).await;
clear_active_download(ctx, id).await;
}
async fn catalog_contains(ctx: &Ctx, id: &str) -> bool {
ctx.catalog.read().await.contains(id)
}
@@ -650,11 +792,13 @@ fn active_operation_kind(operation: OperationKind) -> ActiveOperationKind {
mod tests {
use std::{
collections::HashSet,
net::SocketAddr,
path::{Path, PathBuf},
sync::Arc,
sync::{Arc, Mutex},
time::Duration,
};
use lanspread_proto::{Availability, GameSummary};
use tokio::sync::mpsc;
use tokio_util::{sync::CancellationToken, task::TaskTracker};
@@ -698,7 +842,41 @@ mod tests {
.expect("event channel should remain open")
}
fn addr(port: u16) -> SocketAddr {
SocketAddr::from(([127, 0, 0, 1], port))
}
fn summary(id: &str, version: &str, availability: Availability) -> GameSummary {
GameSummary {
id: id.to_string(),
name: id.to_string(),
size: 42,
downloaded: availability.is_downloaded(),
installed: true,
eti_version: Some(version.to_string()),
manifest_hash: 7,
availability,
}
}
fn file_desc(game_id: &str, relative_path: &str, size: u64) -> GameFileDescription {
GameFileDescription {
game_id: game_id.to_string(),
relative_path: relative_path.to_string(),
is_dir: false,
size,
}
}
fn assert_local_update(event: PeerEvent, installed: bool, downloaded: bool) {
let _ = local_update_game(event, installed, downloaded);
}
fn local_update_game(
event: PeerEvent,
installed: bool,
downloaded: bool,
) -> lanspread_db::db::Game {
let PeerEvent::LocalGamesUpdated {
games,
active_operations,
@@ -711,11 +889,135 @@ mod tests {
"settled local update should not report active operations"
);
let game = games
.iter()
.into_iter()
.find(|game| game.id == "game")
.expect("game should be announced");
assert_eq!(game.installed, installed);
assert_eq!(game.downloaded, downloaded);
game
}
#[test]
fn update_source_selects_latest_ready_peer_manifest() {
let old_addr = addr(12_000);
let new_addr = addr(12_001);
let local_only_addr = addr(12_002);
let mut db = PeerGameDB::new();
db.upsert_peer("old".to_string(), old_addr);
db.upsert_peer("new".to_string(), new_addr);
db.upsert_peer("local-only".to_string(), local_only_addr);
db.update_peer_games(
&"old".to_string(),
vec![summary("game", "20240101", Availability::Ready)],
);
db.update_peer_games(
&"new".to_string(),
vec![summary("game", "20250101", Availability::Ready)],
);
db.update_peer_games(
&"local-only".to_string(),
vec![summary("game", "20990101", Availability::LocalOnly)],
);
assert_eq!(
GameDetailSource::LatestPeersOnly.select_peers(&db, "game"),
vec![new_addr]
);
}
#[tokio::test]
async fn update_fetch_emits_fresh_manifest_from_latest_peer() {
let old_addr = addr(12_010);
let new_addr = addr(12_011);
let peer_game_db = Arc::new(RwLock::new(PeerGameDB::new()));
{
let mut db = peer_game_db.write().await;
db.upsert_peer("old".to_string(), old_addr);
db.upsert_peer("new".to_string(), new_addr);
db.update_peer_games(
&"old".to_string(),
vec![summary("game", "20240101", Availability::Ready)],
);
db.update_peer_games(
&"new".to_string(),
vec![summary("game", "20250101", Availability::Ready)],
);
}
let peers = {
let db = peer_game_db.read().await;
GameDetailSource::LatestPeersOnly.select_peers(&db, "game")
};
let (tx, mut rx) = mpsc::unbounded_channel();
let fetched_peers = Arc::new(Mutex::new(Vec::new()));
fetch_game_details_from_peers(peers, "game".to_string(), peer_game_db.clone(), tx, {
let fetched_peers = fetched_peers.clone();
move |peer_addr, game_id, peer_game_db| {
let fetched_peers = fetched_peers.clone();
async move {
fetched_peers
.lock()
.expect("fetched peer list should not be poisoned")
.push(peer_addr);
let files = vec![
file_desc(&game_id, "game/version.ini", 8),
file_desc(&game_id, "game/new.eti", 11),
];
peer_game_db.write().await.update_peer_game_files(
&"new".to_string(),
&game_id,
files.clone(),
);
Ok(files)
}
}
})
.await;
assert_eq!(
*fetched_peers
.lock()
.expect("fetched peer list should not be poisoned"),
vec![new_addr]
);
let PeerEvent::GotGameFiles {
id,
file_descriptions,
} = recv_event(&mut rx).await
else {
panic!("expected GotGameFiles");
};
assert_eq!(id, "game");
assert!(
file_descriptions
.iter()
.any(|desc| desc.relative_path == "game/new.eti" && desc.size == 11),
"latest peer manifest should be emitted to the download path"
);
}
#[tokio::test]
async fn update_request_skips_local_manifest_even_when_download_exists() {
let temp = TempDir::new("lanspread-handler-latest-peer");
let root = temp.game_root();
write_file(&root.join("version.ini"), b"20240101");
write_file(&root.join("game.eti"), b"old archive");
let ctx = test_ctx(temp.path().to_path_buf());
let (tx, mut rx) = mpsc::unbounded_channel();
handle_get_game_command(
&ctx,
&tx,
"game".to_string(),
GameDetailSource::LatestPeersOnly,
)
.await;
assert!(matches!(
recv_event(&mut rx).await,
PeerEvent::NoPeersHaveGame { id } if id == "game"
));
}
#[tokio::test]
@@ -826,6 +1128,62 @@ mod tests {
assert_local_update(recv_event(&mut rx).await, true, true);
}
#[tokio::test]
async fn download_handoff_waits_for_readers_and_auto_installs() {
let temp = TempDir::new("lanspread-handler-download-handoff");
let root = temp.game_root();
write_file(&root.join("version.ini"), b"20250101");
write_file(&root.join("game.eti"), b"archive");
let ctx = test_ctx(temp.path().to_path_buf());
ctx.active_operations
.write()
.await
.insert("game".to_string(), OperationKind::Downloading);
ctx.active_downloads
.write()
.await
.insert("game".to_string(), CancellationToken::new());
let (prepare_tx, _prepare_rx) = mpsc::unbounded_channel();
let prepared = prepare_install_operation(&ctx, &prepare_tx, "game")
.await
.expect("downloaded game should be installable");
let read_guard = ctx.active_operations.read().await;
let (tx, mut rx) = mpsc::unbounded_channel();
let install_task = tokio::spawn({
let ctx = ctx.clone();
let tx = tx.clone();
async move {
assert!(
transition_download_to_install(&ctx, "game", prepared.operation_kind).await
);
clear_active_download(&ctx, "game").await;
run_started_install_operation(&ctx, &tx, "game".to_string(), prepared).await;
}
});
tokio::task::yield_now().await;
assert_eq!(read_guard.get("game"), Some(&OperationKind::Downloading));
drop(read_guard);
install_task.await.expect("handoff task should finish");
match recv_event(&mut rx).await {
PeerEvent::InstallGameBegin { id, operation } => {
assert_eq!(id, "game");
assert_eq!(operation, InstallOperation::Installing);
}
_ => panic!("expected InstallGameBegin"),
}
assert!(matches!(
recv_event(&mut rx).await,
PeerEvent::InstallGameFinished { id } if id == "game"
));
assert!(ctx.active_operations.read().await.is_empty());
assert!(ctx.active_downloads.read().await.is_empty());
assert_local_update(recv_event(&mut rx).await, true, true);
}
#[tokio::test]
async fn update_refreshes_settled_state_after_guard_release() {
let temp = TempDir::new("lanspread-handler-update");
@@ -854,6 +1212,63 @@ mod tests {
assert_local_update(recv_event(&mut rx).await, true, true);
}
#[tokio::test]
async fn install_update_uninstall_sequence_reports_new_version_and_settled_state() {
let temp = TempDir::new("lanspread-handler-sequence");
let root = temp.game_root();
write_file(&root.join("version.ini"), b"20240101");
write_file(&root.join("game.eti"), b"old archive");
let ctx = test_ctx(temp.path().to_path_buf());
let (tx, mut rx) = mpsc::unbounded_channel();
run_install_operation(&ctx, &tx, "game".to_string()).await;
assert!(matches!(
recv_event(&mut rx).await,
PeerEvent::InstallGameBegin {
id,
operation: InstallOperation::Installing
} if id == "game"
));
assert!(matches!(
recv_event(&mut rx).await,
PeerEvent::InstallGameFinished { id } if id == "game"
));
let game = local_update_game(recv_event(&mut rx).await, true, true);
assert_eq!(game.local_version.as_deref(), Some("20240101"));
write_file(&root.join("version.ini"), b"20250101");
write_file(&root.join("game.eti"), b"new archive");
run_install_operation(&ctx, &tx, "game".to_string()).await;
assert!(matches!(
recv_event(&mut rx).await,
PeerEvent::InstallGameBegin {
id,
operation: InstallOperation::Updating
} if id == "game"
));
assert!(matches!(
recv_event(&mut rx).await,
PeerEvent::InstallGameFinished { id } if id == "game"
));
let game = local_update_game(recv_event(&mut rx).await, true, true);
assert_eq!(game.local_version.as_deref(), Some("20250101"));
run_uninstall_operation(&ctx, &tx, "game".to_string()).await;
assert!(matches!(
recv_event(&mut rx).await,
PeerEvent::UninstallGameBegin { id } if id == "game"
));
assert!(matches!(
recv_event(&mut rx).await,
PeerEvent::UninstallGameFinished { id } if id == "game"
));
let game = local_update_game(recv_event(&mut rx).await, false, true);
assert_eq!(game.local_version.as_deref(), Some("20250101"));
assert!(ctx.active_operations.read().await.is_empty());
}
#[tokio::test]
async fn uninstall_refreshes_settled_state_after_guard_release() {
let temp = TempDir::new("lanspread-handler-uninstall");