Impl poll_readable and poll_writable.

[dvc94ch]

pub struct ListenStream {
    listener: Async<TcpListener>,
}

impl Stream for ListenStream {
    type Item = Result<Async<TcpStream>, std::io::Error>;

    fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context) -> Poll<Option<Self::Item>> {
        loop {
            match self.listener.accept().now_or_never() {
                Some(Ok((stream, _))) => {
                    return Poll::Ready(Some(Ok(stream)));
                }
                Some(Err(err)) => {
                    return Poll::Ready(Some(Err(err)));
                }
                None => {
                    match self.listener.poll_readable(cx) {
                        Poll::Ready(Ok(())) => continue,
                        Poll::Pending => return Poll::Pending,
                        Poll::Ready(Err(err)) => return Poll::Ready(Some(Err(err))),
                    }
                }
            }
        }
    }
}

Closes #36

[ghost]

Thanks for the PR! I have only glossed over it now but will come back to it later. Just leaving a thought so I don't forget...

I think calls to poll_readable()/poll_writable() should return Poll::Ready if (and only if) the reactor has delivered an event since the last call to the same method. Do you agree?

Secondly, I'm not sure if poll_readable()/poll_writable() should take Pin<&mut Self> or &mut self as an argument. It seems the futures create always uses Pin<&mut Self> for everything, so I wonder if are any potential problems with &mut self. If we're not sure, perhaps Pin<&mut Self> is the safer bet. @taiki-e, do you perhaps have an opinion?

[dvc94ch]

I think calls to poll_readable()/poll_writable() should return Poll::Ready if (and only if) the reactor has delivered an event since the last call to the same method. Do you agree?

Not sure if it makes a large difference, but essentially as long as the stream is readable it should return Poll::Ready I guess.

The reason I went with &mut is to avoid having to mess around with pin_mut!.

[ghost]

The reason I went with &mut is to avoid having to mess around with pin_mut!.

Not sure if that makes it any better, but you can also use Pin::new() for !Unpin types (and most, if not all, will be in this case).

[dvc94ch]

readable and writable return !Unpin futures, so by extension any caller needs to use pin_mut!.

[taiki-e]

Secondly, I'm not sure if poll_readable()/poll_writable() should take Pin<&mut Self> or &mut self as an argument. It seems the futures create always uses Pin<&mut Self> for everything, so I wonder if are any potential problems with &mut self. If we're not sure, perhaps Pin<&mut Self> is the safer bet.

Async always implements Unpin, so it shouldn't need self: Pin. (self: Pin is only needed in cases where type is ?Unpin because Self: Unpin is needed to call a &mut self method while pinned.)

[taiki-e]

It is not good to create and destruct a new future each time poll is called. The state of the future will be reset each time.

[dvc94ch]

I understand, but I don't see a different solution without changing the api to be poll based instead of async.

[taiki-e]

Yeah, I thought we can do that by splitting Source::ready into fn poll_ready that returns Poll and async fn ready that calls poll_fn(|cx| self.poll_ready(cx, dir)), but it seems difficult as ready takes &self.

So, I think the solution (at this time) is probably not to provide poll_{readable, writable} on the async-io side, but to store the future returned by {readable, writable} on the user side.

[ghost]

I can help with this tomorrow :)

[dvc94ch]

Fyi storing the future doesn't work, at least not without unsafe code as the future takes &self, you'd need to construct a self referential struct.