-
Notifications
You must be signed in to change notification settings - Fork 135
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
Reflex does not remove file watches #13
Comments
Thanks, I'll look into it. |
@aktau Sorry for the long delay. You may be correct; I haven't looked at older fsnotify code. The newer code, at least, cleans the paths, so this wouldn't be a problem. I'm updating to However... I realized that the removal code is not at all correct. For one thing, it does not recursively remove the watches. For another, you can't even remove watches in fsnotify for files that have been deleted :( I've filed go-fsnotify/fsnotify#40 and go-fsnotify/fsnotify#41. So for right now, it's not even possible to remove watches. I'm removing the watch removal code completely for now. Hopefully I can soon find a workaround or patch fsnotify to make removing recursive watches possible. I've edited the title to reflect the current status of this issue. |
I'm not sure if this is related, but I do find reflex to be progressively slower over time of use.. and starts to happen after about 10 runs of reflex to rerun |
Hi @pkieltyka! Can you describe what you mean by slower? Does it just take longer to respond to file changes? Can you give your exact repro case (what reflex command you're running and what filesystem changes you're making)? Note that this bug would only be related if you're adding and removing directories. It might be best if you opened a new issue here with the repro info. |
While reading of the code I noticed:
This might not be entirely correct.
RemoveWatch
checks ifpath
exists in a map. Because it's been trimmed earlier (TrimPrefix
), it likely will not find it and just return an error. I believewatcher.RemoveWatch(e.Name)
would have a better chance of succeeding. Also perhaps checking the returned error. An error will be returned on many occasions though, because ifpath
is not a folder, a watch was never issued for it. SoRemoveWatch
will probably fail for those cases.I'm also not sure how some platforms handle it, but deleting a file might automatically delete the associated watcher (if any) on the kernel side. Calling
RemoveWatch
could still be beneficial though, to clean up the structure on the Go side.The text was updated successfully, but these errors were encountered: