Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid free memory while dropping #67179

Closed
sidgilles opened this issue Dec 9, 2019 · 3 comments
Closed

Invalid free memory while dropping #67179

sidgilles opened this issue Dec 9, 2019 · 3 comments

Comments

@sidgilles
Copy link

sidgilles commented Dec 9, 2019

-------------------
Edit

I guess this bug will not get fixed by any hook, only by fixing it in the library. Or, at least, i did not find a workaround myself. The problem lies in the way BTreeMap is coded. See the link.

static EMPTY_ROOT_NODE: NodeHeader<(), ()> = NodeHeader {

The static reference is embedded in each binary after compilation and linking process. So, i guess, the "static EMPTY_ROOT_NODE" in main exe and in the library refers to different location. See this output below.

$ nm /home/sidney/Developpement/test_bug/target/debug/libclib.so | grep "EMPTY_ROOT_NODE"
000000000002e780 r _ZN5alloc11collections5btree4node15EMPTY_ROOT_NODE17h0caf86d227d3ce02E
$ nm /home/sidney/Developpement/test_bug/target/debug/libclib.so | rustfilt | grep "2e780"
000000000002e780 r alloc::collections::btree::node::EMPTY_ROOT_NODE

$ nm /home/sidney/Developpement/test_bug/target/debug/main | grep "EMPTY_ROOT_NODE"
0000000000039ff8 R _ZN5alloc11collections5btree4node15EMPTY_ROOT_NODE17h0caf86d227d3ce02E
$ nm /home/sidney/Developpement/test_bug/target/debug/main | rustfilt | grep "39ff8"
0000000000039ff8 R alloc::collections::btree::node::EMPTY_ROOT_NODE

Passing a BTreeMap created in main and passing it to Foo led to a bad behavior because the static reference do not point to the same memory location while dropping. I might be wrong but it make sense to me according to the previous output.

-------------------

Hello,

Basically, my program is dynamically loading a library (cdylib) and retrieve a factory object (Factory design pattern). No problem here.
Then, on demand, a new object with the Foo trait is created. Again, nothing wrong with that. However, moving an object created in the main program to the Foo object instance, lead to an invalid free memory, according to valgrind, when dropping the Foo object.

Edit : This also exist on stable build --> rustc 1.39.0 (2019-11-04)
Edit : Trying to change ["cdylib"] to ["dylib"] lead to a strange behavior : the library is loaded via 'cargo run' but no more via a regular exec by the shell (thus also by valgrind). To let you know that i did not found an understandable workaround.

$ rustc --version
rustc 1.41.0-nightly (59947fcae 2019-12-08)

$ cargo run
    Finished dev [unoptimized + debuginfo] target(s) in 1.29s
     Running `target/debug/main`
--on_plugin_load---
--foo---
dropping Foo object
dropping Drv2
munmap_chunk(): invalid pointer
Abandon (core dumped)


$ valgrind /home/sidney/Developpement/test_bug/target/debug/main
==19143== Memcheck, a memory error detector
==19143== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==19143== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==19143== Command: /home/sidney/Developpement/test_bug/target/debug/main
==19143== 
--on_plugin_load---
--foo---
dropping Foo object
dropping Drv2
==19143== Invalid free() / delete / delete[] / realloc()
==19143==    at 0x483897B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19143==    by 0x4EA64F2: alloc::alloc::dealloc (alloc.rs:106)
==19143==    by 0x4EA63A8: <alloc::alloc::Global as core::alloc::Alloc>::dealloc (alloc.rs:177)
==19143==    by 0x4EA3785: alloc::collections::btree::node::NodeRef<alloc::collections::btree::node::marker::Owned,K,V,alloc::collections::btree::node::marker::Leaf>::deallocate_and_ascend (node.rs:504)
==19143==    by 0x4EA1C5B: <alloc::collections::btree::map::IntoIter<K,V> as core::ops::drop::Drop>::drop (map.rs:1431)
==19143==    by 0x4EA611D: core::ptr::real_drop_in_place (mod.rs:181)
==19143==    by 0x4EA2225: core::mem::drop (mod.rs:749)
==19143==    by 0x4EA1BCE: <alloc::collections::btree::map::BTreeMap<K,V> as core::ops::drop::Drop>::drop (map.rs:132)
==19143==    by 0x4EA60BD: core::ptr::real_drop_in_place (mod.rs:181)
==19143==    by 0x4EA615D: core::ptr::real_drop_in_place (mod.rs:181)
==19143==    by 0x111D07: core::ptr::real_drop_in_place (mod.rs:181)
==19143==    by 0x112ED7: core::mem::drop (mod.rs:749)
==19143==  Address 0x13ef38 is 0 bytes inside data symbol "_ZN5alloc11collections5btree4node15EMPTY_ROOT_NODE17h0caf86d227d3ce02E"
==19143== 
drop Registry #1
==19143== 
==19143== HEAP SUMMARY:
==19143==     in use at exit: 3,116 bytes in 12 blocks
==19143==   total heap usage: 40 allocs, 29 frees, 6,939 bytes allocated
==19143== 
==19143== LEAK SUMMARY:
==19143==    definitely lost: 0 bytes in 0 blocks
==19143==    indirectly lost: 0 bytes in 0 blocks
==19143==      possibly lost: 0 bytes in 0 blocks
==19143==    still reachable: 3,116 bytes in 12 blocks
==19143==         suppressed: 0 bytes in 0 blocks
==19143== Rerun with --leak-check=full to see details of leaked memory
==19143== 
==19143== For counts of detected and suppressed errors, rerun with: -v
==19143== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)



