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

Error Creating PollEvented for RawFd #2413

Closed
dbcfd opened this issue Apr 17, 2020 · 7 comments · Fixed by #2419
Closed

Error Creating PollEvented for RawFd #2413

dbcfd opened this issue Apr 17, 2020 · 7 comments · Fixed by #2419
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-io Module: tokio/io

Comments

@dbcfd
Copy link
Contributor

dbcfd commented Apr 17, 2020

Version

0.2.18

Platform

19.4.0 Darwin Kernel Version 19.4.0: Wed Mar 4 22:28:40 PST 2020; root:xnu-6153.101.6~15/RELEASE_X86_64 x86_64

Description

I am attempting to create PollEvented from a RawFd, but it looks to be getting an invalid token.

[2020-04-17T17:27:17Z TRACE mio::sys::unix::kqueue] registering; token=Token(34359738368); interests=Readable | Writable | Error | Hup

This is leading to an IO Error for invalid argument

Code occurring inside an async context:

let mio_ev = mio::unix::EventedFd(&args.fd);
debug!("Create event for {:?}", mio_ev);
let ev = tokio::io::PollEvented::new(mio_ev).map_err(|e| {
   error!("Failed to create evented");
   Error::Io(e)
})?;

When I create a mio event manually, I do not have an issue.

let ev = mio::unix::EventedFd(&self.fd);
let ready = mio::Ready::readable();
let opts = mio::PollOpt::edge();
ev.register(&self.poll, mio::Token(0), ready, opts).map_err(Error::Io)?;
dbcfd pushed a commit to protectwise/pcap-async that referenced this issue Apr 17, 2020
Convert to a future that returns when packets are available, up to a specified
buffering time limit.

Currently using mio due to tokio-rs/tokio#2413
@kleimkuhler
Copy link
Contributor

@dbcfd Do you have any additional output from the error? Token is just a wrapped usize so that value should be okay. Also do you have a way to reproduce this?

@Darksonn Darksonn added A-tokio Area: The main tokio crate C-bug Category: This is a bug. S-waiting-on-author Status: awaiting some action (such as code changes) from the PR or issue author. labels Apr 18, 2020
@dbcfd
Copy link
Contributor Author

dbcfd commented Apr 19, 2020

Not a way to reproduce easily, but a way to fix.

PollEvented registers both read and write, but not all file descriptors are read and write. Providing a method to register a PollEvented with just read prevents this error from happening.

