Skip to content

Commit 363cfe7

Browse files
authored
fix: prevent memory leaks in FFI tests (#323)
* Fix `FFIArray` test cleanup memory leaks The tests were wrapping `FFIArray` in a `Box` before passing to `dash_spv_ffi_array_destroy`, but that function only frees the internal data buffer, not the struct itself. This caused the `Box` allocations to leak. * Free sync progress and stats in `test_client_resource_cleanup` * Prevent wallet related memory leaks in tests - 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. * Prevent `FFIError` message leaks in tests - Modified `set_error`, `set_success`, and `ffi_error_set` macro to free previous error messages before setting new ones - Added `FFIError::free_message` helper for test cleanup - Added cleanup in all relevalt tests
1 parent 90a081d commit 363cfe7

23 files changed

+409
-188
lines changed

dash-spv-ffi/tests/test_types.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ mod tests {
4343
fn test_ffi_array_new_and_destroy() {
4444
let test_data = vec![1u32, 2, 3, 4, 5];
4545
let len = test_data.len();
46-
let array = FFIArray::new(test_data);
46+
let mut array = FFIArray::new(test_data);
4747

4848
assert!(!array.data.is_null());
4949
assert_eq!(array.len, len);
@@ -54,26 +54,22 @@ mod tests {
5454
assert_eq!(slice.len(), len);
5555
assert_eq!(slice, &[1, 2, 3, 4, 5]);
5656

57-
// Allocate on heap for proper FFI destroy
58-
let array_ptr = Box::into_raw(Box::new(array));
59-
dash_spv_ffi_array_destroy(array_ptr);
57+
dash_spv_ffi_array_destroy(&mut array as *mut FFIArray);
6058
}
6159
}
6260

6361
#[test]
6462
fn test_ffi_array_empty() {
6563
let empty_vec: Vec<u8> = vec![];
66-
let array = FFIArray::new(empty_vec);
64+
let mut array = FFIArray::new(empty_vec);
6765

6866
assert_eq!(array.len, 0);
6967

7068
unsafe {
7169
let slice = array.as_slice::<u8>();
7270
assert_eq!(slice.len(), 0);
7371

74-
// Allocate on heap for proper FFI destroy
75-
let array_ptr = Box::into_raw(Box::new(array));
76-
dash_spv_ffi_array_destroy(array_ptr);
72+
dash_spv_ffi_array_destroy(&mut array as *mut FFIArray);
7773
}
7874
}
7975

dash-spv-ffi/tests/unit/test_client_lifecycle.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,11 @@ mod tests {
142142
assert!(!client.is_null());
143143

144144
// Do some operations
145-
let _ = dash_spv_ffi_client_get_sync_progress(client);
146-
let _ = dash_spv_ffi_client_get_stats(client);
145+
let progress = dash_spv_ffi_client_get_sync_progress(client);
146+
let stats = dash_spv_ffi_client_get_stats(client);
147+
148+
dash_spv_ffi_sync_progress_destroy(progress);
149+
dash_spv_ffi_spv_stats_destroy(stats);
147150

148151
dash_spv_ffi_client_destroy(client);
149152
dash_spv_ffi_config_destroy(config);

dash-spv-ffi/tests/unit/test_memory_management.rs

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -45,25 +45,25 @@ mod tests {
4545
unsafe {
4646
// Test with different types and sizes
4747
let small_array: Vec<u32> = vec![1, 2, 3, 4, 5];
48-
let small_ffi = FFIArray::new(small_array);
48+
let mut small_ffi = FFIArray::new(small_array);
4949
assert!(!small_ffi.data.is_null());
5050
assert_eq!(small_ffi.len, 5);
51-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(small_ffi)));
51+
dash_spv_ffi_array_destroy(&mut small_ffi as *mut FFIArray);
5252

5353
// Test with large array
5454
let large_array: Vec<u64> = (0..100_000).collect();
55-
let large_ffi = FFIArray::new(large_array);
55+
let mut large_ffi = FFIArray::new(large_array);
5656
assert!(!large_ffi.data.is_null());
5757
assert_eq!(large_ffi.len, 100_000);
58-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(large_ffi)));
58+
dash_spv_ffi_array_destroy(&mut large_ffi as *mut FFIArray);
5959

6060
// Test with empty array
6161
let empty_array: Vec<u8> = vec![];
62-
let empty_ffi = FFIArray::new(empty_array);
62+
let mut empty_ffi = FFIArray::new(empty_array);
6363
// Even empty arrays have valid pointers
6464
assert!(!empty_ffi.data.is_null());
6565
assert_eq!(empty_ffi.len, 0);
66-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(empty_ffi)));
66+
dash_spv_ffi_array_destroy(&mut empty_ffi as *mut FFIArray);
6767
}
6868
}
6969

