Skip to content

Commit 9fc3cc9

Browse files
committed
fix clean up in tests
Signed-off-by: Simon Davies <[email protected]>
1 parent 7d6e385 commit 9fc3cc9

File tree

1 file changed

+22
-14
lines changed

1 file changed

+22
-14
lines changed

src/hyperlight_host/src/mem/linux_dirty_page_tracker.rs

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use log::error;
2626
use crate::mem::shared_mem::{HostMapping, SharedMemory};
2727
use crate::{Result, new_error};
2828

29-
// Tracker metadata stored in TLS
29+
// Tracker metadata stored in global lock-free storage
3030
struct TrackerData {
3131
pid: u32,
3232
base_addr: usize,
@@ -56,7 +56,7 @@ static HANDLER_INSTALLED: AtomicBool = AtomicBool::new(false);
5656

5757
/// Dirty page tracker for Linux
5858
/// This tracks which pages have been written for a memory region once new has been called
59-
/// It marks pages as RO and then uses SIGSEGV to detect writes to pages, then updates the page to RW and notes the page index as dirty by writing details to TLS
59+
/// It marks pages as RO and then uses SIGSEGV to detect writes to pages, then updates the page to RW and notes the page index as dirty by writing details to global lock-free storage
6060
///
6161
/// A user calls get_dirty_pages to get a list of dirty pages to get details of the pages that were written to since the tracker was created
6262
///
@@ -97,7 +97,6 @@ impl LinuxDirtyPageTracker {
9797
// Get the current process ID
9898
let current_pid = std::process::id();
9999

100-
#[cfg(any(debug_assertions, test))]
101100
// Check that there is not already a tracker that includes this address range
102101
// within the same process (virtual addresses are only unique per process)
103102
for guard in get_trackers().iter() {
@@ -636,7 +635,7 @@ mod tests {
636635
"Failed to create first tracker for test case: {}",
637636
description
638637
);
639-
let _tracker1 = tracker1.unwrap();
638+
let tracker1 = tracker1.unwrap();
640639

641640
// Try to create overlapping tracker - this should fail
642641
let test_memory2 = TestSharedMemory::new(new_addr, *new_size);
@@ -654,7 +653,7 @@ mod tests {
654653
println!(" ✓ Correctly rejected overlap");
655654

656655
// Clean up by dropping the tracker
657-
drop(_tracker1);
656+
drop(tracker1);
658657
println!();
659658
}
660659

@@ -723,7 +722,7 @@ mod tests {
723722
"Failed to create first tracker for non-overlap test: {}",
724723
description
725724
);
726-
let _tracker1 = tracker1.unwrap();
725+
let tracker1 = tracker1.unwrap();
727726

728727
// Try to create non-overlapping tracker - this should succeed
729728
let test_memory2 = TestSharedMemory::new(new_addr, *new_size);
@@ -738,12 +737,12 @@ mod tests {
738737
new_addr + new_size
739738
);
740739

741-
let _tracker2 = tracker2_result.unwrap();
740+
let tracker2 = tracker2_result.unwrap();
742741
println!(" ✓ Correctly allowed non-overlapping ranges");
743742

744743
// Clean up
745-
drop(_tracker1);
746-
drop(_tracker2);
744+
drop(tracker1);
745+
drop(tracker2);
747746
println!();
748747
}
749748

@@ -758,8 +757,8 @@ mod tests {
758757
let base_addr = memory_ptr as usize;
759758

760759
// Create two non-overlapping trackers first
761-
let _tracker1 = create_test_tracker(base_addr + PAGE_SIZE * 2, PAGE_SIZE * 3).unwrap();
762-
let _tracker2 = create_test_tracker(base_addr + PAGE_SIZE * 8, PAGE_SIZE * 3).unwrap();
760+
let tracker1 = create_test_tracker(base_addr + PAGE_SIZE * 2, PAGE_SIZE * 3).unwrap();
761+
let tracker2 = create_test_tracker(base_addr + PAGE_SIZE * 8, PAGE_SIZE * 3).unwrap();
763762

764763
// Try to create a tracker that overlaps with tracker1
765764
let overlap_with_1 = create_test_tracker(base_addr + PAGE_SIZE * 3, PAGE_SIZE * 3);
@@ -786,6 +785,11 @@ mod tests {
786785
let no_overlap = create_test_tracker(base_addr + PAGE_SIZE * 12, PAGE_SIZE * 2);
787786
assert!(no_overlap.is_ok(), "Should allow non-overlapping tracker");
788787

788+
// Explicitly drop all trackers before freeing memory
789+
drop(tracker1);
790+
drop(tracker2);
791+
drop(no_overlap);
792+
789793
unsafe {
790794
free_aligned_memory(memory_ptr, memory_size);
791795
}
@@ -915,7 +919,7 @@ mod tests {
915919
}
916920

917921
#[test]
918-
fn test_tls_cleanup_on_drop() {
922+
fn test_cleanup_on_drop() {
919923
let (memory_ptr, memory_size) = create_aligned_memory(PAGE_SIZE * 2);
920924
let addr = memory_ptr as usize;
921925

@@ -932,11 +936,11 @@ mod tests {
932936
} // tracker is dropped here
933937

934938
// Create a new tracker for the same memory region
935-
// This should work without issues if TLS was properly cleaned up
939+
// This should work without issues if data was properly cleaned up
936940
let new_tracker = create_test_tracker(addr, memory_size);
937941
assert!(
938942
new_tracker.is_ok(),
939-
"TLS not properly cleaned up on tracker drop"
943+
"Data not properly cleaned up on tracker drop"
940944
);
941945

942946
unsafe {
@@ -1246,6 +1250,10 @@ mod tests {
12461250
"Tracker 2 should store the current process ID"
12471251
);
12481252

1253+
// Explicitly drop trackers before freeing memory
1254+
drop(tracker1);
1255+
drop(tracker2);
1256+
12491257
// Clean up
12501258
unsafe {
12511259
free_aligned_memory(memory_ptr1, memory_size1);

0 commit comments

Comments
 (0)