broke io::read/write out into non-buffering and buffering solutions

[rrichardson]

OK. Here is the pull request which addresses issues #45, #46, and #47. It is pretty large, the IoEventCtx plays a more important role in things, since re-registration is so common. I renamed to IoEventCtx and added the Token to it, to simplify the interface into the the register functions.

For an advanced example of how the event re-registration can actually help simplify a state machine, check out the revised test_echo_server.

• more tweaks to make the tests pass, also made the return type uint instead of () so the customer doesn't have to go looking through the buffer for answers
• addressing issues 45, 46, and 47. Added re-registration via an IoEventContext into the event_loop. Supports both edge triggered and level triggered setups.
• Changed * net::read and write to not use a loop.. added read_all and write_all to use them and perform the old behavior
• got all tests passing

[rrichardson]

I also added a close() function for TcpSocket, because sometime you want to close a socket without it going out of scope first :)

[ayosec]

I think that the self.desc should be set to something equivalent to -1.

A file descriptor number is reused by the kernel after closing it, so it is possible that a future read on a closed TcpSocket get data from another connection.

[rrichardson]

That's a good point. I'm not sure where this should happen, though because
net:: doesn't really know about file descriptors. Maybe it should go into
posix::
On Nov 15, 2014 9:20 AM, "Ayose Cazorla" notifications@github.com wrote:

In src/net.rs:

•    pub fn connect(&self, addr: &SockAddr) -> MioResult<()> {
  

•        debug!("socket connect; addr={}", addr);
  

•        // Attempt establishing the context. This may not complete immediately.
  

•        if try!(os::connect(&self.desc, addr)) {
  

•            // On some OSs, connecting to localhost succeeds immediately. In
  

•            // this case, queue the writable callback for execution during the
  

•            // next event loop tick.
  

•            debug!("socket connected immediately; addr={}", addr);
  

•        }
  

•        Ok(())
  

•    }
  

•    pub fn close(&self) -> MioResult<()> {
  

•      os::close(&self.desc)
  

I think that the self.desc should be set to something equivalent to -1.

A file descriptor number is reused by the kernel after closing it, so it
is possible that a future read on a closed TcpSocket get data from
another connection.

—
Reply to this email directly or view it on GitHub
https://github.com/carllerche/mio/pull/51/files#r20402565.

[ayosec]

Maybe, the safest option is to not provide a tcp_socket.close().

If TcpSocket is stored in another struct, the user can use something like Option<TcpSocket>, instead of just TcpSocket. Thus, the socket can be closed with self.foo = None

[rrichardson]

That seems like a lot of overhead and hassle for the purposes of avoiding accidentally re-using a socket that one has explicitly closed (e.g. probability near 0). I am for changing the fd to -2 in os::close() (such as posix)

[carllerche]

I'll review the PR in more detail today, but quick question. What is a case where you would want to call sock.close() vs. drop(sock) (or having close consume self)?

(I used to think the same re: having a close fn, but I changed my mind as of now, so if you have a more specific example, I would love to see it).

[rrichardson]

I've been thinking about this a lot today. I guess my problem with
languages like Java and Haskell was nondeterminism when things like
descriptors went out of scope, I wouldn't know exactly when close would get
called, so I would explicitly call it.

Rust doesn't suffer from this problem since drop() is called synchronously,
RAII style.

All of the examples I can think of where I'd want to force-close a bunch of
sockets, I'd probably just end up overwriting those slots in a vector.

So consider me convinced. I can ditch the close functions.
On Nov 15, 2014 3:51 PM, "Carl Lerche" notifications@github.com wrote:

I'll review the PR in more detail today, but quick question. What is a
case where you would want to call sock.close() vs. drop(sock) (or having
close consume self)?

(I used to think the same re: having a close fn, but I changed my mind as
of now, so if you have a more specific example, I would love to see it).

—
Reply to this email directly or view it on GitHub
#51 (comment).

[carllerche]

Alright, I'm not dead! I will go over this in detail today. Promise

[carllerche]

I pulled this in on master already.

[carllerche]

As we discussed in IRC. IoEventCtx would not be part of the public API in favor of passing the components directly:

pub fn register<H: IoHandle>(&mut self, io: &H, token: Token) -> MioResult<()>;
pub fn register_opt<H: IoHandle>(&mut self, io: &H, token: Token, interests: IoInterests, opts: PollOpt) -> MioResult<()>;
pub fn reregister<H: IoHandle>(&mut self, io: &H, token: Token, interests: IoInterests, opts: PollOpt) -> MioResult<()>;

(not 100% sure if reregister takes PollOpt)

[rrichardson]

I can't think of a case where I would want to register a function with the poller and not indicate at least Interest.
(In epoll, if you don't indicate EdgeTriggered it will default to Level)

[carllerche]

Would a good default for interests not just be all possible interests?

[rrichardson]

Not it's level triggered by default. It would spam the readable and
writable handlers.
On Nov 19, 2014 5:17 PM, "Carl Lerche" notifications@github.com wrote:

Would a good default for interests not just be all possible interests?

—
Reply to this email directly or view it on GitHub
#51 (comment).

[carllerche]

I would be OK w/ having the EventLoop::register require the interests. There should be some helpers though like Interest::all() or whatever.

[rrichardson]

SGTM.
On Nov 19, 2014 7:18 PM, "Carl Lerche" notifications@github.com wrote:

I would be OK w/ having the EventLoop::register require the interests.
There should be some helpers though like Interest::all() or whatever.

—
Reply to this email directly or view it on GitHub
#51 (comment).

[rrichardson]

superceded by #57