emoon / dynamic_reload Goto Github PK

Sorry for the poor title,
I'm using your crate do enable dynamic loading of modules for my application.
It works fine on linux, tested on a physical gentoo and a docker debian 9.3
On OSX 10.13 High SIerra (haven't tested on other versions) it fails with:

rustegram(4467,0x7fffad81d340) malloc: *** error for object 0x106817040: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
Abort trap: 6

The dylib itself is a Rust written lib, and it passes the tests without problems.

Thanks for making this crate :)
I plan to use it for live coding, and I noticed it uses a 2s delay (notify::watcher(tx, Duration::from_secs(2))), which is too long for my use case, because the recompilation of the dll also adds wait time.
Could you please make this delay configurable? :)

It starts tryng to load the lib even when still compiling packages (for example if I do cargo build --release) it start downloading things, then building and I get the error while it is still building.

Windows 10 here.

I have created the start of a Rust wrapper for cr.h:
https://github.com/Neopallium/cr-rs

Right now I am planning on a host-side and guest-side sys crates, since two different libraries need to be crated. Might be able to use features to keep it as one library that can be compiled as either side as needed.

Right now a basic host example compiles and can load the libbasic_guest.so from cr.h samples.

The only problem I see right now is that cr.h doesn't provide access to the library handle. The cr-sys crate could patch in a function to return the library handle.

When I try the example on my Linux (Ubuntu 16.04) machine, it works properly after the first change to test_shared.rs, but it does nothing if I change it a second time.

if I add println!("{:?}", file); just above this line and then run the example, changing 42 to 24 I get the following output when I run cargo build:

Value 42
Event { path: Some("target/debug/libtest_shared.so"), op: Ok(CHMOD) }
Value 24
Event { path: Some("target/debug/libtest_shared.so"), op: Ok(REMOVE) }
Value 24
Event { path: Some("target/debug/libtest_shared.so"), op: Ok(IGNORED) }
Value 24

But as mentioned before, if I try to change 24 to something else, the output just remains Value 24.

I'm not sure if this is the cause of the problem but I found this in the latest version of the notify library's docs:

Linux

When a watched file or directory is removed, its watch gets destroyed and no new events will be sent.

On windows, if a unix path is used in "search_paths" (ex: DynamicReload::new(Some(vec!["./routes/target/debug/"]), Some("target/debug"), Search::Default); ), the changes to the libraries are not reported. This is most likely because dynamic_reload cannot match the path sent by the watcher to the one saved in the Lib.

Here's an example of events that fails to be matched
ex: RawEvent { path: Some("C:\\Users\\Gab\\Documents\\projects\\test\\./routes/target/debug\\routes.dll"), op: Ok(WRITE), cookie: None }

I've managed to fix this by mapping the path in search_paths with .canonicalize. Therefore replacing search_paths: Vec<&'a str> by search_paths: Vec<PathBuf>

With this the event becomes

RawEvent { path: Some("\\\\?\\C:\\Users\\Gab\\Documents\\projects\\test\\routes\\target\\debug\\routes.dll"), op: Ok(WRITE), cookie: None }

And everything works fine. Not exactly sure how this would impact non Windows OS though...

String based errors are, from what I've seen, not the idiomatic way to do errors.

Currently writing a pr for this.

1:
Here it says:

1. Current directory
2. In the search paths (relative to current directory)
3. Current directory of the executable
4. Search backwards from executable if Backwards has been set DynamicReload::new

What's the difference between 1 and 3?

2:
When wanting to auto-reload multiple dlls, is it recommended to use separate instances of DynamicReload or should only one instance be used?
I'm asking because the crate example uses a Vec for the list of plugins, but conceptually it would be easier to use separate instances, so that there is no way to accidentally use the wrong Plugin trait object with the wrong Lib:

struct PluginManager {
	plugin: Box<dyn Plugin>,
	lib: Arc<Lib>,
}

Also so that individual plugins can be unloaded or stopped from being auto-reloaded.
(Since DynamicReload has no remove_library method.)

3:
Is it safe to move the lib (of type Lib) after this?

		let plugin = unsafe {
			let constructor: Symbol<PluginCreate> = lib.get(b"plugin_create\0").expect("plugin_create");
			let boxed_raw = constructor();
			Box::from_raw(boxed_raw)
		};

Or would moving lib invalidate the references into the dll (the trait object vtable)?
Is it correct that on all platforms this only moves the handle to the dll, and thus doesn't invalidate references into it?

4:
Have you seen this crate? It allows ensuring that no references to the dll outlive it:
https://docs.rs/sharedlib/7.0.0/sharedlib/#choosing-your-guarantees
It could make sense to use it in this crate.

5:
In the example you're using extern "C" for getting the function pointer:

It seems notify-rs 5.0.0 changed the debouncing mechanism. Now it's in a separate crate:
https://crates.io/crates/notify-debouncer-mini

Maybe it would fix this debounce issue: #27

Is there a suggested way to use this together with lazy_static and possibly also multi threading...?

I tried

lazy_static! {
    pub static ref PLUGINS: Plugins = create_plugins();
}

but lazy_static does not work with Rc<>, and changing Rc<> to Arc<> results in the following error:

the trait `std::marker::Sync` is not implemented for `*mut std::os::raw::c_void`
   = note: `*mut std::os::raw::c_void` cannot be shared between threads safely

Do you have any ideas? The error suggests that we shouldn't use it with multi threading anyway...