Here the code:

Common traits definition - 'common' crate

use std::any::Any;
use std::collections::BTreeMap;

pub trait Plugin: Any + Send + Sync + std::fmt::Debug {
    fn new_subplugin_trait(&self, bmap : BTreeMap<usize, usize>) -> Box<dyn Foo>;
    fn on_plugin_load(&self);
}

pub trait Foo: Any + Send + Sync + std::fmt::Debug {
    fn foo(&self);
}

Cdynlib... Rust book was around...

use common::*;


use std::collections::BTreeMap;
//use libloading::{Library, Symbol};


#[macro_export]
macro_rules! declare_plugin {
    ($plugin_type:ty, $constructor:path) => {
        #[no_mangle]
        pub extern "C" fn _plugin_create() -> *mut dyn $crate::Plugin {
            // make sure the constructor is the correct type.
            let constructor: fn() -> $plugin_type = $constructor;

            let object = constructor();
            //println!("Plugin : object = {:?}", object);
            let boxed: Box<dyn $crate::Plugin> = Box::new(object);
            //println!("Plugin : boxed = {:?}", boxed);
            let raw_pointer = Box::into_raw(boxed);
            //println!("Plugin : raw_pointer = {:?}", raw_pointer);
            return raw_pointer;
        }
    };
}


//--------

#[derive(Debug)]
pub(crate) struct Drv2 {
    bus_driver : BTreeMap<usize, usize>,
}
impl Drv2{
    pub fn new(bmap : BTreeMap<usize, usize>,) -> Self {
        Self {
            bus_driver : bmap,
        }
    }
}
impl Foo for Drv2 {

    fn foo(&self) {
         println!("--foo---");
    }
}
impl Drop for Drv2 { // mandatory. if not, segfault occurs
    fn drop(&mut self) {
        println!("dropping Drv2");
    }
}


// ------

#[derive(Debug)]
pub(crate) struct Drv {
    bus_driver : BTreeMap<usize, usize>,
}
impl Drv{
    pub fn new() -> Self {
        Self {
            bus_driver : BTreeMap::<usize, usize>::new(),
        }
    }
}
impl Plugin for Drv {

    fn new_subplugin_trait(&self, bmap : BTreeMap<usize, usize>) -> Box<dyn Foo> {
        return Box::new(Drv2::new(bmap));
    }

