Skip to content

Commit dc06309

Browse files
committed
fix(ffi): Use weak reference for callback registry
Convert CALLBACK_REGISTRY from static Lazy<Arc<...>> to use the same weak reference pattern as RUNTIME_WEAK. This ensures the callback registry is properly cleaned up when the last client is destroyed, avoiding memory leak reports from sanitizers.
1 parent caef0a1 commit dc06309

1 file changed

Lines changed: 43 additions & 14 deletions

File tree

dash-spv-ffi/src/client.rs

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ use dash_spv::Hash;
1212
use dashcore::Txid;
1313

1414
use futures::future::{AbortHandle, Abortable};
15-
use once_cell::sync::Lazy;
1615
use std::collections::HashMap;
1716
use std::ffi::{CStr, CString};
1817
use std::os::raw::{c_char, c_void};
@@ -24,9 +23,30 @@ use tokio::runtime::Runtime;
2423
use tokio::sync::mpsc::{error::TryRecvError, UnboundedReceiver};
2524
use tokio_util::sync::CancellationToken;
2625

27-
/// Global callback registry for thread-safe callback management
28-
static CALLBACK_REGISTRY: Lazy<Arc<Mutex<CallbackRegistry>>> =
29-
Lazy::new(|| Arc::new(Mutex::new(CallbackRegistry::new())));
26+
/// Weak reference to the shared callback registry.
27+
/// The registry is created when the first client is created and dropped when
28+
/// the last client is destroyed. This avoids memory leaks from static Lazy
29+
/// allocations that are never cleaned up.
30+
static CALLBACK_REGISTRY_WEAK: Mutex<Weak<Mutex<CallbackRegistry>>> = Mutex::new(Weak::new());
31+
32+
/// Get or create the shared callback registry.
33+
/// Returns an Arc that keeps the registry alive while clients exist.
34+
/// When all clients are destroyed, the registry is dropped and cleaned up.
35+
fn get_or_create_callback_registry() -> Arc<Mutex<CallbackRegistry>> {
36+
let mut weak = CALLBACK_REGISTRY_WEAK.lock().unwrap();
37+
38+
// Try to upgrade existing weak reference
39+
if let Some(registry) = weak.upgrade() {
40+
return registry;
41+
}
42+
43+
// Create new registry if none exists
44+
let registry = Arc::new(Mutex::new(CallbackRegistry::new()));
45+
46+
// Store weak reference
47+
*weak = Arc::downgrade(&registry);
48+
registry
49+
}
3050

