refactor(rt): Clean up Timer trait

[sfackler]

This both avoids an unnecessary extra layer of boxing and removes some dubious unsafe code.

Closes #3028

[notgull]

Unrelated: Would it be good to have the resulting futures as an associated type rather than as a boxed future? Both tokio and smol have well-defined "sleep" types instead of async methods, so we may be able to avoid an allocation here by not boxing it.

[sfackler]

That would require parameterizing the ~entire API over the timer trait which I'm not sure is desired.

[notgull]

As a workaround, you could just do:

struct BoxedTimer {
    inner: Box<dyn Timer<Sleep = Pin<Box<dyn Future>>>>>
}

But this might not be idiomatic, so I agree with your assessment.

[bossmc]

Is there an assumption in hyper that the Sleep future(s) cancel on drop? I could imagine a fairly simple implementation (a BinaryHeap of wakers) where sleep-futures continue to fire their wakers after the future has been dropped - would this be a problem?

If this is expected, it would be nice to specify in the trait comment so implementors get this right.

[sfackler]

Any Future implementation needs to work in the presence of polls that don't make progress, so I can't see how hyper could make that assumption.

[bossmc]

Fair enough, deregistering/cancelling the timer on Drop of the Sleep future is therefore simply an optimization (to reduce unnecessary wakeups) but not needed for correctness, thanks for confirming!