Update Mio to 0.7 #1767

kleimkuhler · 2019-11-12T19:20:47Z

Summary

This PR updates Tokio to Mio master. A version 0.7 has not been released
yet, but that is the intention of this change.

There should be no user-facing changes from this update. All changes are
confined to tokio::net with the majority taking place in
net::driver::reactor and net::util::PollEvented (now
net::util::IoResource). Therefore, no tests have been added or changed.

Details

`PollEvented` -> `IoResource`

With the change of mio::Evented -> mio::event::Source, it made sense to
change the Tokio wrapper as well. As this type is responsible for driving
all I/O resources, the name seemed to fit.

The internal API for IoResource have changed so that an interest/readiness
does not need to be passed in to the poll functions (poll_read_ready and
poll_write_ready). Both poll functions expand via poll_readiness! which is
responsible for driving the actual readiness of events on the I/O resource.

Readiness represents any cross-platform event that has occurred on the I/O
resource since the last poll. The possible values are: READABLE,
WRITEABLE, READ_CLOSED, and WRITE_CLOSED.

poll_readiness! will poll the I/O resource until there is a readiness event
that matches an event of interest. If there already is an event of interest
then it will return.

Finally, poll_readiness! does not actually implement the low-level polling
on the I/O resource. It dispatches to a Registration which changed mainly to
handle closed events.

Temporarily removed methods

The following TcpStream methods were removed due to changes in the Mio API.
They will be reintroduced in further changes that either implement these
directly in Tokio, or Mio introducing a TcpSocket builder type that can be
used by Tokio.

TcpStream::

connect_std
recv_buff_size
set_recv_buffer_size
send_buffer_size
set_send_buffer_size
keepalive
set_keepalive
linger
set_linger

`{AsyncRead, AsyncWrite}::poll_read_buf`

This method relies on the Bytes update so that IoSlice can be used. This
temporarily performs un-vectored reads and writes.

Signed-off-by: Kevin Leimkuhler [email protected]

tokio/src/net/driver/readiness.rs

kleimkuhler · 2019-11-13T05:44:25Z

This currently fails on Windows because of the use of mio-named-pipes which still depends on Mio 0.6. Will work on a fix for this, but those changes should be confined to tokio::process::windows

carllerche · 2019-11-13T05:53:07Z

tokio/src/net/driver/platform.rs

@@ -1,28 0,0 @@
-pub(crate) use self::sys::*;


Deleting this file makes me happy.

carllerche · 2019-11-13T05:56:30Z

tokio/src/net/driver/reactor/mod.rs