    fn on_plugin_load(&self) {
        println!("--on_plugin_load---");
    }

}


declare_plugin!(Drv, Drv::new);

corresponding TOML

[dependencies]
common = { path = "../common" } # from a path in the local filesystem
[lib]
crate-type = ["cdylib"]

And finally, main program here

use libloading::{Library, Symbol};
use common::Plugin;
use std::collections::BTreeMap;



// This Registry class maintain alive and in scope the library import and thus -- important -- the memory space allocated for it.
// Loosing scope will desallocate this memory. Uses of each memory object created via this library will lead to segmentation fault.

#[derive(Debug)]
struct Entry {
    name : String,
    lib : libloading::Library,
    factory : Box<dyn Plugin>,
}

#[derive(Debug)]
pub struct Registry {
    factory_list : Vec<Entry>,
}


impl Registry {

    pub fn new() -> Self {
        Self {
            factory_list : Vec::new(),
        }

    }

    pub fn load_from_folder(&mut self, path_str : String) -> () {

        unsafe {

            //load and reference the library
            if let libloading::Result::Ok(lib) = libloading::Library::new(&path_str) {
                if let libloading::Result::Ok(constructor) = lib.get::<libloading::Symbol<unsafe fn() -> *mut dyn Plugin>>(b"_plugin_create") {

                    //println!("Plugin : constructor = {:?}", constructor);
                    let boxed_raw = constructor();
                    //println!("Plugin : boxed_raw = {:?}", boxed_raw);

                    let factory = Box::<dyn Plugin>::from_raw(boxed_raw);
                    //println!("Plugin : boxed_any_trait = {:?}", factory);

                    factory.on_plugin_load();
                    self.factory_list.push( Entry {
                        name : path_str,
                        lib : lib,
                        factory : factory,
                    });

                    //self.factory_list.push(boxed_any_trait);

                }
            }
        }


    }


}

impl Drop for Registry { // mandatory. if not, segfault occurs
    fn drop(&mut self) {
        println!("drop Registry #1");
        for Entry {
                name,
                lib,
                factory,
            } in self.factory_list.drain(..) {

            //factory.on_plugin_unload();
            std::mem::drop(factory);
        }
    }
}



fn main() {

    let mut a  = Registry::new();
    //println!("{:#?}", a);
    a.load_from_folder("/home/sidney/Developpement/test_bug/target/debug/libclib.so".to_string());
    //println!("{:#?}", a);
    let mut b = a.factory_list[0].factory.new_subplugin_trait(BTreeMap::<usize, usize>::new());
    //println!("{:#?}", b);
    b.foo();
    println!("dropping Foo object");
    std::mem::drop(b);

}

and corresponding TOML

[dependencies]
common = { path = "../common" } # from a path in the local filesystem
libloading = "0.5.2"

Note : i had an issue with loadlibrary because of the created Symbol lifetime. Here is another matter.

Final note : this bug does not appear on zero-sized type (replacing BTreeMap by ZST_Object {} in signatures ).

@sidgilles sidgilles changed the title Invalid free memory while dropping - cdynlib and factory pattern Invalid free memory while dropping Dec 13, 2019
@ollie27
Copy link
Member

ollie27 commented Dec 31, 2019

See #63338.

@jonas-schievink
Copy link
Contributor

Closing since this is not a supported configuration

@sidgilles
Copy link
Author

Ok, sounds clear. (#63338 (comment))

This means articles and threads like https://users.rust-lang.org/t/how-to-write-plugins-in-rust/6325/18 , https://michael-f-bryan.github.io/rust-ffi-guide/dynamic_loading.html , https://s3.amazonaws.com/temp.michaelfbryan.com/dynamic-loading/index.html are not intend to be used within the rust ecosystem.
But, i guess, for many projects, it is desirable to introduce a kind of modularity. Any way to achieve this, now or in a distant future ? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants