Skip to content

Commit 5c7ade7

Browse files
xdustinfacexdustinface
committed
fix: prevent wallet related memory leaks in tests
This fixes a bunch of memory leaks in tests detected by sanitizer in CI overhaul PR dashpay#201. - Properly keep track of all create wallets and wallet infos and clean them up at the end. - Head allocate wallet infos like it would happen in real FFI usage and free them properly at the end.
1 parent 67b3f96 commit 5c7ade7

File tree

4 files changed

+56
-50
lines changed

4 files changed

+56
-50
lines changed

‎key-wallet-ffi/src/managed_wallet.rs‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -748,14 +748,14 @@ mod tests {
748748
assert!(!wallet.is_null());
749749
assert_eq!(error.code, FFIErrorCode::Success);
750750

751-
// Create managed wallet info from the wallet
751+
// Create managed wallet info from the wallet heap-allocated like C would do
752752
let wallet_rust = unsafe { &(*wallet).wallet };
753753
let managed_info = ManagedWalletInfo::from_wallet(wallet_rust);
754-
let mut ffi_managed = FFIManagedWalletInfo::new(managed_info);
754+
let ffi_managed = Box::into_raw(Box::new(FFIManagedWalletInfo::new(managed_info)));
755755

756756
// Test get_next_receive_address with valid pointers
757757
let receive_addr = unsafe {
758-
managed_wallet_get_next_bip44_receive_address(&mut ffi_managed, wallet, 0, &mut error)
758+
managed_wallet_get_next_bip44_receive_address(ffi_managed, wallet, 0, &mut error)
759759
};
760760

761761
if !receive_addr.is_null() {
@@ -776,7 +776,7 @@ mod tests {
776776

777777
// Test get_next_change_address with valid pointers
778778
let change_addr = unsafe {
779-
managed_wallet_get_next_bip44_change_address(&mut ffi_managed, wallet, 0, &mut error)
779+
managed_wallet_get_next_bip44_change_address(ffi_managed, wallet, 0, &mut error)
780780
};
781781

782782
if !change_addr.is_null() {
@@ -795,6 +795,7 @@ mod tests {
795795

796796
// Clean up
797797
unsafe {
798+
managed_wallet_free(ffi_managed);
798799
wallet::wallet_free(wallet);
799800
}
800801
}
@@ -869,16 +870,16 @@ mod tests {
869870
// Insert the managed account directly into managed_info's accounts
870871
managed_info.accounts.insert(managed_account);
871872

872-
// Create wrapper for managed info
873-
let mut ffi_managed = FFIManagedWalletInfo::new(managed_info);
873+
// Create wrapper for managed info heap-allocated like C would do
874+
let ffi_managed = Box::into_raw(Box::new(FFIManagedWalletInfo::new(managed_info)));
874875

875876
// Use the existing wallet pointer
876877
let ffi_wallet_ptr = wallet_ptr;
877878

878879
// Test 1: Get next receive address
879880
let receive_addr = unsafe {
880881
managed_wallet_get_next_bip44_receive_address(
881-
&mut ffi_managed,
882+
ffi_managed,
882883
ffi_wallet_ptr,
883884
0,
884885
&mut error,
@@ -895,12 +896,7 @@ mod tests {
895896

896897
// Test 2: Get next change address
897898
let change_addr = unsafe {
898-
managed_wallet_get_next_bip44_change_address(
899-
&mut ffi_managed,
900-
ffi_wallet_ptr,
901-
0,
902-
&mut error,
903-
)
899+
managed_wallet_get_next_bip44_change_address(ffi_managed, ffi_wallet_ptr, 0, &mut error)
904900
};
905901

906902
assert!(!change_addr.is_null());
@@ -917,7 +913,7 @@ mod tests {
917913

918914
let success = unsafe {
919915
managed_wallet_get_bip_44_external_address_range(
920-
&mut ffi_managed,
916+
ffi_managed,
921917
ffi_wallet_ptr,
922918
0,
923919
0,
@@ -950,7 +946,7 @@ mod tests {
950946

951947
let success = unsafe {
952948
managed_wallet_get_bip_44_internal_address_range(
953-
&mut ffi_managed,
949+
ffi_managed,
954950
ffi_wallet_ptr,
955951
0,
956952
0,
@@ -980,6 +976,7 @@ mod tests {
980976

981977
// Clean up
982978
unsafe {
979+
managed_wallet_free(ffi_managed);
983980
wallet::wallet_free(wallet_ptr);
984981
}
985982
}

‎key-wallet-ffi/src/utxo_tests.rs‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,18 +113,20 @@ mod utxo_tests {
113113
let mut utxos_out: *mut FFIUTXO = ptr::null_mut();
114114
let mut count_out: usize = 0;
115115

116-
// Create an empty managed wallet info
116+
// Create an empty managed wallet info (heap-allocated like C would do)
117117
let managed_info = ManagedWalletInfo::new(Network::Testnet, [0u8; 32]);
118-
let ffi_managed_info = FFIManagedWalletInfo::new(managed_info);
118+
let ffi_managed_info = Box::into_raw(Box::new(FFIManagedWalletInfo::new(managed_info)));
119119

120120
let result = unsafe {
121-
managed_wallet_get_utxos(&ffi_managed_info, &mut utxos_out, &mut count_out, error)
121+
managed_wallet_get_utxos(&*ffi_managed_info, &mut utxos_out, &mut count_out, error)
122122
};
123123

124124
assert!(result);
125125
assert_eq!(unsafe { (*error).code }, FFIErrorCode::Success);
126126
assert_eq!(count_out, 0);
127127
assert!(utxos_out.is_null());
128+
129+
unsafe { crate::managed_wallet::managed_wallet_free(ffi_managed_info) };
128130
}
129131

130132
// Note: There's no individual utxo_free function, only utxo_array_free
@@ -229,10 +231,10 @@ mod utxo_tests {
229231

230232
managed_info.accounts.insert(bip44_account);
231233

232-
let ffi_managed_info = FFIManagedWalletInfo::new(managed_info);
234+
let ffi_managed_info = Box::into_raw(Box::new(FFIManagedWalletInfo::new(managed_info)));
233235

234236
let result = unsafe {
235-
managed_wallet_get_utxos(&ffi_managed_info, &mut utxos_out, &mut count_out, error)
237+
managed_wallet_get_utxos(&*ffi_managed_info, &mut utxos_out, &mut count_out, error)
236238
};
237239

238240
assert!(result);
@@ -267,6 +269,7 @@ mod utxo_tests {
267269
// Clean up
268270
unsafe {
269271
utxo_array_free(utxos_out, count_out);
272+
crate::managed_wallet::managed_wallet_free(ffi_managed_info);
270273
}
271274
}
272275

@@ -393,10 +396,10 @@ mod utxo_tests {
393396
}
394397
managed_info.accounts.insert(coinjoin_account);
395398

396-
let ffi_managed_info = FFIManagedWalletInfo::new(managed_info);
399+
let ffi_managed_info = Box::into_raw(Box::new(FFIManagedWalletInfo::new(managed_info)));
397400

398401
let result = unsafe {
399-
managed_wallet_get_utxos(&ffi_managed_info, &mut utxos_out, &mut count_out, error)
402+
managed_wallet_get_utxos(&*ffi_managed_info, &mut utxos_out, &mut count_out, error)
400403
};
401404

402405
assert!(result);
@@ -406,6 +409,7 @@ mod utxo_tests {
406409
// Clean up
407410
unsafe {
408411
utxo_array_free(utxos_out, count_out);
412+
crate::managed_wallet::managed_wallet_free(ffi_managed_info);
409413
}
410414
}
411415

@@ -464,16 +468,17 @@ mod utxo_tests {
464468
testnet_account.utxos.insert(outpoint, utxo);
465469
managed_info.accounts.insert(testnet_account);
466470

467-
let ffi_managed_info = FFIManagedWalletInfo::new(managed_info);
471+
let ffi_managed_info = Box::into_raw(Box::new(FFIManagedWalletInfo::new(managed_info)));
468472

469473
// Get UTXOs
470474
let result = unsafe {
471-
managed_wallet_get_utxos(&ffi_managed_info, &mut utxos_out, &mut count_out, error)
475+
managed_wallet_get_utxos(&*ffi_managed_info, &mut utxos_out, &mut count_out, error)
472476
};
473477
assert!(result);
474478
assert_eq!(count_out, 1);
475479
unsafe {
476480
utxo_array_free(utxos_out, count_out);
481+
crate::managed_wallet::managed_wallet_free(ffi_managed_info);
477482
}
478483
}
479484

@@ -573,22 +578,24 @@ mod utxo_tests {
573578
let mut count_out: usize = 0;
574579

575580
let managed_info = ManagedWalletInfo::new(Network::Testnet, [4u8; 32]);
576-
let ffi_managed_info = FFIManagedWalletInfo::new(managed_info);
581+
let ffi_managed_info = Box::into_raw(Box::new(FFIManagedWalletInfo::new(managed_info)));
577582

578583
// Test with null utxos_out
579584
let result = unsafe {
580-
managed_wallet_get_utxos(&ffi_managed_info, ptr::null_mut(), &mut count_out, error)
585+
managed_wallet_get_utxos(&*ffi_managed_info, ptr::null_mut(), &mut count_out, error)
581586
};
582587
assert!(!result);
583588
assert_eq!(unsafe { (*error).code }, FFIErrorCode::InvalidInput);
584589

585590
// Test with null count_out
586591
let mut utxos_out: *mut FFIUTXO = ptr::null_mut();
587592
let result = unsafe {
588-
managed_wallet_get_utxos(&ffi_managed_info, &mut utxos_out, ptr::null_mut(), error)
593+
managed_wallet_get_utxos(&*ffi_managed_info, &mut utxos_out, ptr::null_mut(), error)
589594
};
590595
assert!(!result);
591596
assert_eq!(unsafe { (*error).code }, FFIErrorCode::InvalidInput);
597+
598+
unsafe { crate::managed_wallet::managed_wallet_free(ffi_managed_info) };
592599
}
593600

594601
#[test]

‎key-wallet-ffi/src/wallet_manager_serialization_tests.rs‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,8 @@ mod tests {
212212
let error = &mut error as *mut FFIError;
213213

214214
// Create a wallet manager
215-
let manager = wallet_manager::wallet_manager_create(FFINetwork::Testnet, error);
216-
assert!(!manager.is_null());
215+
let manager1 = wallet_manager::wallet_manager_create(FFINetwork::Testnet, error);
216+
assert!(!manager1.is_null());
217217

218218
let mnemonic = CString::new(TEST_MNEMONIC).unwrap();
219219
let passphrase = CString::new("").unwrap();
@@ -225,7 +225,7 @@ mod tests {
225225
// First create and serialize a wallet
226226
let success = unsafe {
227227
wallet_manager::wallet_manager_add_wallet_from_mnemonic_return_serialized_bytes(
228-
manager,
228+
manager1,
229229
mnemonic.as_ptr(),
230230
passphrase.as_ptr(),
231231
0,
@@ -243,13 +243,13 @@ mod tests {
243243
assert!(!wallet_bytes_out.is_null());
244244

245245
// Now import the wallet into a new manager
246-
let manager = wallet_manager::wallet_manager_create(FFINetwork::Testnet, error);
247-
assert!(!manager.is_null());
246+
let manager2 = wallet_manager::wallet_manager_create(FFINetwork::Testnet, error);
247+
assert!(!manager2.is_null());
248248

249249
let mut imported_wallet_id = [0u8; 32];
250250
let import_success = unsafe {
251251
wallet_manager::wallet_manager_import_wallet_from_bytes(
252-
manager,
252+
manager2,
253253
wallet_bytes_out,
254254
wallet_bytes_len_out,
255255
imported_wallet_id.as_mut_ptr(),
@@ -269,7 +269,8 @@ mod tests {
269269
wallet_bytes_out,
270270
wallet_bytes_len_out,
271271
);
272-
wallet_manager::wallet_manager_free(manager);
272+
wallet_manager::wallet_manager_free(manager1);
273+
wallet_manager::wallet_manager_free(manager2);
273274
}
274275
}
275276

‎key-wallet-ffi/src/wallet_manager_tests.rs‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -812,65 +812,65 @@ mod tests {
812812
let wallet_id_slice = unsafe { slice::from_raw_parts(wallet_ids, 32) };
813813

814814
// Test getting the wallet
815-
let wallet = unsafe {
815+
let valid_wallet = unsafe {
816816
wallet_manager::wallet_manager_get_wallet(manager, wallet_id_slice.as_ptr(), error)
817817
};
818-
assert!(!wallet.is_null());
818+
assert!(!valid_wallet.is_null());
819819
assert_eq!(unsafe { (*error).code }, FFIErrorCode::Success);
820820

821821
// Test getting the managed wallet info
822-
let wallet_info = unsafe {
822+
let valid_wallet_info = unsafe {
823823
wallet_manager::wallet_manager_get_managed_wallet_info(
824824
manager,
825825
wallet_id_slice.as_ptr(),
826826
error,
827827
)
828828
};
829-
assert!(!wallet_info.is_null());
829+
assert!(!valid_wallet_info.is_null());
830830
assert_eq!(unsafe { (*error).code }, FFIErrorCode::Success);
831831

832832
// Test with invalid wallet ID (all zeros)
833833
let invalid_wallet_id = [0u8; 32];
834834

835-
let wallet = unsafe {
835+
let invalid_wallet = unsafe {
836836
wallet_manager::wallet_manager_get_wallet(manager, invalid_wallet_id.as_ptr(), error)
837837
};
838-
assert!(wallet.is_null());
838+
assert!(invalid_wallet.is_null());
839839
assert_eq!(unsafe { (*error).code }, FFIErrorCode::NotFound);
840840

841-
let wallet_info = unsafe {
841+
let invalid_wallet_info = unsafe {
842842
wallet_manager::wallet_manager_get_managed_wallet_info(
843843
manager,
844844
invalid_wallet_id.as_ptr(),
845845
error,
846846
)
847847
};
848-
assert!(wallet_info.is_null());
848+
assert!(invalid_wallet_info.is_null());
849849
assert_eq!(unsafe { (*error).code }, FFIErrorCode::NotFound);
850850

851851
// Test with null manager
852-
let wallet = unsafe {
852+
let null_wallet = unsafe {
853853
wallet_manager::wallet_manager_get_wallet(ptr::null(), wallet_id_slice.as_ptr(), error)
854854
};
855-
assert!(wallet.is_null());
855+
assert!(null_wallet.is_null());
856856
assert_eq!(unsafe { (*error).code }, FFIErrorCode::InvalidInput);
857857

858-
let wallet_info = unsafe {
858+
let null_wallet_info = unsafe {
859859
wallet_manager::wallet_manager_get_managed_wallet_info(
860860
ptr::null(),
861861
wallet_id_slice.as_ptr(),
862862
error,
863863
)
864864
};
865-
assert!(wallet_info.is_null());
865+
assert!(null_wallet_info.is_null());
866866
assert_eq!(unsafe { (*error).code }, FFIErrorCode::InvalidInput);
867867

868868
// Clean up
869869
unsafe {
870-
// Free the wallet (cast from const to mut for free)
871-
wallet::wallet_free(wallet as *mut _);
872-
// Free the managed wallet info
873-
crate::managed_wallet::managed_wallet_info_free(wallet_info);
870+
// Free the valid wallet (cast from const to mut for free)
871+
wallet::wallet_free(valid_wallet as *mut _);
872+
// Free the valid managed wallet info
873+
crate::managed_wallet::managed_wallet_info_free(valid_wallet_info);
874874
// Free the wallet IDs
875875
wallet_manager::wallet_manager_free_wallet_ids(wallet_ids, id_count);
876876
// Free the manager
@@ -1166,6 +1166,7 @@ mod tests {
11661166

11671167
// Clean up
11681168
unsafe {
1169+
wallet::wallet_free(wallet as *mut _);
11691170
crate::wallet_manager::wallet_manager_free_wallet_bytes(
11701171
wallet_bytes_out,
11711172
wallet_bytes_len_out,

0 commit comments

Comments
 (0)