dynamic_reload icon indicating copy to clipboard operation
dynamic_reload copied to clipboard

I get failed to reload

Open tyoc213 opened this issue 7 years ago • 28 comments

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.

tyoc213 avatar Nov 23 '16 15:11 tyoc213

What errors are you seeing? Could you please include the errors generated here.

emoon avatar Nov 23 '16 15:11 emoon

Well, I dont see errors (as a backtrace)

PS C:\Users\tyoc\Documents\GitHub\dynamic_reload> cargo run --example example  --release
   Compiling dynamic_reload v0.2.0 (file:///C:/Users/tyoc/Documents/GitHub/dynamic_reload)
    Finished release [optimized] target(s) in 2.16 secs
     Running `target\release\examples\example.exe`
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Value 4
Failed to reload

If you know how can I debug I would give a try on win...


Wel it just stops there, there is no termination of the program, I need to Ctrl+C and enter to exit.

tyoc213 avatar Nov 23 '16 15:11 tyoc213

And as I have said, it try to reload almost at the same time I hit cargo build --release or cargo build so it doesnt give time for the process to finish.

It is a problem of timming, I hit sometimes the sleep thread::sleep(Duration::from_millis(500)); and maybe other times not when building, so it loads OK.

So I wonder what would will be the correct "momment" to load (not time based).

tyoc213 avatar Nov 23 '16 15:11 tyoc213

When I test now I actually get the same issue. I will try to look at this.

emoon avatar Nov 23 '16 15:11 emoon

Maybe the "timestamp" of files is now changed before building? or something like that? if prior testing did not result on this "misses".

tyoc213 avatar Nov 23 '16 15:11 tyoc213

Might be. I have some code in there to handle if I can't load the file. Also I always copy the file to a separate directory and load it from there (you can't write an open dll on windows) I will try to investigate it but right now a bit busy but I will try to do it next week unless I get some free time this one.

emoon avatar Nov 23 '16 16:11 emoon

@emoon if oyu provide a little direction I could try :)

tyoc213 avatar Jul 27 '17 01:07 tyoc213

Oh crap. I totally forgot about this. Let me get back to you.

emoon avatar Jul 27 '17 01:07 emoon

Hi again,

So the way this library works:

  1. Before loading a DLL (or sharedlib) it will make a copy of it
  2. The copy of the dll will be loaded and the original file will be watched for changes.
  3. If a change is detected in the original file the copy will be unloaded and a new copy of the original file will be done and reloaded.

The reason for this code is that Windows doesn't allow for modifying already loaded dlls which this works on macOS and Linux for example.

I have a bunch of code in there that should ensure that reload/copy of the file should work but there might be some cases that doesn't work. Like sometimes when you get a notification that a file has changed it's actually not possible to copy it yet. I have some code that tries to deal with this by checking we can read from it. If we can't we sleep for a while and try again with fixed number. If this still fails I just bail and say fail reload.

Hopefully this gives you an overview of how it works. If you have any more specific question I will love to help you out and I really appreciate that you want to take a look at this :)

emoon avatar Jul 27 '17 07:07 emoon

@tyoc213 You could also try adding a delay to the UpdateState::Before handler to give the build system time to finish the build.

Neopallium avatar Mar 28 '19 07:03 Neopallium

@emoon hmm. It might be a safer to watch the .cargo-lock file. When .cargo-lock is opened, only mark libraries for reload. Then .cargo-lock is closed, reload marked libraries.

Neopallium avatar Mar 28 '19 08:03 Neopallium

That won't work if you produce dynamic libs from somewhere else than cargo tho.

emoon avatar Mar 28 '19 08:03 emoon

It could be added as an optional feature. Also I think we could look for .cargo-lock in the same folder as the library. When any .cargo-lock file is opened, delay reloading.

Neopallium avatar Mar 28 '19 08:03 Neopallium

Yeah that might work but I would rather have the general case be reliable that this wouldn't be needed.

emoon avatar Mar 28 '19 08:03 emoon

I think the general case would need to wait X seconds after the last detected change, before reloading. While testing I have seen multiple events trigger a reload, even when using the Debounce watcher. It doesn't seem to be easy to be 100% sure that the compiler has finished writing the library.

Neopallium avatar Mar 28 '19 08:03 Neopallium

Yeah I guess so :/ It's annoying that there seems to be no reliable way to do this. Maybe wait, hash file, wait rehash to see if file has still changed may be a way forward?

emoon avatar Mar 28 '19 08:03 emoon

watching .cargo-lock with the Debounce watcher doesn't work, since it is only opened/closed. Would have to switch back to the raw watcher and do our own debouncing.

Neopallium avatar Mar 28 '19 08:03 Neopallium

yes. Hashing would be good. Could do the hash check after shadow copying the lib.

  1. Shadow copy.
  2. hash copy.
  3. wait 100ms.
  4. hash original and compare.
  5. Goto 1 if hash doesn't match?

Neopallium avatar Mar 28 '19 08:03 Neopallium

Something like that yeah

emoon avatar Mar 28 '19 08:03 emoon

Someone told me to look at https://github.com/fungos/cr for reference as they say it works there. Might be interesting to see if they have solved these problems in a good way there.

emoon avatar Mar 28 '19 09:03 emoon

cr just checks timestamps to detect library changes.

One important feature from cr is that they version the library file each time they reload. I was just adding timestamps to the library filename to fix some crashes I have been having on Linux.

Calling dlopen() with the same filename, doesn't seem to be reliable. Using a unique name for each reload and not closing the old copy seems to fix the issue with TLS usage in the library.

Neopallium avatar Mar 28 '19 10:03 Neopallium

I see

emoon avatar Mar 28 '19 10:03 emoon

aah. They rollback to the previous library version if the reload fails. Also they validate the library file sections and reload data in the sections.

Seems it would be easier to just wrap cr.h in a crate and use that instead of libloading.

Neopallium avatar Mar 28 '19 10:03 Neopallium

Yeah that might be a good idea

emoon avatar Mar 28 '19 10:03 emoon

I might look into wrapping cr.h in a crate later.

For now, versioning the library copy on each reload, hash checks, and leaking/forgetting the previous instance would help a lot.

Neopallium avatar Mar 28 '19 10:03 Neopallium

Indeed. Sounds good 👍

emoon avatar Mar 28 '19 10:03 emoon

Any update on this? :)

Isn't it the case that this crate only gets notified by the OS when the dll file was closed after writing to it? In that case, why would debounce be necessary? Or is it the case that this crate can only get notified every time data is written to the dll file, but not when it gets closed after being opened for writing?

Btw, this recent article goes into detail about the issue that the lib is not reloading due to the TLS destructor: https://fasterthanli.me/articles/so-you-want-to-live-reload-rust

Boscop avatar Oct 27 '20 23:10 Boscop

@Boscop I ran into those same TLS destructor issues over a year ago https://github.com/fungos/cr/issues/35 while making a Rust binding for cr.h: A Simple C Hot Reload Header-only Library. See my comments on that issue. If I remember correctly, the simplest solution is to alway use a different filename (rename/copy the SO file and add a version number before loading). That way the if the old SO version is not released/unloaded because of TLS destructors, the new version can still be loaded.

Neopallium avatar Nov 01 '20 05:11 Neopallium