@@ -266,7 275,7 @@ impl Reactor {
 #[cfg(all(unix, not(target_os = "fuchsia")))]


We can delete any mention of fuchsia as there is no fuchsia support ATM.

carllerche · 2019-11-13T06:02:20Z

tokio/src/net/driver/registration.rs

+ // This consumes the current readiness state **except** for
+ // `READ_CLOSED` and `WRITE_CLOSED`. These are excluded because:
+ // - They are a final state and never transitioned out of
+ // - Both the read and the write directions should be able to observe


This line isn't really relevant anymore.

carllerche · 2019-11-13T06:20:43Z

tokio/src/net/util/io_resource.rs

+/// [`poll_read_ready`]: #method.poll_read_ready
+/// [`poll_write_ready`]: #method.poll_write_ready
+pub struct IoResource<S: mio::event::Source> {
+ io: Option<S>,


Since you are in here, can Option be removed?

It can! I was going to do it as part of this PR, but since IoResource::into_inner moves the value out, the Drop impl requires some additional handling to forget the value when this occurs. I can include it now, but had it as a note for a follow-up. It should be a fairly small change.

Thomasdezeeuw

I took a quick look and left some comments.

Overall I don't see many change that relate to using edge-triggers. Was Mio already using edge-trigger before? Last time I look was with futures 0.1, so its been a while and lots has changed.

Thomasdezeeuw · 2019-11-13T12:10:34Z

tokio/Cargo.toml

@@ -93,14 91,13 @@ iovec = "0.1"
 fnv = { version = "1.0.6", optional = true }
 lazy_static = { version = "1.0.2", optional = true }
 memchr = { version = "2.2", optional = true }
-mio = { version = "0.6.14", optional = true }
+mio = { git = "https://github.com/tokio-rs/mio", branch = "master", optional = true }


Might want to fix to a specific commit to avoid breaking changes.

Thomasdezeeuw · 2019-11-13T12:12:30Z

tokio/src/net/driver/reactor/mod.rs

 Ok(_) => {}
- Err(e) => return Err(e),
+ Err(ref e) if e.kind() == io::ErrorKind::Interrupted => {}


You should probably add a loop here, now you're continueing with no events.

Thomasdezeeuw · 2019-11-13T12:17:30Z

tokio/src/net/driver/reactor/mod.rs

@@ -321,7 330,7 @@ impl Handle {
 /// return immediately.
 fn wakeup(&self) {
 if let Some(inner) = self.inner() {
- inner.wakeup.set_readiness(mio::Ready::readable()).unwrap();
+ inner.waker.wake().expect("failed to wake reactor")


Don't you want to return the error here?

wakeup is only used in Handle::unpark which does not return anything. I think handling an error here could be considered, but not necessarily as part of this change. Similar to before, if there an unexpected reason why wakeup fails then a panic will happen.

Thomasdezeeuw · 2019-11-13T12:19:20Z

tokio/src/net/driver/reactor/mod.rs

 };

 waker.register(w);
- if readiness & ready.as_usize() != 0 {
+ if readiness & interest.as_usize() != 0 {
 waker.wake();


This call is now quite a bit more expansive then in Mio v0.6, since in v0.7 this is a system call. You might want to consider using a different mechanism for this.

Thomasdezeeuw · 2019-11-13T12:21:31Z

tokio/src/net/driver/readiness.rs

+const READABLE: usize = 0b0_01;
+const WRITABLE: usize = 0b0_10;
+const READ_CLOSED: usize = 0b0_0100;
+const WRITE_CLOSED: usize = 0b0_1000;


This is missing AIO (not supported), LIO (not supported), error and priority. This is the full list (from tokio-rs/mio#1145):

const READABLE: usize = 0b00_000_001; const WRITABLE: usize = 0b00_000_010; const AIO: usize = 0b00_000_100; const LIO: usize = 0b00_001_000; const ERROR: usize = 0b00_010_000; const READ_CLOSED: usize = 0b00_100_000; const WRITE_CLOSED: usize = 0b01_000_000; const PRIORITY: usize = 0b10_000_000;

It also just fits in 8 bits, if size is something to optimise for here.

Yep I left those out for now. Readable and writeable readiness were supported before, and now Mio offers the cross-platform closed readiness that allows this change to remove platform-specific behavior. If support for these platform specific events provides values I think it should be done in a PR that is focused on adding functionality and not just updating.

Thomasdezeeuw · 2019-11-13T12:27:38Z

tokio/src/net/util/io_resource.rs

+/// [`TcpListener`] implements poll_accept by using [`poll_read_ready`] and
+/// [`clear_read_ready`].
+///
+/// TODO


Couple TODOs left in these docs.

Signed-off-by: Kevin Leimkuhler <[email protected]>

…pdate Signed-off-by: Kevin Leimkuhler <[email protected]>

kleimkuhler · 2020-01-14T01:20:07Z

Closing as I'd like to keep this branch up to date as both libraries change towards their next big releases, and I will be making some temporary changes so that it stays compiling/testing on at least some platforms.

This also makes Mio an implementation detail, removing it from the public API. This is based on #1767.

monkeygroover · 2021-01-06T14:24:44Z

Hi QQ, set_keepalive is noted as being temporarily removed, has this subsequently moved elsewhere or is there an alternative way to set the socket option?

cheers

Rich

Darksonn · 2021-01-06T14:29:03Z

See more at #3082.

hawkw reviewed Nov 12, 2019

View reviewed changes

tokio/src/net/driver/readiness.rs Outdated Show resolved Hide resolved

kleimkuhler marked this pull request as ready for review November 13, 2019 05:23

kleimkuhler force-pushed the kleimkuhler/mio-update branch from 0263c98 to c89744e Compare November 13, 2019 05:33

kleimkuhler requested a review from carllerche November 13, 2019 05:40

carllerche reviewed Nov 13, 2019

View reviewed changes

tokio/src/net/driver/platform.rs Outdated

@@ -1,28 0,0 @@

pub(crate) use self::sys::*;

Copy link

Member

carllerche Nov 13, 2019

Choose a reason for hiding this comment

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

Deleting this file makes me happy.

carllerche reviewed Nov 13, 2019

View reviewed changes

carllerche assigned carllerche, ipetkov and Thomasdezeeuw Nov 13, 2019

Thomasdezeeuw reviewed Nov 13, 2019

View reviewed changes

kleimkuhler force-pushed the kleimkuhler/mio-update branch from 1fee31f to c89744e Compare November 13, 2019 21:13

kleimkuhler added 16 commits November 18, 2019 14:04

Need to resolve Poll::poll taking &mut self

90f8ebc

Signed-off-by: Kevin Leimkuhler <[email protected]>

Tests passing; lots of cleanup

f4cd344

Signed-off-by: Kevin Leimkuhler <[email protected]>

Documentation and cleanup needed

4f912f1

Signed-off-by: Kevin Leimkuhler <[email protected]>

Naming and docs

88dab3e

Signed-off-by: Kevin Leimkuhler <[email protected]>

Remove platform specific event handling

1a2a5a9

Signed-off-by: Kevin Leimkuhler <[email protected]>

Remove old code

3c20f33

Signed-off-by: Kevin Leimkuhler <[email protected]>

Add additional comments and cleanup

b256539

Signed-off-by: Kevin Leimkuhler <[email protected]>

Remove mio-uds from Cargo.toml

7141696

Signed-off-by: Kevin Leimkuhler <[email protected]>

Depend on Mio fix

381ebbd

Signed-off-by: Kevin Leimkuhler <[email protected]>

Fix readiness checks for process and handle interruption

1e6caeb

Signed-off-by: Kevin Leimkuhler <[email protected]>

Add Readiness documentation

5052bfa

Signed-off-by: Kevin Leimkuhler <[email protected]>

Stop point for draft PR

01841a3

Signed-off-by: Kevin Leimkuhler <[email protected]>

Change windows process to use IoResource

c9177e7

Signed-off-by: Kevin Leimkuhler <[email protected]>

Clippy and use mio master

d0391e3

Signed-off-by: Kevin Leimkuhler <[email protected]>

send_framed is flakey on master as well

e0000e9

Signed-off-by: Kevin Leimkuhler <[email protected]>

Remove PollEvented references

43b885e

Signed-off-by: Kevin Leimkuhler <[email protected]>

kleimkuhler added 5 commits November 18, 2019 14:06

Make Readiness functions associated consts

b0de5ec

Signed-off-by: Kevin Leimkuhler <[email protected]>

Fix doc example

79586ec

Signed-off-by: Kevin Leimkuhler <[email protected]>

Misc edits

b3b4b04

Signed-off-by: Kevin Leimkuhler <[email protected]>

Remove unnecessary readiness

16ab873

Signed-off-by: Kevin Leimkuhler <[email protected]>

Remove ready import after rebase

78c17df

Signed-off-by: Kevin Leimkuhler <[email protected]>

kleimkuhler force-pushed the kleimkuhler/mio-update branch from c89744e to 78c17df Compare November 18, 2019 22:20

kleimkuhler added 2 commits November 27, 2019 16:49

Merge remote-tracking branch 'upstream/master' into kleimkuhler/mio-u…

bc695c4

…pdate Signed-off-by: Kevin Leimkuhler <[email protected]>

Merge remote-tracking branch 'upstream/master' into kleimkuhler/mio-u…

ff08bef

…pdate Signed-off-by: Kevin Leimkuhler <[email protected]>

kleimkuhler closed this Jan 14, 2020

jmagnuson mentioned this pull request May 21, 2020

tokio: update to mio 0.7 #1190

Closed

carllerche mentioned this pull request Sep 28, 2020

io: update to Mio 0.7 #2893

Merged

2 tasks

carllerche added a commit that referenced this pull request Oct 2, 2020

io: update to Mio 0.7 (#2893)

1e585cc

This also makes Mio an implementation detail, removing it from the public API. This is based on #1767.

DavisNT mentioned this pull request Jan 19, 2021

Add running scripts on connect/disconnect alvr-org/ALVR#436

Merged

BarbossHack mentioned this pull request Jun 17, 2022

[libs] Add TcpStream::keepalive and TcpStream::set_keepalive rust-lang/rust#69774

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Mio to 0.7 #1767

Update Mio to 0.7 #1767

kleimkuhler commented Nov 12, 2019 •

edited

Loading

kleimkuhler commented Nov 13, 2019

carllerche Nov 13, 2019

carllerche Nov 13, 2019

carllerche Nov 13, 2019

carllerche Nov 13, 2019

kleimkuhler Nov 13, 2019

Thomasdezeeuw left a comment

Thomasdezeeuw Nov 13, 2019

Thomasdezeeuw Nov 13, 2019

Thomasdezeeuw Nov 13, 2019

kleimkuhler Nov 14, 2019

Thomasdezeeuw Nov 13, 2019

Thomasdezeeuw Nov 13, 2019

Thomasdezeeuw Nov 13, 2019

kleimkuhler Nov 14, 2019

Thomasdezeeuw Nov 13, 2019

kleimkuhler commented Jan 14, 2020

monkeygroover commented Jan 6, 2021

Darksonn commented Jan 6, 2021

		@@ -266,7 275,7 @@ impl Reactor {
		#[cfg(all(unix, not(target_os = "fuchsia")))]

Update Mio to 0.7 #1767

Update Mio to 0.7 #1767

Conversation

kleimkuhler commented Nov 12, 2019 • edited Loading

Summary

Details

PollEvented -> IoResource

Temporarily removed methods

{AsyncRead, AsyncWrite}::poll_read_buf

kleimkuhler commented Nov 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Thomasdezeeuw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kleimkuhler commented Jan 14, 2020

monkeygroover commented Jan 6, 2021

Darksonn commented Jan 6, 2021

kleimkuhler commented Nov 12, 2019 •

edited

Loading

`PollEvented` -> `IoResource`

`{AsyncRead, AsyncWrite}::poll_read_buf`