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

I get failed to reload #9

Open
tyoc213 opened this issue Nov 23, 2016 · 29 comments
Open

I get failed to reload #9

tyoc213 opened this issue Nov 23, 2016 · 29 comments

Comments

@tyoc213
Copy link

tyoc213 commented Nov 23, 2016

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.

@emoon
Copy link
Owner

emoon commented Nov 23, 2016

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

@tyoc213
Copy link
Author

tyoc213 commented Nov 23, 2016

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
Copy link
Author

tyoc213 commented Nov 23, 2016

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).

@emoon
Copy link
Owner

emoon commented Nov 23, 2016

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

@tyoc213
Copy link
Author

tyoc213 commented Nov 23, 2016

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

@emoon
Copy link
Owner

emoon commented Nov 23, 2016

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.

@tyoc213
Copy link
Author

tyoc213 commented Jul 27, 2017

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

@emoon
Copy link
Owner

emoon commented Jul 27, 2017

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

@emoon
Copy link
Owner

emoon commented Jul 27, 2017

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 :)

@Neopallium
Copy link
Contributor

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

@Neopallium
Copy link
Contributor

@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.

@emoon
Copy link
Owner

emoon commented Mar 28, 2019

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

@Neopallium
Copy link
Contributor

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.

@emoon
Copy link
Owner

emoon commented Mar 28, 2019

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

@Neopallium
Copy link
Contributor

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.

@emoon
Copy link
Owner

emoon commented Mar 28, 2019

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?

@Neopallium
Copy link
Contributor

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
Copy link
Contributor

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?

@emoon
Copy link
Owner

emoon commented Mar 28, 2019

Something like that yeah

@emoon
Copy link
Owner

emoon commented Mar 28, 2019

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.

@Neopallium
Copy link
Contributor

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.

@emoon
Copy link
Owner

emoon commented Mar 28, 2019

I see

@Neopallium
Copy link
Contributor

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.

@emoon
Copy link
Owner

emoon commented Mar 28, 2019

Yeah that might be a good idea

@Neopallium
Copy link
Contributor

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.

@emoon
Copy link
Owner

emoon commented Mar 28, 2019

Indeed. Sounds good 👍

@Boscop
Copy link
Contributor

Boscop commented Oct 27, 2020

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

@Neopallium
Copy link
Contributor

@Boscop I ran into those same TLS destructor issues over a year ago fungos/cr#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.

@xxshady
Copy link

xxshady commented Dec 18, 2024

I've made new tool that fixes thread-locals issue and more: https://github.com/xxshady/relib

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

5 participants