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

Generic Stream and Listener #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

youngspe
Copy link

I've taken a crack at combining the common functionality between tcp and unix into a generic Stream, Reader, Writer, and Listener. This should massively reduce the amount of duplicate code between those two modules. As far as I can tell, this introduces no breaking changes.

This implementation also exposes the generic versions so that a user of the crate can define their own streams and listeners. I have an example of this in action here, where I've written a wrapper around SslStream to run a simple HTTPS server with promises.

Criticism is welcome. I hope my ideas can be useful.

@dwrensha
Copy link
Owner

I plan to respond more fully once I've wrapped by head around this, but I just wanted to say right now: thanks for the pull request and thanks especially for the complete example! I have in fact often wondered what would be involved in integrating with an SSL library.

@dwrensha
Copy link
Owner

I really like how your patch allows users to define their own streams that can be registered with the event loop. However, I'm not sure that exposing details about mio is the best way to provide such functionality. For one thing, it seems like mio::Evented is a rather clumsy interface for allowing user-defined streams. Just look at all the boilerplate you had to write.

I think it would be better for us to define our own trait, like this:

#[cfg(unix)]
pub struct RawIoHandle(std::os::unix::io::RawFd);

#[cfg(windows)]
pub enum RawIoHandle {
  Socket(::std::os::windows::io::RawSocket),
  Handle(::std::os::windows::io::RawHandle),
  // ... other variants ?
}

pub trait AsRawIoHandle {
   fn as_raw_io_handle(&self) -> RawIoHandle;
}

Then we could have something like:

pub struct Stream<T> where T: std::io::Read + std::io::Write + AsRawIoHandle {
  //...
}

impl <T> Stream<T> {
   pub fn new(inner: T) -> Stream<T> {
     //...
  }
} 

impl <T> AsyncRead for Stream<T> {...}
impl <T> AsyncWrite for Stream<T> {...}

@dwrensha
Copy link
Owner

Another reason that I think we don't want to expose mio to users is that on Windows it might make a lot of sense to avoid using mio at all.

GJ I/O is completion-based, so it can likely be implemented directly on top of the low-level Windows API (which is completion-based) fairly easily. mio, on the other hand, is going through a lot of pain to contort Windows IOCP into a readiness model. GJ converts it right back into a completion model. It would be better to avoid these two layers of indirection!

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.

None yet

2 participants