hannobraun / inotify-rs

Idiomatic inotify wrapper for the Rust programming language
ISC License
260 stars 65 forks source link

Async await support #135

Closed blckngm closed 1 year ago

blckngm commented 5 years ago

(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 commented 5 years ago

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

hannobraun commented 5 years ago

@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.)

blckngm commented 5 years ago

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.

hannobraun commented 5 years ago

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.

blckngm commented 5 years ago

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.

hannobraun commented 5 years ago

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

hannobraun commented 1 year ago

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