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

Inotify::read_events does not read all events #156

Closed
jimblandy opened this issue Nov 4, 2020 · 0 comments
Closed

Inotify::read_events does not read all events #156

jimblandy opened this issue Nov 4, 2020 · 0 comments

Comments

@jimblandy
Copy link
Contributor

The documentation for Inotify::read_events says:

Returns any available events

Returns an iterator over all events that are currently available.

However, the function only reads one buffers' worth of data from the inotify file descriptor; if there are more events available than can fit in the buffer, the returned Events iterator returns None without trying to read any more from the file descriptor.

This causes the notify crate to delay reporting events. That crate registers its inotify file descriptor with mio::PollOpt::edge(), which requests edge-triggered notifications, as described here: https://docs.rs/mio/0.6.15/mio/struct.Poll.html#edge-triggered-and-level-triggered

If when the socket was registered with Poll, edge triggered events were requested, then the call to Poll::poll done in step 5 will (probably) hang despite there being another 1kb still present in the socket read buffer. The reason for this is that edge-triggered mode delivers events only when changes occur on the monitored Evented. So, in step 5 the caller might end up waiting for some data that is already present inside the socket buffer.

This means that notify::inotify::EventLoop::handle_inotify is indeed counting on inotify::Inotify::read_events to behave as documented: to fully consume all available data from the file descriptor. Indeed, if I change notify::inotify::EventLoop::new to request level-triggered events on inotify_fd, I get all my file creation notifications.

jimblandy added a commit to jimblandy/inotify that referenced this issue Nov 4, 2020
Fixes hannobraun#156.

Change the documentation of `Inotify::read_events` to match the actual behavior:
return an iterator over only as many events can fit in the supplied buffer, not
over any available events. Since this is a backwards-incompatible change, bump
the crate version number to 0.9.0.

It would be possible to make `Events` behave as documented, but it would need to
hold a strong reference to `fd` and a mutable reference to `buffer`, and it
seems awkward. The `notify` crate's use of `read_events` is easily switched by
requesting level-sensitive notification on the inotify file descriptor, which is
designed for exactly this sort of situation.
jimblandy added a commit to jimblandy/notify that referenced this issue Nov 4, 2020
The `inotify` crate's `Inotify::read_events` method does not read all available
events from inotify (see hannobraun/inotify-rs#156), only one buffer's worth. Using
level-triggered events tells Mio to report events on the inotify fd until all
events have been read.
jimblandy added a commit to jimblandy/notify that referenced this issue Nov 4, 2020
The `inotify` crate's `Inotify::read_events` method does not read all available
events from inotify (see hannobraun/inotify-rs#156), only one buffer's worth. Using
level-triggered events tells Mio to report events on the inotify fd until all
events have been read.

Fixes notify-rs#267.
jimblandy added a commit to jimblandy/notify that referenced this issue Nov 5, 2020
The `inotify` crate's `Inotify::read_events` method does not read all available
events from inotify (see hannobraun/inotify-rs#156), only one buffer's worth. Using
level-triggered events tells Mio to report events on the inotify fd until all
events have been read.

Fixes notify-rs#267.
jimblandy added a commit to jimblandy/notify that referenced this issue Nov 5, 2020
The `inotify` crate's `Inotify::read_events` method does not read all available
events from inotify (see hannobraun/inotify-rs#156), only one buffer's worth. Using
level-triggered events tells Mio to report events on the inotify fd until all
events have been read.

Fixes notify-rs#267.
jimblandy added a commit to jimblandy/notify that referenced this issue Nov 5, 2020
The `inotify` crate's `Inotify::read_events` method does not read all available
events from inotify (see hannobraun/inotify-rs#156), only one buffer's worth. Using
level-triggered events tells Mio to report events on the inotify fd until all
events have been read.

Fixes notify-rs#267.
jimblandy added a commit to jimblandy/notify that referenced this issue Nov 5, 2020
The `inotify` crate's `Inotify::read_events` method does not read all available
events from inotify (see hannobraun/inotify-rs#156), only one buffer's worth. Using
level-triggered events tells Mio to report events on the inotify fd until all
events have been read.

Fixes notify-rs#267.
jimblandy added a commit to jimblandy/notify that referenced this issue Nov 5, 2020
The `inotify` crate's `Inotify::read_events` method does not read all available
events from inotify (see hannobraun/inotify-rs#156), only one buffer's worth. Using
level-triggered events tells Mio to report events on the inotify fd until all
events have been read.

Fixes notify-rs#267.
jimblandy added a commit to jimblandy/notify that referenced this issue Nov 5, 2020
The `inotify` crate's `Inotify::read_events` method does not read all available
events from inotify (see hannobraun/inotify-rs#156), only one buffer's worth. Using
level-triggered events tells Mio to report events on the inotify fd until all
events have been read.

Fixes notify-rs#267.
JohnTitor pushed a commit to notify-rs/notify that referenced this issue Nov 6, 2020
The `inotify` crate's `Inotify::read_events` method does not read all available
events from inotify (see hannobraun/inotify-rs#156), only one buffer's worth. Using
level-triggered events tells Mio to report events on the inotify fd until all
events have been read.

Fixes #267.
JohnTitor pushed a commit to notify-rs/notify that referenced this issue Nov 6, 2020
…to v4) (#269)

The `inotify` crate's `Inotify::read_events` method does not read all available
events from inotify (see hannobraun/inotify-rs#156), only one buffer's worth. Using
level-triggered events tells Mio to report events on the inotify fd until all
events have been read.

Fixes #267.
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

1 participant