Merge 'Fix boxed memory leaks' from Ihor Andrianov

We should recreate original box to drop it properly
Also made a fast path for hashing. When key div by 2. It should decrease
cpu cycles on hot path by x10 approximately
This thing is tricky, made a long running test that verify bug, put
#[ignore] on it to not slow down CI

Reviewed-by: Preston Thorpe (@PThorpe92)

Closes #1873
This commit is contained in:
Pekka Enberg
2025-07-02 19:42:54 +03:00
3 changed files with 52 additions and 3 deletions

11
Cargo.lock generated
View File

@@ -1966,6 +1966,16 @@ dependencies = [
"autocfg",
]
[[package]]
name = "memory-stats"
version = "1.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c73f5c649995a115e1a0220b35e4df0a1294500477f97a91d0660fb5abeb574a"
dependencies = [
"libc",
"windows-sys 0.52.0",
]
[[package]]
name = "miette"
version = "7.6.0"
@@ -3679,6 +3689,7 @@ dependencies = [
"libloading",
"libm",
"lru",
"memory-stats",
"miette",
"mimalloc",
"parking_lot",

View File

@@ -78,6 +78,7 @@ built = { version = "0.7.5", features = ["git2", "chrono"] }
pprof = { version = "0.14.0", features = ["criterion", "flamegraph"] }
[dev-dependencies]
memory-stats = "1.2.0"
criterion = { version = "0.5", features = [
"html_reports",
"async",

View File

@@ -138,7 +138,9 @@ impl DumbLruPageCache {
// Try to detach from LRU list first, can fail
self.detach(ptr, clean_page)?;
let ptr = self.map.borrow_mut().remove(&key).unwrap();
unsafe { std::ptr::drop_in_place(ptr.as_ptr()) };
unsafe {
let _ = Box::from_raw(ptr.as_ptr());
};
Ok(())
}
@@ -299,7 +301,9 @@ impl DumbLruPageCache {
unsafe {
assert!(!current_entry.as_ref().page.is_dirty());
}
unsafe { std::ptr::drop_in_place(current_entry.as_ptr()) };
unsafe {
let _ = Box::from_raw(current_entry.as_ptr());
};
current = next;
}
let _ = self.head.take();
@@ -570,7 +574,11 @@ impl PageHashMap {
}
fn hash(&self, key: &PageCacheKey) -> usize {
key.pgno % self.capacity
if self.capacity.is_power_of_two() {
key.pgno & (self.capacity - 1)
} else {
key.pgno % self.capacity
}
}
pub fn rehash(&self, new_capacity: usize) -> PageHashMap {
@@ -1184,4 +1192,33 @@ mod tests {
cache.verify_list_integrity();
assert!(cache.insert(create_key(4), page_with_content(4)).is_ok());
}
#[test]
#[ignore = "long running test, remove to verify"]
fn test_clear_memory_stability() {
let initial_memory = memory_stats::memory_stats().unwrap().physical_mem;
for _ in 0..100000 {
let mut cache = DumbLruPageCache::new(1000);
for i in 0..1000 {
let key = create_key(i);
let page = page_with_content(i);
cache.insert(key, page).unwrap();
}
cache.clear().unwrap();
drop(cache);
}
let final_memory = memory_stats::memory_stats().unwrap().physical_mem;
let growth = final_memory.saturating_sub(initial_memory);
println!("Growth: {}", growth);
assert!(
growth < 10_000_000,
"Memory grew by {} bytes over 10 cycles",
growth
);
}
}