3151
/// Weak reference to the shared tokio runtime.
3252
/// The runtime is created when the first client is created and dropped when
@@ -149,6 +169,7 @@ type SharedClient = Arc<Mutex<Option<InnerClient>>>;
149169
pub struct FFIDashSpvClient {
150170
pub(crate) inner: SharedClient,
151171
pub(crate) runtime: Arc<Runtime>,
172+
callback_registry: Arc<Mutex<CallbackRegistry>>,
152173
event_callbacks: Arc<Mutex<FFIEventCallbacks>>,
153174
active_threads: Arc<Mutex<Vec<std::thread::JoinHandle<()>>>>,
154175
sync_callbacks: Arc<Mutex<Option<SyncCallbackData>>>,
@@ -218,9 +239,13 @@ pub unsafe extern "C" fn dash_spv_ffi_client_new(
218239

219240
match client_result {
220241
Ok(client) => {
242+
// Get or create shared callback registry (cleaned up when last client is destroyed)
243+
let callback_registry = get_or_create_callback_registry();
244+
221245
let ffi_client = FFIDashSpvClient {
222246
inner: Arc::new(Mutex::new(Some(client))),
223247
runtime,
248+
callback_registry,
224249
event_callbacks: Arc::new(Mutex::new(FFIEventCallbacks::default())),
225250
active_threads: Arc::new(Mutex::new(Vec::new())),
226251
sync_callbacks: Arc::new(Mutex::new(None)),
@@ -412,7 +437,7 @@ fn stop_client_internal(client: &mut FFIDashSpvClient) -> Result<(), dash_spv::S
412437
{
413438
let mut cb_guard = client.sync_callbacks.lock().unwrap();
414439
if let Some(ref callback_data) = *cb_guard {
415-
CALLBACK_REGISTRY.lock().unwrap().unregister(callback_data.callback_id);
440+
client.callback_registry.lock().unwrap().unregister(callback_data.callback_id);
416441
}
417442
*cb_guard = None;
418443
}
@@ -594,13 +619,14 @@ pub unsafe extern "C" fn dash_spv_ffi_client_sync_to_tip(
594619
let client = &(*client);
595620
let inner = client.inner.clone();
596621
let runtime = client.runtime.clone();
622+
let callback_registry = client.callback_registry.clone();
597623

598-
// Register callbacks in the global registry for safe lifetime management
624+
// Register callbacks in the registry for safe lifetime management
599625
let callback_info = CallbackInfo::Simple {
600626
completion_callback,
601627
user_data,
602628
};
603-
let callback_id = CALLBACK_REGISTRY.lock().unwrap().register(callback_info);
629+
let callback_id = callback_registry.lock().unwrap().register(callback_info);
604630

605631
// Execute sync in the runtime
606632
let result = runtime.block_on(async {
@@ -621,7 +647,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_sync_to_tip(
621647
// Progress callbacks removed as sync_to_tip doesn't provide real progress updates
622648

623649
// Report completion and unregister callbacks
624-
let mut registry = CALLBACK_REGISTRY.lock().unwrap();
650+
let mut registry = callback_registry.lock().unwrap();
625651
if let Some(CallbackInfo::Simple {
626652
completion_callback: Some(callback),
627653
user_data,
@@ -641,7 +667,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_sync_to_tip(
641667
}
642668
Err(e) => {
643669
// Report error and unregister callbacks
644-
let mut registry = CALLBACK_REGISTRY.lock().unwrap();
670+
let mut registry = callback_registry.lock().unwrap();
645671
if let Some(CallbackInfo::Simple {
646672
completion_callback: Some(callback),
647673
user_data,
@@ -793,14 +819,15 @@ pub unsafe extern "C" fn dash_spv_ffi_client_sync_to_tip_with_progress(
793819
null_check!(client);
794820

795821
let client = &(*client);
822+
let callback_registry = client.callback_registry.clone();
796823

797-
// Register callbacks in the global registry
824+
// Register callbacks in the registry
798825
let callback_info = CallbackInfo::Detailed {
799826
progress_callback,
800827
completion_callback,
801828
user_data,
802829
};
803-
let callback_id = CALLBACK_REGISTRY.lock().unwrap().register(callback_info);
830+
let callback_id = callback_registry.lock().unwrap().register(callback_info);
804831

805832
// Store callback ID in the client
806833
let callback_data = SyncCallbackData {
@@ -824,6 +851,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_sync_to_tip_with_progress(
824851
let runtime_handle = runtime.handle().clone();
825852
let sync_callbacks_clone = sync_callbacks.clone();
826853
let shutdown_token_monitor = client.shutdown_token.clone();
854+
let callback_registry_clone = callback_registry.clone();
827855

828856
let handle = std::thread::spawn(move || {
829857
runtime_handle.block_on(async move {
@@ -846,7 +874,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_sync_to_tip_with_progress(
846874
let cb_guard = sync_callbacks_clone.lock().unwrap();
847875

848876
if let Some(ref callback_data) = *cb_guard {
849-
let registry = CALLBACK_REGISTRY.lock().unwrap();
877+
let registry = callback_registry_clone.lock().unwrap();
850878
if let Some(CallbackInfo::Detailed {
851879
progress_callback: Some(callback),
852880
user_data,
@@ -901,6 +929,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_sync_to_tip_with_progress(
901929
let runtime_handle = runtime.handle().clone();
902930
let sync_callbacks_clone = sync_callbacks.clone();
903931
let shutdown_token_sync = client.shutdown_token.clone();
932+
let callback_registry_sync = callback_registry.clone();
904933
let sync_handle = std::thread::spawn(move || {
905934
let shutdown_token_callback = shutdown_token_sync.clone();
906935
// Run monitoring loop
@@ -949,7 +978,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_sync_to_tip_with_progress(
949978
{
950979
let mut cb_guard = sync_callbacks_clone.lock().unwrap();
951980
if let Some(ref callback_data) = *cb_guard {
952-
let mut registry = CALLBACK_REGISTRY.lock().unwrap();
981+
let mut registry = callback_registry_sync.lock().unwrap();
953982
if let Some(CallbackInfo::Detailed {
954983
completion_callback: Some(callback),
955984
user_data,
@@ -1310,7 +1339,7 @@ pub unsafe extern "C" fn dash_spv_ffi_client_destroy(client: *mut FFIDashSpvClie
13101339

13111340
// Clean up any registered callbacks
13121341
if let Some(ref callback_data) = *client.sync_callbacks.lock().unwrap() {
1313-
CALLBACK_REGISTRY.lock().unwrap().unregister(callback_data.callback_id);
1342+
client.callback_registry.lock().unwrap().unregister(callback_data.callback_id);
13141343
}
13151344

13161345
// Stop the SPV client

0 commit comments

Comments
 (0)