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

Update to mio 0.7, remove mio-extras #278

Merged
merged 2 commits into from
Jan 28, 2021
Merged

Conversation

roblabla
Copy link
Contributor

This MR updates mio to version 0.7 and removes mio-extras.

To replace the channel used by mio-extras, we use crossbeam_channel along with a mio::Waker that will be used to wake up the mio event loop whenever we send a message through the crossbeam channel.

Note that mio 0.7 has no level triggering (see tokio-rs/mio#928 ). As such, we need another solution for #267. What I implemented is simply looping over inotify.read_buffer until it returns an empty iterator, at which point we can safely go back to waiting.

Fixes #248

When INotify::read_buffer finally returns an empty iterator, we break
out of the loop. This should allow polling with edge triggering.
@0xpr03
Copy link
Member

0xpr03 commented Jan 26, 2021

Hey, thanks for the research and PR. Sadly we will need a solution for #267 first, otherwise this is a regression..

@roblabla
Copy link
Contributor Author

@0xpr03 this PR fixes #267 by repeatedly calling INotify::read_buffer, see 5c48373

Copy link
Member

@0xpr03 0xpr03 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As folks at tokio also approve the workaround I see no issues anymore with merging.

@0xpr03 0xpr03 requested a review from JohnTitor January 26, 2021 16:21
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for migrating! Looks great to me.

@JohnTitor JohnTitor merged commit 609c705 into notify-rs:main Jan 28, 2021
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

Successfully merging this pull request may close these issues.

upgrade to mio 0.7
3 participants