fix(lite-backend): serialize clear and rescan with a running sync
clear_state_from wiped the wallet's blocks, txs and mempool maps without taking sync_lock. The console `clear` and `rescan` commands run concurrently with the refresh worker's sync (litelib_execute drops the client mutex before a command runs), so a clear could land after a sync had already taken its nullifier snapshot. scan_block then unwrapped a note that was no longer in the wallet and panicked under the txs write guard, which poisons the wallet until restart. - clear_state_from now takes sync_lock first (lock order sync_lock -> wallet, as in do_save and do_sync). No caller holds sync_lock, so it cannot self-deadlock; do_rescan holds no lock when it clears. - clear_state reads the birthday into a local before calling clear_state_from. Before, the wallet read guard was a temporary that lived across the call, which read-locks the wallet again: with a writer (a save, an import, a new address) queued in between, the writer-preferring RwLock on Linux and Windows could deadlock. Holding it while waiting for sync_lock would also have inverted the lock order against do_save. - scan_block resolves each spent note with and_then instead of three unwraps. A spend of a note that is no longer in the wallet logs a warning and is skipped. Flagged behaviour changes: - console clear/rescan and the GUI rescan wait for an in-flight sync or save to finish instead of racing it; - the recursive wallet read lock that could deadlock a concurrent save is gone; - a spend of a vanished note now warns and is skipped instead of panicking; it is counted after the next rescan. No ABI, symbol or wallet format change. Tests: clear_state_from waits for sync_lock, clear_state holds no wallet guard while it waits, neither leaks the lock, and a 200x clear/write loop finishes within a 20 s watchdog (a regression guard only). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+93
-1
@@ -1299,10 +1299,18 @@ pub fn start_mempool_monitor(lc: Arc<LightClient>) -> Result<(), String> {
|
||||
}
|
||||
|
||||
pub fn clear_state(&self) {
|
||||
self.clear_state_from(self.wallet.read().unwrap().get_birthday());
|
||||
// Read the birthday into a local first: the wallet read guard must NOT be alive while
|
||||
// clear_state_from waits for sync_lock (lock order is sync_lock -> wallet, as in do_save/do_sync).
|
||||
// Held across the call it was also a recursive read lock, which a queued writer (a save) deadlocks.
|
||||
let birthday = self.wallet.read().unwrap().get_birthday();
|
||||
self.clear_state_from(birthday);
|
||||
}
|
||||
|
||||
pub fn clear_state_from(&self, height: u64) {
|
||||
// Serialize with sync/save/send: a sync that has already snapshotted the wallet's notes would
|
||||
// otherwise find them gone mid-scan. No caller holds sync_lock, and the order is sync_lock -> wallet.
|
||||
let _sync = self.sync_lock.lock().unwrap();
|
||||
|
||||
// First, clear the state from the wallet
|
||||
self.wallet.read().unwrap().clear_blocks();
|
||||
|
||||
@@ -1918,4 +1926,88 @@ pub fn test_bad_import() {
|
||||
assert!(!src.contains(&mempool_needle), "the mempool monitor turns TLS verification off");
|
||||
}
|
||||
|
||||
// U2: clear_state/clear_state_from serialize with a running sync (sync_lock) and hold no wallet guard
|
||||
// while they wait for it (lock order sync_lock -> wallet, as in do_save/do_sync). No test joins a thread
|
||||
// that could be deadlocked: they wait on channels with timeouts.
|
||||
fn shared_client(tmp: &TempDir) -> std::sync::Arc<LightClient> {
|
||||
std::sync::Arc::new(LightClient::new_from_phrase(TEST_SEED.to_string(), &main_config(tmp), 0, 0, false).unwrap())
|
||||
}
|
||||
|
||||
#[test]
|
||||
pub fn test_clear_state_waits_for_sync_lock() {
|
||||
use std::sync::mpsc::{channel, RecvTimeoutError};
|
||||
use std::time::Duration;
|
||||
let tmp = TempDir::new("lctest").unwrap();
|
||||
let lc = shared_client(&tmp);
|
||||
|
||||
let sync = lc.sync_lock.lock().unwrap();
|
||||
let (tx, rx) = channel();
|
||||
let lc2 = lc.clone();
|
||||
let t = std::thread::spawn(move || { lc2.clear_state_from(0); let _ = tx.send(()); });
|
||||
let early = rx.recv_timeout(Duration::from_millis(300));
|
||||
drop(sync);
|
||||
assert_eq!(early, Err(RecvTimeoutError::Timeout), "clear_state_from ran while a sync held sync_lock");
|
||||
assert!(rx.recv_timeout(Duration::from_secs(5)).is_ok(), "clear_state_from did not finish after the sync ended");
|
||||
t.join().unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
pub fn test_clear_state_holds_no_wallet_guard_while_waiting() {
|
||||
use std::sync::mpsc::channel;
|
||||
use std::time::Duration;
|
||||
let tmp = TempDir::new("lctest").unwrap();
|
||||
let lc = shared_client(&tmp);
|
||||
|
||||
let sync = lc.sync_lock.lock().unwrap();
|
||||
let (btx, brx) = channel();
|
||||
let lc_b = lc.clone();
|
||||
let b = std::thread::spawn(move || { lc_b.clear_state(); let _ = btx.send(()); });
|
||||
std::thread::sleep(Duration::from_millis(200));
|
||||
let (wtx, wrx) = channel();
|
||||
let lc_w = lc.clone();
|
||||
let w = std::thread::spawn(move || { let _g = lc_w.wallet.write().unwrap(); let _ = wtx.send(()); });
|
||||
let writer_ok = wrx.recv_timeout(Duration::from_secs(2)).is_ok();
|
||||
let b_early = brx.try_recv().is_ok();
|
||||
drop(sync);
|
||||
assert!(writer_ok, "clear_state held a wallet read guard while it waited for sync_lock (a concurrent save deadlocks)");
|
||||
assert!(!b_early, "clear_state finished while a sync held sync_lock");
|
||||
assert!(brx.recv_timeout(Duration::from_secs(5)).is_ok(), "clear_state did not finish after the sync ended");
|
||||
b.join().unwrap();
|
||||
w.join().unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
pub fn test_clear_state_does_not_leak_sync_lock() {
|
||||
let tmp = TempDir::new("lctest").unwrap();
|
||||
let lc = shared_client(&tmp);
|
||||
lc.clear_state();
|
||||
assert!(lc.sync_lock.try_lock().is_ok(), "clear_state left sync_lock held");
|
||||
lc.clear_state_from(0);
|
||||
assert!(lc.sync_lock.try_lock().is_ok(), "clear_state_from left sync_lock held");
|
||||
}
|
||||
|
||||
// Regression guard only (the race is timing-dependent, so it is no fails-before proof): before the birthday
|
||||
// hoist, clear_state held a wallet read guard across clear_state_from, which read-locks the wallet again; a
|
||||
// writer queued in between deadlocks a writer-preferring RwLock (Linux futex, Windows SRW).
|
||||
#[test]
|
||||
pub fn test_clear_state_no_recursive_read_deadlock() {
|
||||
use std::sync::mpsc::channel;
|
||||
use std::time::{Duration, Instant};
|
||||
let tmp = TempDir::new("lctest").unwrap();
|
||||
let lc = shared_client(&tmp);
|
||||
|
||||
let (tx, rx) = channel();
|
||||
let (lc_r, tx_r) = (lc.clone(), tx.clone());
|
||||
let r = std::thread::spawn(move || { for _ in 0..200 { lc_r.clear_state(); } let _ = tx_r.send(()); });
|
||||
let lc_w = lc.clone();
|
||||
let w = std::thread::spawn(move || { for _ in 0..200 { let _g = lc_w.wallet.write().unwrap(); } let _ = tx.send(()); });
|
||||
let deadline = Instant::now() + Duration::from_secs(20);
|
||||
for _ in 0..2 {
|
||||
let left = deadline.saturating_duration_since(Instant::now());
|
||||
assert!(rx.recv_timeout(left).is_ok(), "clear_state and a wallet writer deadlocked (20 s watchdog)");
|
||||
}
|
||||
r.join().unwrap();
|
||||
w.join().unwrap();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
+13
-10
@@ -2371,18 +2371,21 @@ pub fn scan_full_mempool_tx(&self, tx: &Transaction, height: i32, _datetime: u64
|
||||
//info!("Txid {} belongs to wallet", tx.txid);
|
||||
|
||||
for spend in &tx.shielded_spends {
|
||||
let txid = nfs
|
||||
// The note can be gone if the wallet was cleared or reorged after `nfs` was taken. Never panic
|
||||
// here (it would poison `txs`): skip the spend with a warning; the next rescan counts it.
|
||||
let spent_note = match nfs
|
||||
.iter()
|
||||
.find(|(nf, _, _)| &nf[..] == &spend.nf[..])
|
||||
.unwrap()
|
||||
.2;
|
||||
let mut spent_note = txs
|
||||
.get_mut(&txid)
|
||||
.unwrap()
|
||||
.notes
|
||||
.iter_mut()
|
||||
.find(|nd| &nd.nullifier[..] == &spend.nf[..])
|
||||
.unwrap();
|
||||
.and_then(|(_, _, txid)| txs.get_mut(txid))
|
||||
.and_then(|wtx| wtx.notes.iter_mut().find(|nd| &nd.nullifier[..] == &spend.nf[..]))
|
||||
{
|
||||
Some(nd) => nd,
|
||||
None => {
|
||||
warn!("Block {}: tx {} spends a note that is no longer in the wallet; skipping it (a rescan counts it)",
|
||||
block_data.height, tx.txid);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
// Mark the note as spent, and remove the unconfirmed part of it
|
||||
info!("Marked a note as spent");
|
||||
|
||||
Reference in New Issue
Block a user