feat(client, server) make timers generic

[Robert-Cunningham]

Hi Sean,

I'm working through the timer series of issues (#2847, #2846, #2848, #2857) right now. This PR isn't (near) done, but I wanted to do a midway check-in and see if you agree with the design decisions, or have any feedback that I can integrate now, relatively early on in the process.

In particular, I'd appreciate any broad design feedback you have on rt.rs or tim.rs.

Otherwise I'll continue moving forwards with this series of issues!

Thanks for your work on hyper!

[Robert-Cunningham]

One note: pause, advance and resume from tokio::time control only tokio::time::Instant, not std::time::Instant. If we want to retain the ability to use functions like tokio::time::pause, we need to somehow make all the Instant objects tokio::time::Instant (or potentially any other Instant-ish type for a different library) when the user uses Tokio timers.

If we go that route, I believe we'd need to add functions like Instant::now() to the Timer trait and then and pass Timers around to anywhere we need to call Instant::now().

The alternative is just to use std::time::Instant everywhere and choose not to support functions like tokio::time::pause().

I suspect allowing changes to our Instant type is not worth the additional complexity, but let me know if you think otherwise.

[seanmonstar]

Yea I don't want to depend directly on tokio::time::Instant. It might make sense to eventually allow the trait Timer to tell us what the time is, to allow mocking... But I'd probably start with having whatever tests be commented out with a TODO to figure them out later.

[Robert-Cunningham]

Alright, I'm now working through the last major hiccup in this PR--would appreciate some guidance here if possible (potentially from @seanmonstar ?).

I have a trait Timer mirroring tokio::time::* with functions like sleep and reset. The last function I need is in Timer is timeout<T>, but I can't include it directly without making Timer object-unsafe. (If Timer becomes object-unsafe, we have to pass it as a generic all over the code base, which I tried before and quickly becomes disgusting.)

I'm trying to cook up a function signature which will allow me to include timeout<T> in the Timer trait, something like fn timeout(&self, duration: Duration, future: Box<dyn Future<Output = Box<dyn Any>>>). Then inside the function I need to do something like tokio::time::timeout(duration, *future), but this doesn't work because future isn't Sized (makes sense).

Do you have any idea how I can include or wrap something similar to tokio::time::timeout<T> in an object-safe Timer trait?

[ibraheemdev]

@Robert-Cunningham you can write your own timeout function that wraps sleep:

pub fn timeout<F>(duration: Duration, future: F) -> Timeout<F>
where
    F: Future,
{
    Timeout {
        future,
        sleep: sleep(duration),
    }
}


pin_project_lite::pin_project! {
    pub struct Timeout<F> {
        #[pin]
        future: F,
        sleep: Box<dyn Sleep>
    }
}

impl<F> Future for Timeout<F>
where
    F: Future,
{
    type Output = Result<F::Output, TimedOut>;

    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
        let mut this = self.project();

        if let Poll::Ready(v) = this.future.poll(cx) {
            return Poll::Ready(Ok(v));
        }

        match Pin::new(&mut this.sleep).poll(cx) {
            Poll::Ready(()) => Poll::Ready(Err(TimedOut)),
            Poll::Pending => Poll::Pending,
        }
    }
}

[Robert-Cunningham]

Thanks a bunch @ibraheemdev; your answer was instructive for me. With that, I think this PR is ready for review (from @seanmonstar, I think?). Please let me know if I need to do anything else before review; this is my first contribution to hyper.

Once this PR is merged, I'll open a PR that adds the same TokioTimer to hyper-util.

[seanmonstar]

Thanks so much for working on this! I'm at RustConf today, but I'll take a look beginning of next week <3

[Robert-Cunningham]

Ah, thanks for your feedback--I appreciate the detail. I'm headed to burning man until September 6, but I'm going to try to get this over the goal line before I arrive today.

If I don't manage to finish before I lose cell service, please feel free to make any changes as you see fit and merge--I don't want to stand in the way of progress on 1.0!

[seanmonstar]

Thanks so much for taking it this far! I've opened #2974 that rebases this, and I may tweak a little more, and then merge.