index d8535d9a..1521e245 100644
--- a/tokio/src/io/driver/mod.rs
+++ b/tokio/src/io/driver/mod.rs
@@ -241,6 +241,10 @@ impl Inner {
     ///
     /// The registration token is returned.
     pub(super) fn add_source(&self, source: &dyn Evented) -> io::Result<Address> {
+        self.add_source_with_ready(source, mio::Ready::all())
+    }
+
+    pub(super) fn add_source_with_ready(&self, source: &dyn Evented, ready: mio::Ready) -> io::Result<Address> {
         let address = self.io_dispatch.alloc().ok_or_else(|| {
             io::Error::new(
                 io::ErrorKind::Other,
@@ -253,7 +257,7 @@ impl Inner {
         self.io.register(
             source,
             mio::Token(address.to_usize()),
-            mio::Ready::all(),
+            ready,
             mio::PollOpt::edge(),
         )?;

diff --git a/tokio/src/io/poll_evented.rs b/tokio/src/io/poll_evented.rs
index 298e6e58..19b56019 100644
--- a/tokio/src/io/poll_evented.rs
+++ b/tokio/src/io/poll_evented.rs
@@ -175,14 +175,18 @@ where
     /// from a future driven by a tokio runtime, otherwise runtime can be set
     /// explicitly with [`Handle::enter`](crate::runtime::Handle::enter) function.
     pub fn new(io: E) -> io::Result<Self> {
-        let registration = Registration::new(&io)?;
+        PollEvented::new_with_ready(io, mio::Ready::all())
+    }
+
+    pub fn new_with_ready(io: E, ready: mio::Ready) -> io::Result<Self> {
+        let registration = Registration::new_with_ready(&io, ready)?;
         Ok(Self {
             io: Some(io),
             inner: Inner {
                 registration,
                 read_readiness: AtomicUsize::new(0),
                 write_readiness: AtomicUsize::new(0),
-            },
+            }
         })
     }

diff --git a/tokio/src/io/registration.rs b/tokio/src/io/registration.rs
index 4df11999..425ed2f5 100644
--- a/tokio/src/io/registration.rs
+++ b/tokio/src/io/registration.rs
@@ -63,12 +63,19 @@ impl Registration {
     /// from a future driven by a tokio runtime, otherwise runtime can be set
     /// explicitly with [`Handle::enter`](crate::runtime::Handle::enter) function.
     pub fn new<T>(io: &T) -> io::Result<Registration>
+    where
+        T: Evented,
+    {
+        Registration::new_with_ready(io, mio::Ready::all())
+    }
+
+    pub fn new_with_ready<T>(io: &T, ready: mio::Ready) -> io::Result<Registration>
     where
         T: Evented,
     {
         let handle = Handle::current();
         let address = if let Some(inner) = handle.inner() {
-            inner.add_source(io)?
+            inner.add_source_with_ready(io, ready)?
         } else {
             return Err(io::Error::new(
                 io::ErrorKind::Other,

I then create PollEvented with tokio::io::PollEvented::new_with_ready(ev, mio::Ready::readable()) and there is no error.

@dbcfd
Copy link
Contributor Author

dbcfd commented Apr 19, 2020

I believe the PR referenced above can verify the fix with sudo cargo test stream::tests::packets_from_lookup -- --exact.

@Ralith Ralith removed the S-waiting-on-author Status: awaiting some action (such as code changes) from the PR or issue author. label Apr 20, 2020
dbcfd pushed a commit to dbcfd/tokio that referenced this issue Apr 20, 2020
Add additional methods to allow PollEvented to be created with an appropriate
`mio::Ready` state so that it can be properly registered with the reactor.
dbcfd added a commit to dbcfd/tokio that referenced this issue Apr 20, 2020
Add additional methods to allow PollEvented to be created with an appropriate
`mio::Ready` state so that it can be properly registered with the reactor.
@Darksonn Darksonn added the M-io Module: tokio/io label Apr 20, 2020
dbcfd added a commit to protectwise/pcap-async that referenced this issue Apr 22, 2020
* No Delay

Convert to a future that returns when packets are available, up to a specified
buffering time limit.

Currently using mio due to tokio-rs/tokio#2413
dbcfd added a commit to dbcfd/tokio that referenced this issue Apr 27, 2020
Add additional methods to allow PollEvented to be created with an appropriate
`mio::Ready` state so that it can be properly registered with the reactor.
dbcfd added a commit to dbcfd/tokio that referenced this issue Apr 29, 2020
Add additional methods to allow PollEvented to be created with an appropriate
`mio::Ready` state so that it can be properly registered with the reactor.
@carllerche
Copy link
Member

What type of resource does the FD reference?

@dbcfd
Copy link
Contributor Author

dbcfd commented Apr 29, 2020

In this case it's an interface opened by libpcap for packet capture.

@carllerche
Copy link
Member

@dbcfd could you help provide an example of how to repro? I would like to see the steps to get the RawFd in this case (syscalls, ...). I think we need to move away from PollEvented if we want to support uring.

@dbcfd
Copy link
Contributor Author

dbcfd commented May 2, 2020

  1. Create a handle for receiving packets, can use something like https://github.com/jmmk/rustcap/tree/master/pcap-sys
  2. Get the raw file descriptor from that handle (https://www.tcpdump.org/manpages/pcap_get_selectable_fd.3pcap.html)
  3. Attempt to register a mio RawFd using that raw file descriptor. This will fail.

Darksonn pushed a commit that referenced this issue May 11, 2020
Add additional methods to allow PollEvented to be created with an appropriate
mio::Ready state, so that it can be properly registered with the reactor.

Fixes #2413
jensim pushed a commit to jensim/tokio that referenced this issue Jun 7, 2020
Add additional methods to allow PollEvented to be created with an appropriate
mio::Ready state, so that it can be properly registered with the reactor.

Fixes tokio-rs#2413
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-io Module: tokio/io
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants