Repository navigation
Check for Memory Leaks #119
Description
Activity
I wanted to start by trying the sanitizers
rustcships with (https://github.com/japaric/rust-san), but they apparently don't work withmuslat this point. Runningvalgrinddoes not work with the stable toolchain, because there's no way to switch to the system allocator from jemalloc.Unfortunately, even if this is possible on nightly,
valgrindstill appears to miss detecting any heap allocation. This does not appear to be related tomusl.We should definitely have this issue in mind as an extra precaution, but it seems we need to wait for better/newer tools and/or toolchain versions before getting some relevant results.
- removed this from the milestone
on Nov 20, 2018 As of 1.28, the
#[global_allocator]attribute was stabilized, enabling the switch to the system or a custom allocator on a stable toolchain. Recently, nightly switched the default allocator toSystemon all platforms.Reacted by Adrian Catangiu and Yenlin ChenHi @memoryruins , This is really useful information that we were not aware of. Thank you!
Indeed, it seems that in the meantime there appeared some new additional datapoints from our last investigation that we could make use of:- As per your information,
jemalloccan now be replaced with theSystemallocator in the stable toolchain. We could try do that and give it a go with thevalgrindtool. - As per sys_util: enable build for non-musl libraries #639, firecracker can be built with glibc also. This means that we should check again to see if the provided rustc sanitizers can now work with musl. If not, we could investigate with the glibc built binary and see what we find there.
For sure, this issue reached a state where it could use some fresh investigation and any help from community is greatly appreciated 👍 .
Reacted by Adrian Catangiu- As per your information,
An issue that arises is that Firecracker's main() does not return "cleanly". Termination is done in a mid-subscriber callback with
unsafe { libc::exit() }. Rust'sstd::process::exit()would do some more cleanup than that, but even still would not run destructors for objects on the stack.If one wants to return an integer exit status -and- guarantee all the destructors have run, the idiom is to put your main code in a wrapped function that returns an integer. Then have the true main() call std::process::exit() with the result of the wrapper. This way, all the code that could need destruction has finished.
I've done that, and tried to make some minimally invasive changes to get an
Option<ExitCode>to bubble up for clean exit. Also I've made all the threads join() so that they can release their resources and have a fully clean output.While clean shutdown is nice for Valgrind, it is slower, and it's also introducing new code paths that could wind up in deadlocks. So I put it under an option...
--cleanup-level 2
We have a bunch of unsafe code & usage of CString which needs to be deallocated explicitly.
We should check to see if we find any memory leaks. We could use valgrind. See https://creativcoder.github.io/post/checking_memory_leaks_in_rust_ffi/ as it might not work out of the box.