IO timeout usability improvements

[carllerche]

The current timeout API on the various stream implementations behaves as a deadline vs. a timeout setting. I propose the following.

• Rename set_timeout -> set_deadline. (as well as the read and write variants).
• Move set_deadline to a trait (for example: ReadDeadline and WriteDeadline)
• Provide TimedReader and TimedWriter that operate as follows:

/// Various network related helpers

use std::io::{Reader, Writer, IoResult};
use std::io::net::unix::UnixStream;
use util::Duration;

pub trait ReadDeadline {
    fn set_read_deadline(&mut self, timeout_ms: Option<u64>);
}

impl ReadDeadline for UnixStream {
    fn set_read_deadline(&mut self, timeout_ms: Option<u64>) {
        self.set_read_timeout(timeout_ms);
    }
}

pub trait WriteDeadline {
    fn set_write_deadline(&mut self, timeout_ms: Option<u64>);
}

impl WriteDeadline for UnixStream {
    fn set_write_deadline(&mut self, timeout_ms: Option<u64>) {
        self.set_write_timeout(timeout_ms);
    }
}

pub struct TimedReader<R> {
    reader: R,
    timeout_ms: u64
}

impl<R: Reader + ReadDeadline> TimedReader<R> {
    pub fn new(reader: R, timeout_ms: u64) -> TimedReader<R> {
        TimedReader {
            reader: reader,
            timeout_ms: timeout_ms
        }
    }
}

impl<R: Reader + ReadDeadline> Reader for TimedReader<R> {
    fn read(&mut self, buf: &mut [u8]) -> IoResult<uint> {
        self.reader.set_read_deadline(Some(self.timeout_ms));
        self.reader.read(buf)
    }
}

pub struct TimedWriter<W> {
    writer: W,
    timeout_ms: u64
}

impl<W: Writer + WriteDeadline> TimedWriter<W> {
    pub fn new(writer: W, timeout_ms: u64) -> TimedWriter<W> {
        TimedWriter {
            writer: writer,
            timeout_ms: timeout_ms
        }
    }
}

impl<W: Writer + WriteDeadline> Writer for TimedWriter<W> {
    fn write(&mut self, buf: &[u8]) -> IoResult<()> {
        self.writer.set_write_deadline(Some(self.timeout_ms));
        self.writer.write(buf)
    }
}

cc @alexcrichton

[tomaka]

Move set_deadline to a trait (for example: ReadDeadline and WriteDeadline)

And similarly a AcceptDeadline

[brson]

We also probably want these to not take integer milliseconds but some kind of time type.

[conradkleinespel]

Would love to see this implemented too. Anyway I can help?

Just for the sake of completeness, would be good to add support for this to std::io::net::tcp::TcpStream and std::io::net::udp::UdpSocket.

[nodakai]

(Sorry I posted my comment on a wrong tree before.) I did a quick search for candidates for updating to use Duration instead of number of milliseconds etc. How many of them can be updated to use Duration without adding more dependency? I really want nice and stable API on I/O timeout.

