Skip to content

Commit bea1fc7

Browse files
committed
Make test_drop able to run concurrently
Signed-off-by: Ludvig Liljenberg <[email protected]>
1 parent df35fab commit bea1fc7

File tree

2 files changed

+40
-30
lines changed

2 files changed

+40
-30
lines changed

Justfile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,6 @@ test-isolated target=default-target features="" :
167167
{{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- sandbox::uninitialized::tests::test_log_trace --exact --ignored
168168
{{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- sandbox::initialized_multi_use::tests::create_1000_sandboxes --exact --ignored
169169
{{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- sandbox::outb::tests::test_log_outb_log --exact --ignored
170-
{{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- mem::shared_mem::tests::test_drop --exact --ignored
171170
{{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --test integration_test -- log_message --exact --ignored
172171
@# metrics tests
173172
{{ cargo-cmd }} test {{ if features =="" {''} else if features=="no-default-features" {"--no-default-features" } else {"--no-default-features -F function_call_metrics,init-paging," + features } }} --profile={{ if target == "debug" { "dev" } else { target } }} {{ target-triple-flag }} -p hyperlight-host --lib -- metrics::tests::test_metrics_are_emitted --exact

src/hyperlight_host/src/mem/shared_mem.rs

Lines changed: 40 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1331,50 +1331,61 @@ mod tests {
13311331
assert_eq!(data, ret_vec);
13321332
}
13331333

1334-
/// A test to ensure that, if a `SharedMem` instance is cloned
1335-
/// and _all_ clones are dropped, the memory region will no longer
1336-
/// be valid.
1337-
///
1338-
/// This test is ignored because it is incompatible with other tests as
1339-
/// they may be allocating memory at the same time.
1340-
///
1341-
/// Marking this test as ignored means that running `cargo test` will not
1342-
/// run it. This feature will allow a developer who runs that command
1343-
/// from their workstation to be successful without needing to know about
1344-
/// test interdependencies. This test will, however, be run explicitly as a
1345-
/// part of the CI pipeline.
1334+
/// Test that verifies memory is properly unmapped when all SharedMemory
1335+
/// references are dropped.
13461336
#[test]
1347-
#[ignore]
13481337
#[cfg(target_os = "linux")]
13491338
fn test_drop() {
1350-
use proc_maps::maps_contain_addr;
1339+
use proc_maps::get_process_maps;
1340+
1341+
// Use a unique size that no other test uses to avoid false positives
1342+
// from concurrent tests allocating at the same address.
1343+
// The mprotect calls split the mapping into 3 regions (guard, usable, guard),
1344+
// so we check for the usable region which has this exact size.
1345+
//
1346+
// NOTE: If this test fails intermittently, there may be a race condition
1347+
// where another test allocates memory at the same address between our
1348+
// drop and the mapping check. Ensure UNIQUE_SIZE is not used by any
1349+
// other test in the codebase to avoid this.
1350+
const UNIQUE_SIZE: usize = PAGE_SIZE_USIZE * 17;
13511351

13521352
let pid = std::process::id();
13531353

1354-
let eshm = ExclusiveSharedMemory::new(PAGE_SIZE_USIZE).unwrap();
1354+
let eshm = ExclusiveSharedMemory::new(UNIQUE_SIZE).unwrap();
13551355
let (hshm1, gshm) = eshm.build();
13561356
let hshm2 = hshm1.clone();
1357-
let addr = hshm1.raw_ptr() as usize;
13581357

1359-
// ensure the address is in the process's virtual memory
1360-
let maps_before_drop = proc_maps::get_process_maps(pid.try_into().unwrap()).unwrap();
1358+
// Use the usable memory region (not raw), since mprotect splits the mapping
1359+
let base_ptr = hshm1.base_ptr() as usize;
1360+
let mem_size = hshm1.mem_size();
1361+
1362+
// Helper to check if exact mapping exists (matching both address and size)
1363+
let has_exact_mapping = |ptr: usize, size: usize| -> bool {
1364+
get_process_maps(pid.try_into().unwrap())
1365+
.unwrap()
1366+
.iter()
1367+
.any(|m| m.start() == ptr && m.size() == size)
1368+
};
1369+
1370+
// Verify mapping exists before drop
13611371
assert!(
1362-
maps_contain_addr(addr, &maps_before_drop),
1363-
"shared memory address {:#x} was not found in process map, but should be",
1364-
addr,
1372+
has_exact_mapping(base_ptr, mem_size),
1373+
"shared memory mapping not found at {:#x} with size {}",
1374+
base_ptr,
1375+
mem_size
13651376
);
1366-
// drop both shared memory instances, which should result
1367-
// in freeing the memory region
1377+
1378+
// Drop all references
13681379
drop(hshm1);
13691380
drop(hshm2);
13701381
drop(gshm);
13711382

1372-
let maps_after_drop = proc_maps::get_process_maps(pid.try_into().unwrap()).unwrap();
1373-
// now, ensure the address is not in the process's virtual memory
1383+
// Verify exact mapping is gone
13741384
assert!(
1375-
!maps_contain_addr(addr, &maps_after_drop),
1376-
"shared memory address {:#x} was found in the process map, but shouldn't be",
1377-
addr
1385+
!has_exact_mapping(base_ptr, mem_size),
1386+
"shared memory mapping still exists at {:#x} with size {} after drop",
1387+
base_ptr,
1388+
mem_size
13781389
);
13791390
}
13801391

@@ -1402,7 +1413,7 @@ mod tests {
14021413
// Write data at the given offset
14031414
hshm.copy_from_slice(&test_data, start_offset).unwrap();
14041415

1405-
// Read it back
1416+
// Read it backunk optimization works correctly
14061417
let mut read_buf = vec![0u8; test_len];
14071418
hshm.copy_to_slice(&mut read_buf, start_offset).unwrap();
14081419

0 commit comments

Comments
 (0)