Cross-Activity Coordination

This document audits the write paths across the three SQLite databases (library.db, external_metadata.db, user_data.db), classifies which guards each path holds, and lists the conflict pairs that real-world traffic can exercise. Use it to decide whether a new write needs an Activity guard, a storage_generation check, both, or neither.

For background see Activity System, Connection Pooling, and the “Write-isolation rule” in Database Schema.

Coordination primitives#

• Activity mutex — AppState::try_start_activity (replay-control-app/src/api/activity.rs). At most one Activity != Idle at a time. The returned ActivityGuard resets to Idle on drop, so a panic still releases the slot.
• storage_generation: AtomicU64 on AppState. Bumped inside redetect_storage (and the deferred-storage paths in cancel_storage_scans_if_ready). Long scans capture the generation at start, thread it through ScanInputs/ScanCancellation, and call state.ensure_storage_generation(expected) at every system boundary plus before each writer transaction.
• rom_watcher_generation: AtomicU64 on AppState. Bumped by restart_rom_watcher; the watcher loop self-terminates on mismatch. Independent of storage_generation.
• is_idle() gate on AppState. Used by the ROM watcher to suppress its own work during any non-Idle activity.
• identity_can_run() gate on AppState. Identity workers are allowed while Activity::Identity owns the activity slot, but stop when any foreground activity or storage-generation change appears.
• require_configured_storage_ready_for_mutation — refreshes storage, then rejects mutations when the configured target is not Ready. Single-shot, not a serializing mutex.
• Pool drain on reset_to_empty / reopen / replace_with_file — the pool waits for in-flight Objects to release before unlinking files. A stalled closure aborts the destructive op rather than racing.

1. Inventory of write paths#

1.1 library.db writers#

1.2 external_metadata.db writers#

1.3 user_data.db writers#

user_data.db runs in DELETE mode on most storages (exFAT/NFS-friendly). Per-try_write WriteGate activation serializes readers vs the single-slot writer, so concurrency between U1–U4 is harmless serialization at the pool layer.

2. Conflict matrix#

Pairs with non-trivial overlap. “OK” means existing guards prevent the bad outcome; “gap” means it can be observed and there is no compensating mechanism.

3. Findings#

Severity-ordered. F-3 is kept under Resolved findings because it documents a real data-loss class that the region-preference handlers must not reintroduce.

F-1: storage-swap during Rebuild loses the new pipeline#

Severity: medium (silent failure-to-populate).

Sequence on a storage swap while Activity::Rebuild is held:

1. redetect_storage calls bump_storage_generation(). In-flight rebuild scans see Err(StorageChanged) at their next gate and start unwinding.
2. redetect_storage then calls BackgroundManager::spawn_pipeline(self.clone()).
3. spawn_pipeline runs run_pipeline, which calls try_start_activity(Activity::Startup{...}).
4. Race window: the cancelled rebuild task hasn’t dropped its Activity::Rebuild guard yet (still unwinding). try_start_activity returns Err("Another operation is already running"), the new pipeline aborts with a warn! log, and there is no retry.

Result: the new storage’s library.db is not (re)populated until the next reboot or a manual rebuild. No banner, no automatic recovery.

The retry helper exists for the inverse case (claim_startup_activity retries on activity-busy) but is only used by run_pipeline’s top-level Startup→Rebuild sequencing, not by spawn_pipeline on storage swap.

Fix: route spawn_pipeline through the existing claim_startup_activity retry helper so it lands once the rebuild guard drops.

F-2: regenerate_metadata is a non-atomic clear-then-spawn#

Severity: medium (silent metadata wipe with no recovery prompt).

regenerate_metadata clears the LaunchBox tables on the writer pool, then spawns spawn_external_metadata_refresh, which itself tries to claim Activity::RefreshExternalMetadata. If the slot is busy (e.g. a thumbnail update is running, or the user clicked “Update Thumbnails” a second earlier), the spawned refresh logs "phase_auto_import: another refresh in flight" and returns. The launchbox tables remain empty. The user sees blank metadata until they click “Update” again, and the cause isn’t surfaced.

Fix: move the clear_launchbox call inside phase_auto_import_inner (after the guard is claimed), guarded by a force_clear: bool flag that regenerate_metadata passes through. The user-visible operation becomes “claim slot or fail loudly”, and the clear+refresh stays atomic relative to other activities.

F-4: redetect_storage is not single-flight#

Severity: low (UI flicker + duplicated pool warmup).

After the watcher simplification, four sources call reload_config_and_redetect_storage (which calls redetect_storage) without a serializing mutex:

• the notify config-file watcher (debounced)
• mountinfo_watcher (debounced)
• every user mutation entry, via require_configured_storage_ready_for_mutation
• the user-triggered HTTP refresh_storage server fn

redetect_storage reads self.storage under a short read-lock, awaits probe_storage_ready (NFS-bound, can take seconds), then takes the write-lock. Two callers that both observe a real change run the full reopen sequence in sequence, double-warming the pools and double-emitting ConfigEvent::StorageChanged. On a flapping mount the storage status oscillates Activating → Ready → Activating → Ready publicly visible to clients.

(The previous 10 s / 60 s poll was removed; that takes one chronic source off the table but does not close the race between the remaining four.)

Correctness is preserved (each bump_storage_generation invalidates its own predecessor’s scans), but the duplicated pool reopens are observable as longer “Activating” windows on the UI banner, and any background pipeline that wedged into the gap between two reopens is operating against a pool that will be yanked.

Fix: add a tokio::sync::Mutex<()> (e.g. redetect_storage_lock on AppState) and acquire it at the top of redetect_storage. Concurrent callers serialize and the second one re-reads the now-current state, almost always returning Ok(false) immediately. Every existing call site keeps working without changes.

4. Recommended action order#

1. F-1 — silent failure-to-populate after storage swap; small fix (route through existing retry helper).
2. F-2 — silent metadata wipe; small structural change (clear inside guard).
3. F-4 — low-severity polish; add a serializing mutex.

Each is a self-contained patch; none depends on the others.

5. New writes — checklist#

When adding a new write path, decide:

• Does it write library.db? If yes, claim a relevant Activity (Rebuild / Maintenance) before touching the writer pool. Skip only if the write is per-row idempotent (INSERT OR REPLACE) and the column is owned by an L5-class hook.
• Does it run for more than a couple of seconds? If yes, capture storage_generation at start, plumb it through ScanInputs, and check ensure_current() before each try_write(...) boundary.
• Is it user-initiated? Gate with require_configured_storage_ready_for_mutation so it fails loudly when storage is misconfigured, instead of silently writing to fallback storage.
• Does it touch L1 caches? Caches are independent of activity guards by design; invalidate freely.
• Is it a destructive lifecycle op (reset_to_empty, reopen, replace_with_file)? Trust the pool drain; do not invent a parallel mutex.

6. Resolved findings#

F-3: region-preference change could wipe an in-progress Rebuild#

Previous severity: high (silent data loss).

save_region_preference and save_region_preference_secondary used to call state.cache.invalidate(&state.library_writer), which ran LibraryDb::clear_all_game_library. A user changing region while a Rebuild or auto-import re-enrichment pass was mid-flight could truncate rows that the long operation had already written.

The handlers now call only invalidate_l1() plus invalidate_user_caches(), then run resolve_release_date_for_library to rewrite the region-dependent release_date mirror columns. Do not restore a library-wide clear in this path; if a future change needs destructive library work, claim an Activity guard first.