$ ack --ignore-dir llvm --ignore-dir test 'time.*u64' src/
src/libtime/lib.rs
71:        pub fn mach_absolute_time() -> u64;
145:        let ns_since_1601 = ((time.dwHighDateTime as u64 << 32) |
146:                             (time.dwLowDateTime  as u64 <<  0)) / 10;
175:pub fn precise_time_ns() -> u64 {
179:    fn os_precise_time_ns() -> u64 {
195:    fn os_precise_time_ns() -> u64 {
204:            time * TIMEBASE.numer as u64 / TIMEBASE.denom as u64
209:    fn os_precise_time_ns() -> u64 {

src/libcollections/vec.rs
2390:        b.bytes = (times * src_len) as u64;

src/librbml/io.rs
187:        b.bytes = (times * len) as u64;

src/libnative/io/file_windows.rs
216:    fn set_timeout(&mut self, _t: Option<u64>) {}
217:    fn set_read_timeout(&mut self, _t: Option<u64>) {}
218:    fn set_write_timeout(&mut self, _t: Option<u64>) {}
480:        created: stat.st_ctime as u64,
481:        modified: stat.st_mtime as u64,
482:        accessed: stat.st_atime as u64,
510:pub fn utime(p: &CString, atime: u64, mtime: u64) -> IoResult<()> {

src/libnative/io/pipe_windows.rs
270:    pub fn connect(addr: &CString, timeout: Option<u64>) -> IoResult<UnixStream> {
546:    fn set_timeout(&mut self, timeout: Option<u64>) {
551:    fn set_read_timeout(&mut self, timeout: Option<u64>) {
554:    fn set_write_timeout(&mut self, timeout: Option<u64>) {
736:    fn set_timeout(&mut self, timeout: Option<u64>) {

src/libnative/io/process.rs
127:    fn set_timeout(&mut self, timeout: Option<u64>) {

src/libnative/io/util.rs
57:pub fn ms_to_timeval(ms: u64) -> libc::timeval {
64:pub fn ms_to_timeval(ms: u64) -> libc::timeval {
104:                       timeout_ms: u64) -> IoResult<()> {
151:             timeout: u64) -> libc::c_int {
164:             timeout: u64) -> libc::c_int {

src/libnative/io/mod.rs
167:                   timeout: Option<u64>)
193:                    timeout: Option<u64>) -> IoResult<Box<rtio::RtioPipe + Send>> {
256:    fn fs_utime(&mut self, src: &CString, atime: u64,
257:                mtime: u64) -> IoResult<()> {

src/libnative/io/net.rs
239:                   timeout: Option<u64>) -> IoResult<TcpStream> {
380:    fn set_timeout(&mut self, timeout: Option<u64>) {
385:    fn set_read_timeout(&mut self, timeout: Option<u64>) {
388:    fn set_write_timeout(&mut self, timeout: Option<u64>) {
648:    fn set_timeout(&mut self, timeout: Option<u64>) {
866:    fn set_timeout(&mut self, timeout: Option<u64>) {
871:    fn set_read_timeout(&mut self, timeout: Option<u64>) {
874:    fn set_write_timeout(&mut self, timeout: Option<u64>) {

src/libnative/io/pipe_unix.rs
81:           timeout: Option<u64>) -> IoResult<Inner> {
126:                   timeout: Option<u64>) -> IoResult<UnixStream> {
196:    fn set_timeout(&mut self, timeout: Option<u64>) {
201:    fn set_read_timeout(&mut self, timeout: Option<u64>) {
204:    fn set_write_timeout(&mut self, timeout: Option<u64>) {
307:    fn set_timeout(&mut self, timeout: Option<u64>) {

src/libnative/io/file_unix.rs
183:    fn set_timeout(&mut self, _t: Option<u64>) {}
184:    fn set_read_timeout(&mut self, _t: Option<u64>) {}
185:    fn set_write_timeout(&mut self, _t: Option<u64>) {}
446:    fn mktime(secs: u64, nsecs: u64) -> u64 { secs * 1000 + nsecs / 1000000 }
462:        created: mktime(stat.st_ctime as u64, stat.st_ctime_nsec as u64),
463:        modified: mktime(stat.st_mtime as u64, stat.st_mtime_nsec as u64),
464:        accessed: mktime(stat.st_atime as u64, stat.st_atime_nsec as u64),
494:pub fn utime(p: &CString, atime: u64, mtime: u64) -> IoResult<()> {

src/libstd/io/fs.rs
805:pub fn change_file_times(path: &Path, atime: u64, mtime: u64) -> IoResult<()> {

src/libstd/io/net/udp.rs
186:    pub fn set_timeout(&mut self, timeout_ms: Option<u64>) {
194:    pub fn set_read_timeout(&mut self, timeout_ms: Option<u64>) {
202:    pub fn set_write_timeout(&mut self, timeout_ms: Option<u64>) {

src/libstd/io/net/tcp.rs
118:            io.tcp_connect(addr, Some(timeout.num_milliseconds() as u64)).map(TcpStream::new)
228:    pub fn set_timeout(&mut self, timeout_ms: Option<u64>) {
245:    pub fn set_read_timeout(&mut self, timeout_ms: Option<u64>) {
272:    pub fn set_write_timeout(&mut self, timeout_ms: Option<u64>) {
444:    pub fn set_timeout(&mut self, ms: Option<u64>) { self.obj.set_timeout(ms); }

src/libstd/io/net/unix.rs
77:            let s = io.unix_connect(&path.to_c_str(), Some(timeout.num_milliseconds() as u64));
109:    pub fn set_timeout(&mut self, timeout_ms: Option<u64>) {
117:    pub fn set_read_timeout(&mut self, timeout_ms: Option<u64>) {
125:    pub fn set_write_timeout(&mut self, timeout_ms: Option<u64>) {
212:    pub fn set_timeout(&mut self, timeout_ms: Option<u64>) {

src/libstd/io/process.rs
539:    pub fn set_timeout(&mut self, timeout_ms: Option<u64>) {

src/libstd/io/mem.rs
564:        b.bytes = (times * len) as u64;

src/librustrt/rtio.rs
192:                   timeout: Option<u64>) -> IoResult<Box<RtioTcpStream + Send>>;
200:                    timeout: Option<u64>) -> IoResult<Box<RtioPipe + Send>>;
224:    fn fs_utime(&mut self, src: &CString, atime: u64, mtime: u64) ->
248:    fn set_timeout(&mut self, timeout: Option<u64>);
264:    fn set_timeout(&mut self, timeout_ms: Option<u64>);
265:    fn set_read_timeout(&mut self, timeout_ms: Option<u64>);
266:    fn set_write_timeout(&mut self, timeout_ms: Option<u64>);
290:    fn set_timeout(&mut self, timeout_ms: Option<u64>);
291:    fn set_read_timeout(&mut self, timeout_ms: Option<u64>);
292:    fn set_write_timeout(&mut self, timeout_ms: Option<u64>);
318:    fn set_timeout(&mut self, timeout: Option<u64>);
328:    fn set_timeout(&mut self, timeout_ms: Option<u64>);
329:    fn set_read_timeout(&mut self, timeout_ms: Option<u64>);
330:    fn set_write_timeout(&mut self, timeout_ms: Option<u64>);
339:    fn set_timeout(&mut self, timeout: Option<u64>);

src/librustuv/uvio.rs
143:    fn tcp_connect(&mut self, addr: rtio::SocketAddr, timeout: Option<u64>)
259:    fn fs_utime(&mut self, path: &CString, atime: u64, mtime: u64)
293:    fn unix_connect(&mut self, path: &CString, timeout: Option<u64>)

src/librustuv/timeout.rs
112:    pub fn set_timeout(&mut self, ms: Option<u64>,
235:        mut self, obj: T, timeout: Option<u64>, io: &mut UvIoFactory,

src/librustuv/timer.rs
53:    pub fn start(&mut self, f: uvll::uv_timer_cb, msecs: u64, period: u64) {

src/librustuv/file.rs
248:    pub fn utime(loop_: &Loop, path: &CString, atime: u64, mtime: u64)
276:        fn to_msec(stat: uvll::uv_timespec_t) -> u64 {

src/librustuv/process.rs
276:    fn set_timeout(&mut self, timeout: Option<u64>) {

src/librustuv/net.rs
202:                   timeout: Option<u64>) -> Result<TcpWatcher, UvError> {
305:    fn set_timeout(&mut self, timeout: Option<u64>) {
310:    fn set_read_timeout(&mut self, ms: Option<u64>) {
322:    fn set_write_timeout(&mut self, ms: Option<u64>) {
477:    fn set_timeout(&mut self, ms: Option<u64>) {
793:    fn set_timeout(&mut self, timeout: Option<u64>) {
798:    fn set_read_timeout(&mut self, ms: Option<u64>) {
820:    fn set_write_timeout(&mut self, ms: Option<u64>) {

src/librustuv/pipe.rs
89:    pub fn connect(io: &mut UvIoFactory, name: &CString, timeout: Option<u64>)
175:    fn set_timeout(&mut self, timeout: Option<u64>) {
180:    fn set_read_timeout(&mut self, ms: Option<u64>) {
192:    fn set_write_timeout(&mut self, ms: Option<u64>) {
315:    fn set_timeout(&mut self, ms: Option<u64>) {

[aturon]

cc me

[murarth]

If no one else is working on this, I'll take a stab at it.

[thestinger]

It's a bad time to start working on this because nearly all of this stuff is going to get an overhaul. It would make the most sense to implement it as part of a major rewrite.

[conradkleinespel]

@thestinger Do you know if this overhaul is happening before v1? This issue has been open for a while now.

[aturon]

@conradkleinespel

The overhaul is part of the runtime removal project. The first step is complete: removing rustuv, rtio, and the runtime scheduling abstractions has all landed. We're now revisiting the APIs previously built on the runtime system, including thread-local storage, sync, the std::task module, and afterwards, std::io and std::os.

Please let me know if you would like to be involved in this effort!

[nodakai]

@aturon I'm very interested in this, partily because I frequently write networking code with timeout in C++ and am hoping Rust to become its superiror alternative.

But I'm concerned about removal of libtime from the stdlib. Pthread API with timeout are all based on absolute time which is only offered by libtime right now rather than relative time offered by std::duration. The design decision behind Pthread disagrees with Alex's sync-rs which is based on std::duration.

[aturon]

Thanks @nodakai. (And @conradkleinespel, I see you offered to help a little ways back).

I will be in touch ASAP about the plans here and ways that both of you might be able to help with design or implementation work.

As far as time goes, the reason libtime was moved out of the distro is that the API has gotten relatively little attention and likely needs substantial revision before moving into std. (Note that Duration has its own problems, as well.)

In the 1.0 beta timeframe, perhaps it is best to not stabilize methods taking a timeout (i.e., leave them as #[unstable]; we may want to keep our options open here and put more effort into the underlying time representation.

I will discuss this with the team and let you know what the best avenue forward looks to be.