From 7e10a97124f254c8f442e208bb88e526505e1e52 Mon Sep 17 00:00:00 2001 From: Vitali Lovich Date: Thu, 3 Sep 2026 12:36:45 -0700 Subject: [PATCH] Fix eventfd leak (#448) The task header wasn't being dropped when the task was being destroyed. Verified that the test passes and also that valgrind no longer complains about this when the fix is in-place: valgrind --leak-check=full --trace-children=yes --suppressions=valgrind.supp target/debug/deps/glommio-d6605e578a7ecc14 executor::test::executor_shutdown_does_not_leak_eventfds Before (test leaks about ~32 MiB if I'm not mistaken): ==3196979== Memcheck, a memory error detector ==3196979== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al. ==3196979== Using Valgrind-3.23.0.GIT and LibVEX; rerun with -h for copyright info ==3196979== Command: target/debug/deps/glommio-d6605e578a7ecc14 executor::test::executor_shutdown_does_not_leak_eventfds ==3196979== running 1 test test executor::test::executor_shutdown_does_not_leak_eventfds ... FAILED failures: ---- executor::test::executor_shutdown_does_not_leak_eventfds stdout ---- thread 'executor::test::executor_shutdown_does_not_leak_eventfds' (3196981) panicked at /home/vlovich/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rusty-fork-0.3.1/src/fork_test.rs:135:9: child exited unsuccessfully with exit status: 70 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ==3196982== Memcheck, a memory error detector ==3196982== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al. ==3196982== Using Valgrind-3.23.0.GIT and LibVEX; rerun with -h for copyright info ==3196982== Command: /home/vlovich/projects/byte-harbor/glommio/target/debug/deps/glommio-d6605e578a7ecc14 --quiet --test-threads 1 --nocapture --exact -- executor::test::executor_shutdown_does_not_leak_eventfds ==3196982== running 1 test thread 'executor::test::executor_shutdown_does_not_leak_eventfds' (3196989) panicked at glommio/src/executor/mod.rs:2953:13: assertion `left == right` failed: eventfds leaked after 10 rounds left: 21 right: 1 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace ==3196982== ==3196982== LEAK SUMMARY: ==3196982== definitely lost: 0 bytes in 0 blocks ==3196982== indirectly lost: 0 bytes in 0 blocks ==3196982== possibly lost: 28,476 bytes in 435 blocks ==3196982== still reachable: 4,193 bytes in 40 blocks ==3196982== suppressed: 560 bytes in 1 blocks ==3196982== Reachable blocks (those to which a pointer was found) are not shown. ==3196982== To see them, rerun with: --leak-check=full --show-leak-kinds=all ==3196982== ==3196982== For lists of detected and suppressed errors, rerun with: -s ==3196982== ERROR SUMMARY: 12 errors from 12 contexts (suppressed: 1 from 1) failures: executor::test::executor_shutdown_does_not_leak_eventfds test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 399 filtered out; finished in 2.26s ==3196979== ==3196979== HEAP SUMMARY: ==3196979== in use at exit: 866 bytes in 7 blocks ==3196979== total heap usage: 1,058 allocs, 1,051 frees, 1,439,425 bytes allocated ==3196979== ==3196979== LEAK SUMMARY: ==3196979== definitely lost: 0 bytes in 0 blocks ==3196979== indirectly lost: 0 bytes in 0 blocks ==3196979== possibly lost: 0 bytes in 0 blocks ==3196979== still reachable: 818 bytes in 6 blocks ==3196979== suppressed: 48 bytes in 1 blocks After: ==3202589== Memcheck, a memory error detector ==3202589== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al. ==3202589== Using Valgrind-3.23.0.GIT and LibVEX; rerun with -h for copyright info ==3202589== Command: target/debug/deps/glommio-d6605e578a7ecc14 executor::test::executor_shutdown_does_not_leak_eventfds ==3202589== running 1 test test executor::test::executor_shutdown_does_not_leak_eventfds ... ok test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 399 filtered out; finished in 3.35s ==3202589== ==3202589== HEAP SUMMARY: ==3202589== in use at exit: 592 bytes in 2 blocks ==3202589== total heap usage: 1,043 allocs, 1,041 frees, 968,940 bytes allocated ==3202589== ==3202589== LEAK SUMMARY: ==3202589== definitely lost: 0 bytes in 0 blocks ==3202589== indirectly lost: 0 bytes in 0 blocks ==3202589== possibly lost: 0 bytes in 0 blocks ==3202589== still reachable: 544 bytes in 1 blocks ==3202589== suppressed: 48 bytes in 1 blocks ==3202589== Reachable blocks (those to which a pointer was found) are not shown. ==3202589== To see them, rerun with: --leak-check=full --show-leak-kinds=all ==3202589== ==3202589== For lists of detected and suppressed errors, rerun with: -s ==3202589== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 1 from 1) The "still reachable" is what we suppressed and is only in the parent report (child doesn't report any leaks). --- glommio/Cargo.toml | 1 + glommio/src/executor/mod.rs | 80 +++++++++++++++++++++++++++++++++++++ glommio/src/task/raw.rs | 8 +++- valgrind.supp | 26 ++++++++++++ 4 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 valgrind.supp diff --git a/glommio/Cargo.toml b/glommio/Cargo.toml index 8b6c381fc..443258c79 100755 --- a/glommio/Cargo.toml +++ b/glommio/Cargo.toml @@ -51,6 +51,7 @@ futures = "0" hdrhistogram = "7" pretty_env_logger = "0" rand = "0" +rusty-fork = "0.3" tokio = { version = "1", default-features = false, features = ["rt", "macros", "rt-multi-thread", "net", "io-util", "time", "sync"] } tracing-subscriber = { version = "0", features = ["env-filter"] } diff --git a/glommio/src/executor/mod.rs b/glommio/src/executor/mod.rs index 5cc2d67ee..52e64b209 100644 --- a/glommio/src/executor/mod.rs +++ b/glommio/src/executor/mod.rs @@ -2864,9 +2864,11 @@ impl ExecutorProxy { #[cfg(test)] mod test { use core::mem::MaybeUninit; + use rusty_fork::rusty_fork_test; use std::{ cell::Cell, collections::HashMap, + fs, sync::{ atomic::{AtomicUsize, Ordering}, Arc, Mutex, @@ -2881,12 +2883,90 @@ mod test { use crate::{ enclose, + io::{DmaFile, OpenOptions, OwnedDmaFile}, timer::{self, sleep, Timer}, SharesManager, }; use super::*; + fn eventfd_count() -> usize { + fs::read_dir("/proc/self/fd") + .expect("failed to enumerate this process's file descriptors") + .filter_map(|entry| { + let entry = entry.ok()?; + fs::read_link(entry.path()).ok() + }) + .filter(|target| target == std::path::Path::new("anon_inode:[eventfd]")) + .count() + } + + fn run_shared_channel_round() { + let (sender, receiver) = crate::channels::shared_channel::new_bounded(1); + + let sender = LocalExecutorBuilder::default() + .io_memory(0) + .spawn(move || async move { + let sender = sender.connect().await; + let file = OpenOptions::new() + .create_new(true) + .read(true) + .write(true) + .tmpfile(true) + .dma_open(std::env::temp_dir()) + .await + .unwrap(); + let file: OwnedDmaFile = file.into(); + sender.send(file).await.unwrap(); + }) + .unwrap(); + + let receiver = LocalExecutorBuilder::default() + .io_memory(0) + .spawn(move || async move { + let receiver = receiver.connect().await; + let file: DmaFile = receiver.recv().await.unwrap().into(); + assert!(file.read_at(0, 1).await.unwrap().is_empty()); + file.close().await.unwrap(); + }) + .unwrap(); + + sender.join().unwrap(); + receiver.join().unwrap(); + } + + // The fork is critical here as it makes sure that the eventfd_count check works regardless of other tests + // running in the same process (which is what happens when running with cargo test instead of cargo nextest). + rusty_fork_test! { + #[test] + fn executor_shutdown_does_not_leak_eventfds() { + // The disconnected notifier is a process-wide singleton. Initialize it before + // measuring so the baseline contains every eventfd that is expected to persist. + let _ = crate::sys::get_sleep_notifier_for(usize::MAX); + let initial_eventfds = eventfd_count(); + + // Run enough rounds to make the leak from #448 unambiguous, then check that + // additional executor shutdowns do not accumulate descriptors either. + for _ in 0..10 { + run_shared_channel_round(); + } + assert_eq!( + eventfd_count(), + initial_eventfds, + "eventfds leaked after 10 rounds" + ); + + for _ in 0..90 { + run_shared_channel_round(); + } + assert_eq!( + eventfd_count(), + initial_eventfds, + "eventfds leaked after 100 rounds" + ); + } + } + #[test] fn create_and_destroy_executor() { let mut var = Rc::new(RefCell::new(0)); diff --git a/glommio/src/task/raw.rs b/glommio/src/task/raw.rs index a42a8e237..67759aa9e 100644 --- a/glommio/src/task/raw.rs +++ b/glommio/src/task/raw.rs @@ -407,8 +407,8 @@ where /// Cleans up task's resources and deallocates it. /// - /// The schedule function will be dropped, and the task will then get - /// deallocated. The task must be closed before this function is called. + /// The schedule function and header will be dropped, and the task will then + /// get deallocated. The task must be closed before this function is called. #[inline] unsafe fn destroy(ptr: *const ()) { dbg_context!(ptr, "destroy", { @@ -422,6 +422,10 @@ where abort_on_panic(|| { // Drop the schedule function. (raw.schedule as *mut S).drop_in_place(); + + // Drop the header so resources owned by it, such as the executor's + // sleep notifier, are released before the task allocation is freed. + (raw.header as *mut Header).drop_in_place(); }); // Finally, deallocate the memory reserved by the task. diff --git a/valgrind.supp b/valgrind.supp new file mode 100644 index 000000000..8078b0fd6 --- /dev/null +++ b/valgrind.supp @@ -0,0 +1,26 @@ +# The process-global reactor registry removes each SleepNotifier entry, but the +# hash map retains its bucket allocation for reuse until process exit. +{ + glommio_reactor_notifier_registry_capacity + Memcheck:Leak + match-leak-kinds: possible + fun:malloc + ... + fun:*hashbrown*RawTableInner*new_uninitialized* + ... + fun:*glommio*ReactorGlobalState*new_local_state +} + +# The Rust test harness retains the std::sync::mpmc context for its main +# thread-local test-event receiver until process exit. +{ + rust_test_harness_mpmc_context_tls + Memcheck:Leak + match-leak-kinds: possible + fun:malloc + ... + fun:current_or_unnamed + fun:*std4sync4mpmc7context*Context3new + ... + fun:*test*run_tests* +}