diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightclient.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightclient.rs index 28b84fd..2171807 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightclient.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightclient.rs @@ -1299,10 +1299,18 @@ pub fn start_mempool_monitor(lc: Arc) -> 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 { + 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(); + } + } diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs index 365c2f0..62388f0 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs @@ -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");