@@ -125,12 +125,12 @@ mod tests {
125125
// Each thread creates and destroys arrays
126126
for j in 0..50 {
127127
let array: Vec<u32> = (0..j * 10).collect();
128-
let ffi_array = FFIArray::new(array);
128+
let mut ffi_array = FFIArray::new(array);
129129

130130
// Simulate some work
131131
thread::sleep(Duration::from_micros(10));
132132

133-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(ffi_array)));
133+
dash_spv_ffi_array_destroy(&mut ffi_array as *mut FFIArray);
134134
}
135135
}
136136
});
@@ -163,11 +163,11 @@ mod tests {
163163

164164
// Array allocation
165165
let large_array: Vec<u8> = vec![0xFF; size];
166-
let ffi_array = FFIArray::new(large_array);
166+
let mut ffi_array = FFIArray::new(large_array);
167167
assert!(!ffi_array.data.is_null());
168168
assert_eq!(ffi_array.len, size);
169169

170-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(ffi_array)));
170+
dash_spv_ffi_array_destroy(&mut ffi_array as *mut FFIArray);
171171
}
172172
}
173173
}
@@ -192,18 +192,18 @@ mod tests {
192192
dash_spv_ffi_string_destroy(null_string);
193193

194194
// Test with array
195-
let ffi_array = FFIArray::new(vec![1u32, 2, 3]);
196-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(ffi_array)));
195+
let mut ffi_array = FFIArray::new(vec![1u32, 2, 3]);
196+
dash_spv_ffi_array_destroy(&mut ffi_array as *mut FFIArray);
197197

198198
// Destroying with null should be safe
199-
let null_array = FFIArray {
199+
let mut null_array = FFIArray {
200200
data: std::ptr::null_mut(),
201201
len: 0,
202202
capacity: 0,
203203
elem_size: 0,
204204
elem_align: 1,
205205
};
206-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(null_array)));
206+
dash_spv_ffi_array_destroy(&mut null_array as *mut FFIArray);
207207
}
208208
}
209209

@@ -215,21 +215,21 @@ mod tests {
215215

216216
// u8 - 1 byte alignment
217217
let u8_array = vec![1u8, 2, 3, 4];
218-
let u8_ffi = FFIArray::new(u8_array);
218+
let mut u8_ffi = FFIArray::new(u8_array);
219219
assert_eq!(u8_ffi.data as usize % std::mem::align_of::<u8>(), 0);
220-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(u8_ffi)));
220+
dash_spv_ffi_array_destroy(&mut u8_ffi as *mut FFIArray);
221221

222222
// u32 - 4 byte alignment
223223
let u32_array = vec![1u32, 2, 3, 4];
224-
let u32_ffi = FFIArray::new(u32_array);
224+
let mut u32_ffi = FFIArray::new(u32_array);
225225
assert_eq!(u32_ffi.data as usize % std::mem::align_of::<u32>(), 0);
226-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(u32_ffi)));
226+
dash_spv_ffi_array_destroy(&mut u32_ffi as *mut FFIArray);
227227