I noticed some weird behavior:
On Windows 10, no matter which debounce duration I specify, whenever I overwrite the watched DLL (also tested with manual copying in Windows explorer), it reloads the DLL twice, once immediately and once after debounce_duration.
When I log the calls, I get this sequence:

16:49:47 [INFO] load_lib # Initial load_lib call
# I copy & overwrite the file in Windows Explorer, it reloads immediately
16:50:09 [INFO] reload_callback
16:50:09 [INFO] unload_lib
16:50:09 [INFO] reload_callback
16:50:09 [INFO] load_lib
16:50:09 [INFO] DLL reloaded
# Ten seconds later, it reloads again, even though the DLL wasn't overwritten again!
16:50:19 [INFO] reload_callback
16:50:19 [INFO] unload_lib
16:50:19 [INFO] reload_callback
16:50:19 [INFO] load_lib
16:50:19 [INFO] DLL reloaded

This is my code:

use dynamic_reload::{DynamicReload, Lib, PlatformName, Search, UpdateState};
use std::{path::Path, sync::Arc, time::Duration};

pub trait TypedLibApi: Sized {
	fn load(lib: &libloading::Library) -> Result<Self, libloading::Error>;
	fn before_unload(&self);
}

#[derive(SmartDefault)]
struct LibManager<T: TypedLibApi> {
	loaded_libs: Option<(Arc<Lib>, T)>,
	lib_was_loaded: bool,
}

impl<T: TypedLibApi> LibManager<T> {
	fn load_lib(&mut self, lib: &Arc<Lib>) {
		info!("load_lib");
		let api = T::load(&lib.lib).expect("TypedLibApi::load");
		self.loaded_libs = Some((lib.clone(), api));
		self.lib_was_loaded = true;
	}

	fn unload_lib(&mut self, lib: &Arc<Lib>) {
		info!("unload_lib");
		let (loaded_lib, api) = self.loaded_libs.take().unwrap();
		debug_assert!(loaded_lib == *lib);
		api.before_unload();
	}

	fn reload_callback(&mut self, state: UpdateState, lib: Option<&Arc<Lib>>) {
		info!("reload_callback");
		match state {
			UpdateState::Before => Self::unload_lib(self, lib.unwrap()),
			UpdateState::After => Self::load_lib(self, lib.unwrap()),
			UpdateState::ReloadFailed(e) => panic!("Failed to reload: {}", e),
		}
	}

	pub fn lib(&self) -> &T {
		&self.loaded_libs.as_ref().unwrap().1
	}
}

pub struct LibAutoReloader<T: TypedLibApi> {
	reload_handler: DynamicReload,
	lib_mgr: LibManager<T>,
}

impl<T: TypedLibApi> LibAutoReloader<T> {
	pub fn new(lib_path: impl AsRef<Path>, debounce_ms: u64) -> dynamic_reload::Result<Self> {
		let lib_path = lib_path.as_ref();
		let folder = lib_path.parent().unwrap().display().to_string();
		let file_name = lib_path.file_name().unwrap().to_str().unwrap();

		let shadow_dir = std::env::var_os("TEMP").unwrap().into_string().unwrap();
		let mut reload_handler = DynamicReload::new(
			Some(vec![&folder]),
			Some(&shadow_dir),
			Search::Default,
			Duration::from_millis(debounce_ms),
		);
		let mut lib_mgr = LibManager::default();
		match unsafe { reload_handler.add_library(file_name, PlatformName::No) } {
			Ok(lib) => lib_mgr.load_lib(&lib),
			Err(e) => {
				error!("Unable to load dynamic lib {}: {}", lib_path.display(), e);
				return Err(e);
			}
		}
		Ok(LibAutoReloader { reload_handler, lib_mgr })
	}

	pub fn reload_lib_if_changed(&mut self) -> bool {
		self.lib_mgr.lib_was_loaded = false;
		unsafe { self.reload_handler.update(&LibManager::reload_callback, &mut self.lib_mgr) };
		if self.lib_mgr.lib_was_loaded {
			info!("DLL reloaded");
		}
		self.lib_mgr.lib_was_loaded
	}

	pub fn lib(&self) -> &T {
		self.lib_mgr.lib()
	}
}

Then I instantiate it with LibAutoReloader::new(&dll_path, 10_000) and call reload_lib_if_changed regularly.

Any idea why this strange behavior happens? No matter how large I make the debounce_duration, it always reloads the DLL a second time exactly debounce_duration after it was actually overwritten!

I noticed this crate still uses notify-rs 4. Maybe this behavior would be fixed by upgrading to notify-rs 5.0.0 (which changed the debounce mechanism)? #28

Please derive Debug for UpdateState

This is because tempdir has been deprecated with this info from cargo-deny

31 │ tempdir 0.3.7 registry+https://github.com/rust-lang/crates.io-index
   │ ------------------------------------------------------------------- unmaintained advisory detected
   │
   = ID: RUSTSEC-2018-0017
   = Advisory: https://rustsec.org/advisories/RUSTSEC-2018-0017
   = The [`tempdir`](https://crates.io/crates/tempdir) crate has been deprecated
     and the functionality is merged into [`tempfile`](https://crates.io/crates/tempfile).
   = Announcement: https://github.com/rust-lang-deprecated/tempdir/pull/46
   = Solution: No safe upgrade is available!
   = tempdir v0.3.7
     └── dynamic_reload v0.8.0
         └── rv_core v0.1.0