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

Consider removing the watch when WatchDescriptor is dropped #74

Open
hannobraun opened this issue Aug 20, 2017 · 0 comments
Open

Consider removing the watch when WatchDescriptor is dropped #74

hannobraun opened this issue Aug 20, 2017 · 0 comments

Comments

@hannobraun
Copy link
Owner

Currently, a WatchDescriptor can exist pretty much independently of the actual inotify watch it represents. This means the user has to manually remove any watches (by calling Inotify::rm_watch) that they no longer need. If the user doesn't do this, this can result in lots of useless events being fired.

I'm not sure if this is something we can solve, or if this is one of those issues that has to be left to a higher-level library. Some thoughts:

  • Currently WatchDescriptor implements Clone and Copy. This would probably no longer be desirable, if we wanted it to implement Drop.
  • The Clone implementation could be kept, if WatchDescriptor did reference counting, for example using Rc. This would require a heap allocation per watch. As an application might watch thousands of files and directories, this seems like excessive overhead for a low-level wrapper.
  • If we allow only one WatchDescriptor instance per watch, then Event could no longer contain a WatchDescriptor. Maybe the current WatchDescriptor could be renamed to Watch, implement Drop, and Event would carry a WatchDescriptor whose sole purpose it was to be compared to a Watch.
  • What would happen if the user called Inotify::add_watch to modify an already existing watch? This can happen by calling add_watch for an already watched path, or even for a different path that refers to an already watched inode. If we naively created a new Watch in that case, then dropping that Watch would also remove the watch for the first Watch, if that was still around.

Of course all problems could be solved if Inotify did extensive internal bookkeeping on what watch descriptors are being used. That kind of overhead seems out of the question, especially since any higher-level library would probably want to duplicate all that work in a different way on top.

If this issue is going to be closed without a solution, the problem should be properly documented.

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

No branches or pull requests

1 participant