inotify-rs icon indicating copy to clipboard operation
inotify-rs copied to clipboard

Async await support

Open blckngm opened this issue 6 years ago • 6 comments

(This includes changes from #134.)

This adds an async Inotify type. It has an async fn read_events (close #121):

    pub async fn read_events<'a>(&mut self, buffer: &'a mut [u8])
                           -> io::Result<Events<'a>>

that is basically the same as the original read_events function, but awaits on the read:

    {
        let num_bytes = self.fd.read(buffer).await?;

        if num_bytes == 0 {
            return Err(
                io::Error::new(
                    io::ErrorKind::UnexpectedEof,
                    "`read` return `0`, signaling end-of-file"
                )
            );
        }

        Ok(Events::new(Arc::downgrade(self.fd.get_ref()), buffer, num_bytes))
    }

This addresses the efficiency concern in #112: no additional allocation is required.

blckngm avatar Sep 18 '19 06:09 blckngm

This type also will not have the problems described in #111.

blckngm avatar Sep 18 '19 06:09 blckngm

@sopium Thank you, this looks awesome. I haven't taken a close look yet, but I will once #134 is closer to being merged (feel free to remind me, if I forget).

Once this is merged, do you think there's any reason to keep the original blocking API around?

cc @hawkw (I already pinged you on #134. I figured maybe you're interested in this one too.)

hannobraun avatar Sep 18 '19 15:09 hannobraun

I tried to modify the Inotify type to make it support both async and blockink API. I think it can work.

The trick is to use a fake PollEvented wrapper when the async-await feature is not enabled.

blckngm avatar Sep 19 '19 07:09 blckngm

Interesting. What if we removed the async-await feature and included that by default, would there be a reason to keep the blocking API? Not sure how the new futures and async/await work in detail, but I think I remember that the older futures hat a wait method that could be used to block on a future. Maybe something like that could be used to completely replace the blocking API.

Not sure if that's the right approach. Just wondering.

hannobraun avatar Sep 20 '19 05:09 hannobraun

Interesting. What if we removed the async-await feature and included that by default, would there be a reason to keep the blocking API? Not sure how the new futures and async/await work in detail, but I think I remember that the older futures hat a wait method that could be used to block on a future. Maybe something like that could be used to completely replace the blocking API.

Not sure if that's the right approach. Just wondering.

It should be possible: you can create a current_thread runtime and block_on the future.

But it's quite inefficient, a new runtime need to be created for every blocking read call.

And it means that tokio is always needed. Quite heavy a dependency when you only want to use blocking IO.

blckngm avatar Sep 20 '19 06:09 blckngm

Sounds like it make sense to keep the traditional blocking API then. Thanks for the explanation!

hannobraun avatar Sep 23 '19 05:09 hannobraun

This pull request is very old, and async/await support has been available in inotify-rs for a long time now. Closing.

hannobraun avatar May 25 '23 08:05 hannobraun