tiamak/salvium-rs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add handle lifecycle safety, wallet_close and daemon_close now wait for active sync to finish before dropping, preventing use-after-free on wallet switch

Browse Source
This commit is contained in:
Matt Hess
2026-02-26 15:46:25 +00:00
parent e177fec00e
commit 5db3afca48
3 changed files with 122 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/salvium-ffi/src/daemon.rs
+57 -14
View File
@@ -1,6 +1,7 @@
//! Daemon RPC handle and query functions.
use std::ffi::{c_char, c_void};
use std::sync::atomic::{AtomicUsize, Ordering};
use crate::error::{ffi_try_ptr, ffi_try_string};
use crate::handles::{borrow_handle, drop_handle};
@@ -8,6 +9,35 @@ use crate::strings::c_str_to_str;
use salvium_rpc::DaemonRpc;
/// Wrapper that pairs a DaemonRpc with a usage counter.
///
/// `in_use` tracks how many long-running operations (sync) currently hold a
/// reference. `salvium_daemon_close` waits for `in_use == 0` before dropping,
/// preventing use-after-free when the app closes the daemon while a sync is
/// still running on another thread.
pub(crate) struct DaemonHandle {
pub daemon: DaemonRpc,
pub in_use: AtomicUsize,
}
impl DaemonHandle {
fn new(daemon: DaemonRpc) -> Self {
Self {
daemon,
in_use: AtomicUsize::new(0),
}
}
}
/// RAII guard that decrements `in_use` on drop.
pub(crate) struct DaemonUseGuard<'a>(pub &'a AtomicUsize);
impl Drop for DaemonUseGuard<'_> {
fn drop(&mut self) {
self.0.fetch_sub(1, Ordering::Release);
}
}
/// Connect to a daemon RPC endpoint.
///
/// - `url`: null-terminated URL (e.g. "http://127.0.0.1:19081")
@@ -17,14 +47,27 @@ use salvium_rpc::DaemonRpc;
pub unsafe extern "C" fn salvium_daemon_connect(url: *const c_char) -> *mut c_void {
ffi_try_ptr(|| {
let url_str = unsafe { c_str_to_str(url) }?;
Ok(DaemonRpc::new(url_str))
Ok(DaemonHandle::new(DaemonRpc::new(url_str)))
})
}
/// Close a daemon handle.
///
/// If the daemon is in use by a sync operation, this blocks until the sync
/// finishes before dropping. Safe to call from any thread.
#[no_mangle]
pub unsafe extern "C" fn salvium_daemon_close(handle: *mut c_void) {
drop_handle::<DaemonRpc>(handle);
if handle.is_null() {
return;
}
let dh = unsafe { &*(handle as *const DaemonHandle) };
// Wait for outstanding users (sync) to finish.
while dh.in_use.load(Ordering::Acquire) > 0 {
std::thread::sleep(std::time::Duration::from_millis(50));
}
drop_handle::<DaemonHandle>(handle);
}
/// Get daemon info as JSON.
@@ -34,9 +77,9 @@ pub unsafe extern "C" fn salvium_daemon_close(handle: *mut c_void) {
#[no_mangle]
pub unsafe extern "C" fn salvium_daemon_get_info(handle: *mut c_void) -> *mut c_char {
ffi_try_string(|| {
let daemon = unsafe { borrow_handle::<DaemonRpc>(handle) }?;
let dh = unsafe { borrow_handle::<DaemonHandle>(handle) }?;
let rt = crate::runtime();
let info = rt.block_on(daemon.get_info()).map_err(|e| e.to_string())?;
let info = rt.block_on(dh.daemon.get_info()).map_err(|e| e.to_string())?;
serde_json::to_string(&info).map_err(|e| e.to_string())
})
}
@@ -47,9 +90,9 @@ pub unsafe extern "C" fn salvium_daemon_get_info(handle: *mut c_void) -> *mut c_
#[no_mangle]
pub unsafe extern "C" fn salvium_daemon_get_height(handle: *mut c_void) -> u64 {
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
let daemon = unsafe { borrow_handle::<DaemonRpc>(handle) }.ok()?;
let dh = unsafe { borrow_handle::<DaemonHandle>(handle) }.ok()?;
let rt = crate::runtime();
rt.block_on(daemon.get_height()).ok()
rt.block_on(dh.daemon.get_height()).ok()
}));
match result {
Ok(Some(h)) => h,
@@ -63,9 +106,9 @@ pub unsafe extern "C" fn salvium_daemon_get_height(handle: *mut c_void) -> u64 {
#[no_mangle]
pub unsafe extern "C" fn salvium_daemon_is_synchronized(handle: *mut c_void) -> i32 {
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
let daemon = unsafe { borrow_handle::<DaemonRpc>(handle) }.ok()?;
let dh = unsafe { borrow_handle::<DaemonHandle>(handle) }.ok()?;
let rt = crate::runtime();
rt.block_on(daemon.is_synchronized()).ok()
rt.block_on(dh.daemon.is_synchronized()).ok()
}));
match result {
Ok(Some(true)) => 1,
@@ -81,10 +124,10 @@ pub unsafe extern "C" fn salvium_daemon_is_synchronized(handle: *mut c_void) ->
#[no_mangle]
pub unsafe extern "C" fn salvium_daemon_get_fee_estimate(handle: *mut c_void) -> *mut c_char {
ffi_try_string(|| {
let daemon = unsafe { borrow_handle::<DaemonRpc>(handle) }?;
let dh = unsafe { borrow_handle::<DaemonHandle>(handle) }?;
let rt = crate::runtime();
let fee = rt
.block_on(daemon.get_fee_estimate(0))
.block_on(dh.daemon.get_fee_estimate(0))
.map_err(|e| e.to_string())?;
serde_json::to_string(&fee).map_err(|e| e.to_string())
})
@@ -96,10 +139,10 @@ pub unsafe extern "C" fn salvium_daemon_get_fee_estimate(handle: *mut c_void) ->
#[no_mangle]
pub unsafe extern "C" fn salvium_daemon_get_supply_info(handle: *mut c_void) -> *mut c_char {
ffi_try_string(|| {
let daemon = unsafe { borrow_handle::<DaemonRpc>(handle) }?;
let dh = unsafe { borrow_handle::<DaemonHandle>(handle) }?;
let rt = crate::runtime();
let info = rt
.block_on(daemon.get_supply_info())
.block_on(dh.daemon.get_supply_info())
.map_err(|e| e.to_string())?;
serde_json::to_string(&info).map_err(|e| e.to_string())
})
@@ -111,10 +154,10 @@ pub unsafe extern "C" fn salvium_daemon_get_supply_info(handle: *mut c_void) ->
#[no_mangle]
pub unsafe extern "C" fn salvium_daemon_get_yield_info(handle: *mut c_void) -> *mut c_char {
ffi_try_string(|| {
let daemon = unsafe { borrow_handle::<DaemonRpc>(handle) }?;
let dh = unsafe { borrow_handle::<DaemonHandle>(handle) }?;
let rt = crate::runtime();
let info = rt
.block_on(daemon.get_yield_info())
.block_on(dh.daemon.get_yield_info())
.map_err(|e| e.to_string())?;
serde_json::to_string(&info).map_err(|e| e.to_string())
})
crates/salvium-ffi/src/transfer.rs
+16 -16
View File
@@ -114,8 +114,8 @@ pub unsafe extern "C" fn salvium_wallet_transfer(
params_json: *const c_char,
) -> *mut c_char {
ffi_try_string(|| {
let wallet_ref = unsafe { borrow_handle::<Wallet>(wallet) }?;
let daemon_ref = unsafe { borrow_handle::<DaemonRpc>(daemon) }?;
let wh = unsafe { borrow_handle::<crate::wallet::WalletHandle>(wallet) }?;
let dh = unsafe { borrow_handle::<crate::daemon::DaemonHandle>(daemon) }?;
let json_str = unsafe { c_str_to_str(params_json) }?;
let params: TransferParams =
@@ -125,14 +125,14 @@ pub unsafe extern "C" fn salvium_wallet_transfer(
return Err("no destinations specified".into());
}
if !wallet_ref.can_spend() {
if !wh.wallet.can_spend() {
return Err("wallet is view-only, cannot sign transactions".into());
}
let priority = parse_priority(&params.priority);
let rt = crate::runtime();
rt.block_on(async { do_transfer(wallet_ref, daemon_ref, &params, priority).await })
rt.block_on(async { do_transfer(&wh.wallet, &dh.daemon, &params, priority).await })
})
}
@@ -158,21 +158,21 @@ pub unsafe extern "C" fn salvium_wallet_stake(
params_json: *const c_char,
) -> *mut c_char {
ffi_try_string(|| {
let wallet_ref = unsafe { borrow_handle::<Wallet>(wallet) }?;
let daemon_ref = unsafe { borrow_handle::<DaemonRpc>(daemon) }?;
let wh = unsafe { borrow_handle::<crate::wallet::WalletHandle>(wallet) }?;
let dh = unsafe { borrow_handle::<crate::daemon::DaemonHandle>(daemon) }?;
let json_str = unsafe { c_str_to_str(params_json) }?;
let params: StakeParams =
serde_json::from_str(json_str).map_err(|e| format!("invalid stake params: {e}"))?;
if !wallet_ref.can_spend() {
if !wh.wallet.can_spend() {
return Err("wallet is view-only, cannot sign transactions".into());
}
let priority = parse_priority(&params.priority);
let rt = crate::runtime();
rt.block_on(async { do_stake(wallet_ref, daemon_ref, &params, priority).await })
rt.block_on(async { do_stake(&wh.wallet, &dh.daemon, &params, priority).await })
})
}
@@ -200,21 +200,21 @@ pub unsafe extern "C" fn salvium_wallet_sweep(
params_json: *const c_char,
) -> *mut c_char {
ffi_try_string(|| {
let wallet_ref = unsafe { borrow_handle::<Wallet>(wallet) }?;
let daemon_ref = unsafe { borrow_handle::<DaemonRpc>(daemon) }?;
let wh = unsafe { borrow_handle::<crate::wallet::WalletHandle>(wallet) }?;
let dh = unsafe { borrow_handle::<crate::daemon::DaemonHandle>(daemon) }?;
let json_str = unsafe { c_str_to_str(params_json) }?;
let params: SweepParams =
serde_json::from_str(json_str).map_err(|e| format!("invalid sweep params: {e}"))?;
if !wallet_ref.can_spend() {
if !wh.wallet.can_spend() {
return Err("wallet is view-only, cannot sign transactions".into());
}
let priority = parse_priority(&params.priority);
let rt = crate::runtime();
rt.block_on(async { do_sweep(wallet_ref, daemon_ref, &params, priority).await })
rt.block_on(async { do_sweep(&wh.wallet, &dh.daemon, &params, priority).await })
})
}
@@ -233,8 +233,8 @@ pub unsafe extern "C" fn salvium_wallet_transfer_dry_run(
params_json: *const c_char,
) -> *mut c_char {
ffi_try_string(|| {
let wallet_ref = unsafe { borrow_handle::<Wallet>(wallet) }?;
let daemon_ref = unsafe { borrow_handle::<DaemonRpc>(daemon) }?;
let wh = unsafe { borrow_handle::<crate::wallet::WalletHandle>(wallet) }?;
let dh = unsafe { borrow_handle::<crate::daemon::DaemonHandle>(daemon) }?;
let json_str = unsafe { c_str_to_str(params_json) }?;
let mut params: TransferParams =
@@ -244,14 +244,14 @@ pub unsafe extern "C" fn salvium_wallet_transfer_dry_run(
if params.destinations.is_empty() {
return Err("no destinations specified".into());
}
if !wallet_ref.can_spend() {
if !wh.wallet.can_spend() {
return Err("wallet is view-only, cannot sign transactions".into());
}
let priority = parse_priority(&params.priority);
let rt = crate::runtime();
rt.block_on(async { do_transfer(wallet_ref, daemon_ref, &params, priority).await })
rt.block_on(async { do_transfer(&wh.wallet, &dh.daemon, &params, priority).await })
})
}
crates/salvium-ffi/src/wallet.rs
+49 -4
View File
@@ -10,10 +10,18 @@ use crate::strings::c_str_to_str;
use salvium_wallet::Wallet;
/// Wrapper that pairs a Wallet with its cancellation flag.
/// Wrapper that pairs a Wallet with its cancellation and lifecycle flags.
///
/// `sync_running` is set to `true` while `salvium_wallet_sync` is executing
/// and back to `false` when it returns (even on error / cancel).
/// `salvium_wallet_close` checks this flag and, if sync is active, sets
/// `sync_cancel` and spins until sync finishes before dropping the handle.
/// This prevents use-after-free when the app closes a wallet while sync is
/// still running on another thread.
pub(crate) struct WalletHandle {
pub wallet: Wallet,
pub sync_cancel: Arc<AtomicBool>,
pub sync_running: AtomicBool,
}
impl WalletHandle {
@@ -21,10 +29,21 @@ impl WalletHandle {
Self {
wallet,
sync_cancel: Arc::new(AtomicBool::new(false)),
sync_running: AtomicBool::new(false),
}
}
}
/// RAII guard that sets `sync_running` to `false` on drop, ensuring
/// the flag is cleared even if sync panics or returns early.
struct SyncRunningGuard<'a>(&'a AtomicBool);
impl Drop for SyncRunningGuard<'_> {
fn drop(&mut self) {
self.0.store(false, Ordering::Release);
}
}
// =============================================================================
// Wallet Lifecycle
// =============================================================================
@@ -107,8 +126,26 @@ pub unsafe extern "C" fn salvium_wallet_open(
}
/// Close a wallet handle, releasing all resources.
///
/// If a sync is in progress, this cancels it and blocks until the sync
/// loop has exited before dropping the handle. Safe to call from any thread.
#[no_mangle]
pub unsafe extern "C" fn salvium_wallet_close(handle: *mut c_void) {
if handle.is_null() {
return;
}
// Safety: pointer is non-null and was created by into_handle<WalletHandle>.
// We read the atomic flags through a shared ref before drop_handle takes ownership.
let wh = unsafe { &*(handle as *const WalletHandle) };
// Signal any running sync to stop.
wh.sync_cancel.store(true, Ordering::Relaxed);
// Wait for sync to finish before dropping.
while wh.sync_running.load(Ordering::Acquire) {
std::thread::sleep(std::time::Duration::from_millis(50));
}
drop_handle::<WalletHandle>(handle);
}
@@ -354,12 +391,20 @@ pub unsafe extern "C" fn salvium_wallet_sync(
) -> i32 {
ffi_try(|| {
let handle = unsafe { borrow_handle_mut::<WalletHandle>(wallet) }?;
let daemon_ref = unsafe { borrow_handle::<salvium_rpc::DaemonRpc>(daemon) }?;
let dh = unsafe { borrow_handle::<crate::daemon::DaemonHandle>(daemon) }?;
let rt = crate::runtime();
// Reset cancel flag before starting.
handle.sync_cancel.store(false, Ordering::Relaxed);
// Mark sync as active; the guard clears this on drop (even on panic/early return).
handle.sync_running.store(true, Ordering::Release);
let _guard = SyncRunningGuard(&handle.sync_running);
// Register daemon usage so daemon_close waits for us.
dh.in_use.fetch_add(1, Ordering::Release);
let _daemon_guard = crate::daemon::DaemonUseGuard(&dh.in_use);
rt.block_on(async {
if let Some(cb) = callback {
let (tx, mut rx) = tokio::sync::mpsc::channel::<salvium_wallet::SyncEvent>(256);
@@ -373,7 +418,7 @@ pub unsafe extern "C" fn salvium_wallet_sync(
let result = handle
.wallet
.sync(daemon_ref, Some(&tx), &handle.sync_cancel)
.sync(&dh.daemon, Some(&tx), &handle.sync_cancel)
.await;
drop(tx); // Close channel so forwarder exits.
let _ = forwarder.await;
@@ -382,7 +427,7 @@ pub unsafe extern "C" fn salvium_wallet_sync(
} else {
handle
.wallet
.sync(daemon_ref, None, &handle.sync_cancel)
.sync(&dh.daemon, None, &handle.sync_cancel)
.await
.map(|_| ())
.map_err(|e| e.to_string())