feat(peer): coordinate outbound transfers with local game mutations
Updating or removing a local game rewrites its on-disk files. Peers that were mid-download of that game would keep streaming bytes from files that are being deleted or replaced, handing them a corrupt or stale copy. There was also no authoritative notion of which game version a peer should serve or accept, so a peer could serve whatever happened to be on disk and downloaders could aggregate files from peers running mismatched versions. This introduces a reader-writer coordination scheme between outbound file transfers (readers) and local mutation operations (writers), and gates both serving and downloading on an authoritative game catalog version. Reader-writer coordination: - Track active outbound transfers per game in a shared `OutboundTransfers` map of (id, CancellationToken), threaded through `Ctx`/`PeerCtx` and registered by a `TransferGuard` in the stream service. The guard is registered *before* the serve-eligibility check to close a TOCTOU window where a writer could miss an in-flight reader. - `stream_file_bytes` now honors a cancellation token at every await point (file read, network send, stream close) via `tokio::select!`, so a transfer aborts promptly instead of hanging on a stalled receiver. - `begin_operation` marks a game active first, then cancels its outbound transfers and waits for the count to reach zero before any Updating/RemovingDownload work touches the filesystem. - Active games are now hidden from library snapshots entirely while an operation is in flight, instead of freezing their last announced state, so peers stop discovering a game that is being mutated. Authoritative version catalog: - Replace the `HashSet<String>` catalog with `GameCatalog`, mapping each game id to its expected version (from the bundled game.db / ETI data). - Serving requires the local `version.ini` to match the catalog version (`local_download_matches_catalog`); peer selection, file aggregation, and majority size validation all filter on the expected version (`peers_with_expected_version`, `aggregated_game_files`, and friends). User-visible changes: - The GUI shows confirmation dialogs before Update and Remove, and surfaces a sharing-status indicator on game cards and the detail modal. - A new `OutboundTransferCountChanged` event lets the UI reflect live outbound transfer activity. Test Plan: - just test - just frontend-test - just clippy
This commit is contained in:
@@ -23,7 +23,7 @@ use crate::{
|
||||
game_from_summary,
|
||||
get_game_file_descriptions,
|
||||
local_dir_is_directory,
|
||||
local_download_available,
|
||||
local_download_matches_catalog,
|
||||
rescan_local_game,
|
||||
scan_local_library,
|
||||
version_ini_is_regular_file,
|
||||
@@ -41,7 +41,7 @@ use crate::{
|
||||
/// Handles the `ListGames` command.
|
||||
pub async fn handle_list_games_command(ctx: &Ctx, tx_notify_ui: &UnboundedSender<PeerEvent>) {
|
||||
log::info!("ListGames command received");
|
||||
events::emit_peer_game_list(&ctx.peer_game_db, tx_notify_ui).await;
|
||||
events::emit_peer_game_list(&ctx.peer_game_db, &ctx.catalog, tx_notify_ui).await;
|
||||
}
|
||||
|
||||
/// Tries to serve a game from local files.
|
||||
@@ -54,7 +54,7 @@ async fn try_serve_local_game(
|
||||
|
||||
let active_operations = ctx.active_operations.read().await;
|
||||
let catalog = ctx.catalog.read().await;
|
||||
if !local_download_available(&game_dir, id, &active_operations, &catalog).await {
|
||||
if !local_download_matches_catalog(&game_dir, id, &active_operations, &catalog).await {
|
||||
return false;
|
||||
}
|
||||
drop(active_operations);
|
||||
@@ -90,9 +90,10 @@ pub(crate) async fn handle_get_game_command(
|
||||
}
|
||||
|
||||
log::info!("Requesting game from peers: {id}");
|
||||
let expected_version = catalog_expected_version(ctx, &id).await;
|
||||
let peers = {
|
||||
let peer_game_db = ctx.peer_game_db.read().await;
|
||||
source.select_peers(&peer_game_db, &id)
|
||||
source.select_peers(&peer_game_db, &id, expected_version.as_deref())
|
||||
};
|
||||
if peers.is_empty() {
|
||||
log::warn!("No peers have game {id}");
|
||||
@@ -107,6 +108,7 @@ pub(crate) async fn handle_get_game_command(
|
||||
ctx.task_tracker.spawn(fetch_game_details_from_peers(
|
||||
peers,
|
||||
id,
|
||||
expected_version,
|
||||
peer_game_db,
|
||||
tx_notify_ui,
|
||||
|peer_addr, game_id, peer_game_db| async move {
|
||||
@@ -126,10 +128,16 @@ impl GameDetailSource {
|
||||
matches!(self, Self::LocalOrPeers)
|
||||
}
|
||||
|
||||
fn select_peers(self, peer_game_db: &PeerGameDB, id: &str) -> Vec<SocketAddr> {
|
||||
fn select_peers(
|
||||
self,
|
||||
peer_game_db: &PeerGameDB,
|
||||
id: &str,
|
||||
expected_version: Option<&str>,
|
||||
) -> Vec<SocketAddr> {
|
||||
match self {
|
||||
Self::LocalOrPeers => peer_game_db.peers_with_game(id),
|
||||
Self::LatestPeersOnly => peer_game_db.peers_with_latest_version(id),
|
||||
Self::LocalOrPeers | Self::LatestPeersOnly => {
|
||||
peer_game_db.peers_with_expected_version(id, expected_version)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -154,6 +162,7 @@ async fn request_game_details_and_update(
|
||||
async fn fetch_game_details_from_peers<F, Fut>(
|
||||
peers: Vec<SocketAddr>,
|
||||
id: String,
|
||||
expected_version: Option<String>,
|
||||
peer_game_db: Arc<RwLock<PeerGameDB>>,
|
||||
tx_notify_ui: UnboundedSender<PeerEvent>,
|
||||
mut fetch_details: F,
|
||||
@@ -175,7 +184,12 @@ async fn fetch_game_details_from_peers<F, Fut>(
|
||||
}
|
||||
|
||||
if fetched_any {
|
||||
let aggregated_files = { peer_game_db.read().await.aggregated_game_files(&id) };
|
||||
let aggregated_files = {
|
||||
peer_game_db
|
||||
.read()
|
||||
.await
|
||||
.aggregated_game_files(&id, expected_version.as_deref())
|
||||
};
|
||||
|
||||
if let Err(e) = tx_notify_ui.send(PeerEvent::GotGameFiles {
|
||||
id: id.clone(),
|
||||
@@ -210,6 +224,7 @@ pub async fn handle_download_game_files_command(
|
||||
}
|
||||
|
||||
let games_folder = { ctx.game_dir.read().await.clone() };
|
||||
let expected_version = catalog_expected_version(ctx, &id).await;
|
||||
|
||||
// Use majority validation to get trusted file descriptions and peer whitelist
|
||||
let (validated_descriptions, peer_whitelist, file_peer_map) = {
|
||||
@@ -217,7 +232,7 @@ pub async fn handle_download_game_files_command(
|
||||
.peer_game_db
|
||||
.read()
|
||||
.await
|
||||
.validate_file_sizes_majority(&id)
|
||||
.validate_file_sizes_majority(&id, expected_version.as_deref())
|
||||
{
|
||||
Ok((files, peers, file_peer_map)) => {
|
||||
log::info!(
|
||||
@@ -260,7 +275,7 @@ pub async fn handle_download_game_files_command(
|
||||
let local_dl_available = {
|
||||
let active_operations = ctx.active_operations.read().await;
|
||||
let catalog = ctx.catalog.read().await;
|
||||
local_download_available(&games_folder, &id, &active_operations, &catalog).await
|
||||
local_download_matches_catalog(&games_folder, &id, &active_operations, &catalog).await
|
||||
};
|
||||
|
||||
if peer_whitelist.is_empty() {
|
||||
@@ -732,11 +747,41 @@ async fn begin_operation(
|
||||
}
|
||||
};
|
||||
|
||||
if started {
|
||||
events::emit_active_operations(&ctx.active_operations, tx_notify_ui).await;
|
||||
if !started {
|
||||
return false;
|
||||
}
|
||||
|
||||
started
|
||||
events::emit_active_operations(&ctx.active_operations, tx_notify_ui).await;
|
||||
|
||||
if operation == OperationKind::Updating || operation == OperationKind::RemovingDownload {
|
||||
// Cancel all active outbound transfers for this game
|
||||
let mut tokens_to_cancel = Vec::new();
|
||||
{
|
||||
let active = ctx.active_outbound_transfers.read().await;
|
||||
if let Some(transfers) = active.get(id) {
|
||||
for (_, token) in transfers {
|
||||
tokens_to_cancel.push(token.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
for token in tokens_to_cancel {
|
||||
token.cancel();
|
||||
}
|
||||
|
||||
// Wait until active outbound transfers drop to 0
|
||||
loop {
|
||||
let count = {
|
||||
let active = ctx.active_outbound_transfers.read().await;
|
||||
active.get(id).map_or(0, Vec::len)
|
||||
};
|
||||
if count == 0 {
|
||||
break;
|
||||
}
|
||||
tokio::time::sleep(tokio::time::Duration::from_millis(10)).await;
|
||||
}
|
||||
}
|
||||
|
||||
true
|
||||
}
|
||||
|
||||
async fn transition_download_to_install(
|
||||
@@ -818,6 +863,14 @@ async fn catalog_contains(ctx: &Ctx, id: &str) -> bool {
|
||||
ctx.catalog.read().await.contains(id)
|
||||
}
|
||||
|
||||
async fn catalog_expected_version(ctx: &Ctx, id: &str) -> Option<String> {
|
||||
ctx.catalog
|
||||
.read()
|
||||
.await
|
||||
.expected_version(id)
|
||||
.map(ToOwned::to_owned)
|
||||
}
|
||||
|
||||
/// Handles the `SetGameDir` command.
|
||||
pub async fn handle_set_game_dir_command(
|
||||
ctx: &Ctx,
|
||||
@@ -1008,13 +1061,8 @@ async fn update_and_announce_games_with_policy(
|
||||
active_operation_ids.remove(id);
|
||||
}
|
||||
if !active_operation_ids.is_empty() {
|
||||
let previous = ctx.local_library.read().await.games.clone();
|
||||
for id in &active_operation_ids {
|
||||
if let Some(summary) = previous.get(id.as_str()) {
|
||||
summaries.insert(id.clone(), summary.clone());
|
||||
} else {
|
||||
summaries.remove(id);
|
||||
}
|
||||
summaries.remove(id);
|
||||
}
|
||||
game_db = GameDB::from(summaries.values().map(game_from_summary).collect());
|
||||
}
|
||||
@@ -1068,13 +1116,14 @@ async fn update_and_announce_games_with_policy(
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::{
|
||||
collections::HashSet,
|
||||
collections::HashMap,
|
||||
net::SocketAddr,
|
||||
path::{Path, PathBuf},
|
||||
sync::{Arc, Mutex},
|
||||
time::Duration,
|
||||
};
|
||||
|
||||
use lanspread_db::db::GameCatalog;
|
||||
use lanspread_proto::{Availability, GameSummary};
|
||||
use tokio::sync::mpsc;
|
||||
use tokio_util::{sync::CancellationToken, task::TaskTracker};
|
||||
@@ -1115,7 +1164,8 @@ mod tests {
|
||||
Arc::new(FakeUnpacker),
|
||||
CancellationToken::new(),
|
||||
TaskTracker::new(),
|
||||
Arc::new(RwLock::new(HashSet::from(["game".to_string()]))),
|
||||
Arc::new(RwLock::new(GameCatalog::from_ids(["game".to_string()]))),
|
||||
Arc::new(RwLock::new(HashMap::new())),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1220,7 +1270,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn update_source_selects_latest_ready_peer_manifest() {
|
||||
fn update_source_selects_expected_ready_peer_manifest() {
|
||||
let old_addr = addr(12_000);
|
||||
let new_addr = addr(12_001);
|
||||
let local_only_addr = addr(12_002);
|
||||
@@ -1242,13 +1292,13 @@ mod tests {
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
GameDetailSource::LatestPeersOnly.select_peers(&db, "game"),
|
||||
GameDetailSource::LatestPeersOnly.select_peers(&db, "game", Some("20250101")),
|
||||
vec![new_addr]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_fetch_emits_fresh_manifest_from_latest_peer() {
|
||||
async fn update_fetch_emits_fresh_manifest_from_expected_peer() {
|
||||
let old_addr = addr(12_010);
|
||||
let new_addr = addr(12_011);
|
||||
let peer_game_db = Arc::new(RwLock::new(PeerGameDB::new()));
|
||||
@@ -1267,33 +1317,40 @@ mod tests {
|
||||
}
|
||||
let peers = {
|
||||
let db = peer_game_db.read().await;
|
||||
GameDetailSource::LatestPeersOnly.select_peers(&db, "game")
|
||||
GameDetailSource::LatestPeersOnly.select_peers(&db, "game", Some("20250101"))
|
||||
};
|
||||
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| {
|
||||
fetch_game_details_from_peers(
|
||||
peers,
|
||||
"game".to_string(),
|
||||
Some("20250101".to_string()),
|
||||
peer_game_db.clone(),
|
||||
tx,
|
||||
{
|
||||
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)
|
||||
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!(
|
||||
@@ -1314,7 +1371,7 @@ mod tests {
|
||||
file_descriptions
|
||||
.iter()
|
||||
.any(|desc| desc.relative_path == "game/new.eti" && desc.size == 11),
|
||||
"latest peer manifest should be emitted to the download path"
|
||||
"expected-version peer manifest should be emitted to the download path"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1329,6 +1386,7 @@ mod tests {
|
||||
fetch_game_details_from_peers(
|
||||
vec![first_addr, second_addr],
|
||||
"game".to_string(),
|
||||
Some("20250101".to_string()),
|
||||
peer_game_db,
|
||||
tx.clone(),
|
||||
{
|
||||
@@ -1362,7 +1420,7 @@ mod tests {
|
||||
|
||||
#[tokio::test]
|
||||
async fn update_request_skips_local_manifest_even_when_download_exists() {
|
||||
let temp = TempDir::new("lanspread-handler-latest-peer");
|
||||
let temp = TempDir::new("lanspread-handler-expected-peer");
|
||||
let root = temp.game_root();
|
||||
write_file(&root.join("version.ini"), b"20240101");
|
||||
write_file(&root.join("game.eti"), b"old archive");
|
||||
@@ -1385,23 +1443,37 @@ mod tests {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn local_library_scan_freezes_active_game_state() {
|
||||
let temp = TempDir::new("lanspread-handler-active-freeze");
|
||||
async fn local_library_scan_hides_active_game_state() {
|
||||
let temp = TempDir::new("lanspread-handler-active-hide");
|
||||
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());
|
||||
let (tx, mut rx) = mpsc::unbounded_channel();
|
||||
let catalog = ctx.catalog.read().await.clone();
|
||||
|
||||
// 1. Initial scan: the game is ready and announced
|
||||
let scan = scan_local_library(temp.path(), ctx.state_dir.as_ref(), &catalog)
|
||||
.await
|
||||
.expect("scan should succeed");
|
||||
update_and_announce_games(&ctx, &tx, scan).await;
|
||||
|
||||
let PeerEvent::LocalLibraryChanged { games } = recv_event(&mut rx).await else {
|
||||
panic!("expected LocalLibraryChanged");
|
||||
};
|
||||
assert_eq!(games.len(), 1);
|
||||
assert_eq!(games[0].id, "game");
|
||||
|
||||
// 2. Set the game as active/in-progress and scan again
|
||||
ctx.active_operations
|
||||
.write()
|
||||
.await
|
||||
.insert("game".to_string(), OperationKind::Installing);
|
||||
let (tx, mut rx) = mpsc::unbounded_channel();
|
||||
let catalog = ctx.catalog.read().await.clone();
|
||||
|
||||
let scan = scan_local_library(temp.path(), ctx.state_dir.as_ref(), &catalog)
|
||||
.await
|
||||
.expect("scan should succeed");
|
||||
|
||||
update_and_announce_games(&ctx, &tx, scan).await;
|
||||
|
||||
let PeerEvent::LocalLibraryChanged { games } = recv_event(&mut rx).await else {
|
||||
@@ -1409,7 +1481,7 @@ mod tests {
|
||||
};
|
||||
assert!(
|
||||
games.is_empty(),
|
||||
"active game should keep its previous announced state"
|
||||
"active game should be hidden/unannounced during operations"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user