guard/rb-inotify

INotify::Notifier#process does not necessarily process all events

Closed this issue · 4 comments

README.md says of INotify::Notifier#process:

If you don't want infinite looping,
you can also block until there are available events,
process them all at once,
and then continue on your merry way

However, inotify(7) says:

The behavior when the buffer given to read(2) is too  small  to  return
information about the next event depends on the kernel version: in ker‐
nels before 2.6.21, read(2) returns 0;  since  kernel  2.6.21,  read(2)
fails with the error EINVAL.

Note that an inotify read(2) does not necessarily need to read all events from the queue to be successful; reading at least one event is sufficient. This is not accounted for in INotify::Notifier#read_events (called by #process), which will simply return all events from a successful read(2). The implementation of #process is therefore inconsistent with its description in README.md.

It is also worth noting that the RDoc for #read_events agrees with README.md about what the behaviour should be.

I am not raising a pull request yet because I don't know if this is a documentation bug (and the implementation is correct), or an implementation bug (and the description in README.md is the intended behaviour). Fixing the documentation is easy if that's the correct solution; if a code fix is needed, it's probably easiest to just poll #to_io within #read_events until there are no more events to read.

nex3 commented

The documentation is the intended behavior. Is there a way to make the documented behavior work without using #to_io, which is unavailable on JRuby?

I was going to suggest using FFI to call select(2) on the fd directly, but manipulating fd_set structures requires the use of C macros, which eliminates that idea.

The only other way I can think of at the moment is to call fcntl(2) to set the fd as O_NONBLOCK, then read(2) until getting EWOULDBLOCK, and call fcntl(2) again to unset O_NONBLOCK (so that #run doesn't busyloop indefinitely). It seems kind of clunky, but I can't think of a better way. What do you think?

nex3 commented

That's a little gross, but I think it's worth the nicer API.

It would be nice to just always have the FD be nonblocking, but that would screw with its other uses.

#to_io is avaialble on JRuby now. Feel free to submit a PR.