228228
// u64 - 8 byte alignment
229229
let u64_array = vec![1u64, 2, 3, 4];
230-
let u64_ffi = FFIArray::new(u64_array);
230+
let mut u64_ffi = FFIArray::new(u64_array);
231231
assert_eq!(u64_ffi.data as usize % std::mem::align_of::<u64>(), 0);
232-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(u64_ffi)));
232+
dash_spv_ffi_array_destroy(&mut u64_ffi as *mut FFIArray);
233233
}
234234
}
235235

@@ -341,10 +341,10 @@ mod tests {
341341

342342
// Empty array
343343
let empty_vec: Vec<u8> = vec![];
344-
let empty_array = FFIArray::new(empty_vec);
344+
let mut empty_array = FFIArray::new(empty_vec);
345345
assert!(!empty_array.data.is_null());
346346
assert_eq!(empty_array.len, 0);
347-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(empty_array)));
347+
dash_spv_ffi_array_destroy(&mut empty_array as *mut FFIArray);
348348
}
349349
}
350350

@@ -407,8 +407,8 @@ mod tests {
407407
dash_spv_ffi_string_destroy(s);
408408
}
409409

410-
for a in arrays {
411-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(a)));
410+
for mut a in arrays {
411+
dash_spv_ffi_array_destroy(&mut a as *mut FFIArray);
412412
}
413413

414414
cycle += 1;
@@ -424,7 +424,7 @@ mod tests {
424424
// Test that memory allocated in one thread can be safely used in another
425425
unsafe {
426426
let string = FFIString::new("Allocated in thread 1");
427-
let array = FFIArray::new(vec![1u32, 2, 3, 4, 5]);
427+
let mut array = FFIArray::new(vec![1u32, 2, 3, 4, 5]);
428428

429429
// Verify we can read the data
430430
let s = FFIString::from_ptr(string.ptr).unwrap();
@@ -435,7 +435,7 @@ mod tests {
435435

436436
// Clean up
437437
dash_spv_ffi_string_destroy(string);
438-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(array)));
438+
dash_spv_ffi_array_destroy(&mut array as *mut FFIArray);
439439
}
440440
}
441441
}

dash-spv-ffi/tests/unit/test_type_conversions.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -61,60 +61,60 @@ mod tests {
6161
fn test_ffi_array_different_sizes() {
6262
// Test empty array
6363
let empty: Vec<u32> = vec![];
64-
let empty_array = FFIArray::new(empty);
64+
let mut empty_array = FFIArray::new(empty);
6565
assert_eq!(empty_array.len, 0);
6666
assert!(!empty_array.data.is_null()); // Even empty vec has allocated pointer
6767
unsafe {
6868
let slice = empty_array.as_slice::<u32>();
6969
assert_eq!(slice.len(), 0);
70-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(empty_array)));
70+
dash_spv_ffi_array_destroy(&mut empty_array as *mut FFIArray);
7171
}
7272

7373
// Test single element
7474
let single = vec![42u32];
75-
let single_array = FFIArray::new(single);
75+
let mut single_array = FFIArray::new(single);
7676
assert_eq!(single_array.len, 1);
7777
unsafe {
7878
let slice = single_array.as_slice::<u32>();
7979
assert_eq!(slice.len(), 1);
8080
assert_eq!(slice[0], 42);
81-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(single_array)));
81+
dash_spv_ffi_array_destroy(&mut single_array as *mut FFIArray);
8282
}
8383

8484
// Test large array
8585
let large: Vec<u32> = (0..10000).collect();
86-
let large_array = FFIArray::new(large.clone());
86+
let mut large_array = FFIArray::new(large.clone());
8787
assert_eq!(large_array.len, 10000);
8888
unsafe {
8989
let slice = large_array.as_slice::<u32>();
9090
assert_eq!(slice.len(), 10000);
9191
for (i, &val) in slice.iter().enumerate() {
9292
assert_eq!(val, i as u32);
9393
}
94-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(large_array)));
94+
dash_spv_ffi_array_destroy(&mut large_array as *mut FFIArray);
9595
}
9696
}
9797

