WatchDescriptor usage is not safe

[hannobraun]

From Stack Overflow:

There is nothing to ensure, that inotify watches are closed before closing the inotify descriptor. There is nothing to ensure that watch descriptors don't get reused behind your back (they can be reused after watch descriptor number wraps). Those issues probably won't bite you immediately, but… IMO, the entire inotify-rs should have been declared unsafe. It does not add any actual safety on top of inotify, just some sugar and pointless wrappers. It does not even implement Drop for watch descriptor!

[arsalan86]

I've been trying to wrap my had around #61, but I can't figure out how to implement a solution while keeping the low level binding/wrapper nature of this library intact. The wraparound issue is actually an Inotify issue, not an issue with this library.

On the other hand, I do feel that if the library doesn't implement a solution for this issue, it should at least expose it to the user space in some way.

Maybe this is beyond my current skill level, but I'm learning a fair deal trying to solve this.

[hannobraun]

I currently don't have a full grasp on these issues, and haven't thought of elegant solutions for any of them. If we can solve them without introducing too much overhead on top of inotify, I think we should.

I need to put more thought into this (and read up on the man pages!) before I can say more. Unless we find obvious solutions to those problems, it might be a good idea to bring the notify developers into the discussion, to figure out what should be solved in inotify-rs and what they handle in higher-level code anyway.

In any case, I think the documentation should be more explicit about this stuff. It should state that inotify-rs is a low-level wrapper, it should point to notify as a possibly better alternative for application developers, and it should explicitely list any inotify gotchas that are not being handled by inotify-rs.

[arsalan86]

Yes, the documentation should reflect this issue. As an aside, is it possible to implement compile time warnings in rust?

[hannobraun]

As an aside, is it possible to implement compile time warnings in rust?

Yes, that's possible by writing a compiler plugin. Clippy is a tool that does this.

[hannobraun]

The quoted text above contains three distinct complaints that I'm going to address separately.

There is nothing to ensure, that inotify watches are closed before closing the inotify descriptor.

Yes, and I don't know how there could be. WatchDescriptor exists independently of Inotify. We could change that by including a reference to Inotify in WatchDescriptor, but then the Inotify instance would be useless as long as there are WatchDescriptors referencing it. Since the intended use of inotify seems to be to keep watch descriptors around long-term and compare them with the watch descriptors from events, this is not an acceptable solution.

What we can do, though, is to prevent a WatchDescriptor from being used with any Inotify instance except the one it originated from. Pull request #67 already addressed most of this, and the solution I proposed in issue #60 would address the rest.

There is nothing to ensure that watch descriptors don't get reused behind your back (they can be reused after watch descriptor number wraps).

This is a legitimate concern. I've opend issue #73.

It does not even implement Drop for watch descriptor!

I'm not sure, but I don't think this would make any sense. I've recorded my thoughts in issue #74.

Since there are more focused issues now for each of the complaints, I'm closing this one.