rust-lang / libs-team

The home of the library team
Apache License 2.0
116 stars 18 forks source link

thread::sleep_until, wait until a deadline is reached #237

Closed dvdsk closed 1 year ago

dvdsk commented 1 year ago

Implemented; see tracking issue: https://github.com/rust-lang/rust/issues/113752

Proposal

Problem statement

There is no method to pause a thread until a deadline is reached. Currently that means writing one yourself. I argue the use case is common enough that it is worth it to bring this into the std. It would make code that needs this slightly shorter, a bit more readable and easier to write.

Last but not least while a pause until method is not hard to write it is easy to do incorrectly without noticing. A naive approach subtracts the deadline from Instant::now(). Currently, this works, however it might panic in the future see Monotonicity and Instant::sub. Adding sleep_until lessens the impact of future rust versions panicking again a bit.

Motivating examples or use cases

Most use-cases take the shape of a planning problem, performing some action at a set moment. For example today I wanted to use sleep_until to exponentially back off on requesting a certificate up to a deadline. Like any web request it may take a while before it fails, therefore we must take the time the work takes into account.

const MAX_DURATION: Duration = Duration::from_secs(10);
let mut next_attempt = Instant::now();
let deadline = Instant::now() + MAX_DURATION;
let mut delay = Duration::from_millis(250);

loop {
    if Instant::now() > deadline {
        break Err(()),
    }
    // get_signed_certificate also takes some time
    if let Ready(cert) = get_signed_certificate() {
        break Ok(cert),
    }

    delay *= 2;
    next_attempt = deadline.min(next_attempt + delay);
    sleep_until(next_attempt);
}

This can be generalized by replacing get_signed_certificate with any fallible function that takes a non-trivial amount of time.

On GitHub there are about 3k hits for rust code containing fn sleep_until. Though a minor argument, having this in the std could help porting async code using tokios sleep_until easier.

Solution sketch

I propose we make the solution look like the sleep_until function in tokio::time::sleep_until.

The API:

pub fn sleep_until(deadline: Instant);

The implementation:

// proposed implementation
pub fn sleep_until(deadline: Instant) {
    let now = Instant::now();

    // if now is after the deadline do not sleep
    if let Some(delay) = deadline.checked_duration_since(now) {
        thread::sleep(delay);        
    }
}

playground link

Alternatives

If we chose not to implement sleep_until an example should be added to thread::sleep to steer users away from using subtract on two Instants.

Links and related work

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

if the libs-api team decides to adopt this proposal I would like to implement the RFC

the8472 commented 1 year ago

There is no method to pause a thread until a deadline is reached. A naive approach subtracts the deadline from Instant::now(). Currently, this works, however it might panic in the future see Monotonicity and Instant::sub.

The section also points to checked_duration_since as a guranteed panic-free option. So if that is the motivation... the problem is already solved?

the8472 commented 1 year ago

The other issue is the eternal question whether one wants to sleep for a duration including or excluding system suspend time. The answer often differs depending on the kind of task that is sleeping. That is currently inconsistent across different operating systems.

Solving this properly would need some sort of Clocksource API.

dvdsk commented 1 year ago

There is no method to pause a thread until a deadline is reached. A naive approach subtracts the deadline from Instant::now(). Currently, this works, however it might panic in the future see Monotonicity and Instant::sub.

The section also points to checked_duration_since as a guranteed panic-free option. So if that is the motivation... the problem is already solved?

Ahh sorry, what I meant is that because there is no sleep_until in the std people tend to implement it themselves. While they can do that correctly using checked_duration_since that assumes they noticed the issue with Monotonicity. That requires them to look at the sub implementation documentation. I am not usually that careful and I think I am not alone in that.

I will edit the RFC and clarify that section. I will also add code readability for completeness.

dvdsk commented 1 year ago

The other issue is the eternal question whether one wants to sleep for a duration including or excluding system suspend time.

In my opinion thread::sleep_until should have the same behavior as thread::sleep currently has. Otherwise it could get quite confusing.

the8472 commented 1 year ago

thread::sleep uses nanosleep, Which per posix uses CLOCK_REALTIME (which is represented by SystemTime not Instant) but on linux uses CLOCK_MONOTONIC. So it may not be possible to get consistent behavior across unixes. And then there's windows.

I think some exploration which APIs and behaviors are available on supported platforms will be necessary.

You don't necessarily have to do that work now. The team's answer might be "yes, but only if some portability conditions can be met"

Amanieu commented 1 year ago

We discussed this in the @rust-lang/libs-api meeting today and are mostly happy with this proposal. We did discuss the possibility of support multiple clock sources (SystemTime, Instant, some sort of CLOCK_BOOTTIME, etc), but this question can be deferred for now until the stabilization of this feature.

dvdsk commented 1 year ago

Thanks for accepting the proposal!

I am looking into the various APIs/platforms right now, trying to find a consistent clock source. Once I have learned a bit more I will hammer out an implementation and open a tracking issue and PR.

neachdainn commented 10 months ago

I just want to point out that the proposed solution isn't a full solution in that it ignores the possibility of drift. This probably doesn't matter for many implementations but is critical in situations where it does. Specifically:

pub fn sleep_until(deadline: Instant) {
    let now = Instant::now();

   // What if the thread is suspended here? If that happens, you are no longer sleeping until `deadline` but
   // are instead sleeping until `deadline + epsilon` which is not universally acceptable. The relevant OS's
   // have operations to do actual deadline sleeping and those should be used here.

    if let Some(delay) = deadline.checked_duration_since(now) {
        thread::sleep(delay);        
    }
}

thread::sleep uses nanosleep, Which per posix uses CLOCK_REALTIME (which is represented by SystemTime not Instant) but on linux uses CLOCK_MONOTONIC. So it may not be possible to get consistent behavior across unixes. And then there's windows.

clock_nanosleep is a part of POSIX. Not only that, but specifying TIMER_ABSTIME will give this the correct behavior without the possibility of the bug described above. In fact, it makes the implementation of this relatively trivial (in pseudo-ish code):

pub fn sleep_until(deadline: Instant) {
    while libc::clock_nanosleep(libc::CLOCK_MONOTONIC, libc::TIMER_ABSTIME, deadline, ptr::null()) == libc::EINTR {}     
}