9898
#[test]
9999
fn test_ffi_array_memory_alignment() {
100100
// Test with u8
101101
let bytes: Vec<u8> = vec![1, 2, 3, 4];
102-
let byte_array = FFIArray::new(bytes);
102+
let mut byte_array = FFIArray::new(bytes);
103103
unsafe {
104104
let slice = byte_array.as_slice::<u8>();
105105
assert_eq!(slice, &[1, 2, 3, 4]);
106-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(byte_array)));
106+
dash_spv_ffi_array_destroy(&mut byte_array as *mut FFIArray);
107107
}
108108

109109
// Test with u64 (requires 8-byte alignment)
110110
let longs: Vec<u64> = vec![u64::MAX, 0, 42];
111-
let long_array = FFIArray::new(longs);
111+
let mut long_array = FFIArray::new(longs);
112112
unsafe {
113113
let slice = long_array.as_slice::<u64>();
114114
assert_eq!(slice[0], u64::MAX);
115115
assert_eq!(slice[1], 0);
116116
assert_eq!(slice[2], 42);
117-
dash_spv_ffi_array_destroy(Box::into_raw(Box::new(long_array)));
117+
dash_spv_ffi_array_destroy(&mut long_array as *mut FFIArray);
118118
}
119119
}
120120

key-wallet-ffi/src/account_derivation_tests.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ mod tests {
5959
extended_private_key_free(master_xpriv);
6060
account_free(account);
6161
wallet::wallet_free(wallet);
62+
error.free_message();
6263
}
6364
}
6465

@@ -97,6 +98,8 @@ mod tests {
9798
);
9899
assert_eq!(error.code, FFIErrorCode::InvalidInput);
99100
}
101+
102+
unsafe { error.free_message() };
100103
}
101104

102105
#[test]
@@ -134,6 +137,7 @@ mod tests {
134137
extended_private_key_free(master_xpriv);
135138
account_free(account);
136139
wallet::wallet_free(wallet);
140+
error.free_message();
137141
}
138142
}
139143

@@ -213,6 +217,7 @@ mod tests {
213217
unsafe {
214218
account_free(account);
215219
wallet::wallet_free(wallet);
220+
error.free_message();
216221
}
217222
}
218223

key-wallet-ffi/src/account_tests.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ mod tests {
7777

7878
assert_eq!(count, 0);
7979
assert_eq!(error.code, FFIErrorCode::InvalidInput);
80+
81+
unsafe { error.free_message() };
8082
}
8183

8284
#[test]
@@ -105,6 +107,7 @@ mod tests {
105107
// Clean up
106108
unsafe {
107109
wallet::wallet_free(wallet);
110+
error.free_message();
108111
}
109112
}
110113

@@ -190,6 +193,7 @@ mod tests {
190193
// Clean up
191194
unsafe {
192195
wallet::wallet_free(wallet);
196+
error.free_message();
193197
}
194198
}
195199

key-wallet-ffi/src/address_pool.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,6 +1135,7 @@ mod tests {
11351135
managed_account_free(account);
11361136
wallet_manager_free_wallet_ids(wallet_ids_out, count_out);
11371137
wallet_manager_free(manager);
1138+
error.free_message();
11381139
}
11391140
}
11401141

@@ -1263,6 +1264,7 @@ mod tests {
12631264
managed_account_free(account);
12641265
wallet_manager_free_wallet_ids(wallet_ids_out, count_out);
12651266
wallet_manager_free(manager);
1267+
error.free_message();
12661268
}
12671269
}
12681270
}

0 commit comments

Comments
 (0)