Skip to content

Conversation

@joboet
Copy link
Member

@joboet joboet commented Jan 20, 2026

#149935 demonstrates an issue with thread::sleep on Windows where sleep returns early, before the given duration has elapsed. The proposed fix (#150842) only addresses parts of the problem however – the fundamental issue is that the Windows timers all use a different clock source than the performance counter used by Instant, necessitating conversions that cannot always be performed accurately: If QueryPerformanceFrequency overestimates the frequency, time as measured by Instant will advance slower than the time used for SetWaitableTimer, resulting in sleep returning early for very long duration sleeps.

This problem is not even Windows-specific, e.g. nanosleep is actually specified to use CLOCK_REALTIME, but without respecting clock adjustments, so if CLOCK_MONOTONIC has a slight drift, nanosleep might return early on some UNIXes as well (Linux actually uses CLOCK_MONOTONIC for nanosleep so it is not affected). Actually, most our lower-tier targets don't guarantee any synchronicity between their sleep syscall and their time syscall.

It is of course possible to remediate this problem by implementing sleep as e.g.

fn sleep(duration: Duration) {
    let deadline = Instant::now() + duration;
    while let Some(remaining) = deadline.checked_sub(Instant::now()) {
        platform_sleep(remaining);
    }
}

However, this adds a lot of overhead to what is supposed to be a simple function. Most users call sleep with the intention of just sleeping for a bit and don't care about precise timing guarantees – or else they would have noticed these issues.

Thus, I'd like to propose that we remove the guarantee that

It [the thread] will never sleep less.

from sleep and recommend that users use sleep_until (#113752) if they need specific timing guarantees1. sleep_until is very much built for precise timing, uses the same clock as Instant on most UNIXes and is not affected by issues like oversleeping due to a high number of signals2 or not being scheduled for a long time.

This is actually not really a breaking change: The current sleep documentation does not specify the clock used3, so the change in this PR could also be phrased as saying that the clock used for sleep is not guaranteed to be the same as that of Instant. I just find it clearer to remove the guarantee, as the guarantee is basically worthless without specifying a time source.

r? libs-api
@rustbot label +I-libs-api-nominated

Footnotes

  1. Which should be stabilised ASAP if this PR is merged.

  2. See https://manned.org/man/arch/nanosleep.2#head10

  3. It is however implied by the example that it is the same as that of Instant.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. I-libs-api-nominated Nominated for discussion during a libs-api team meeting. labels Jan 20, 2026
@joboet joboet added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 20, 2026
@rust-log-analyzer

This comment has been minimized.

@joshtriplett
Copy link
Member

#150842

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 20, 2026
@joboet
Copy link
Member Author

joboet commented Jan 20, 2026

#150842

@Amanieu As I mentioned, that still does not fix the issue entirely. Should I open a PR that makes the time source unspecified instead?

@joboet
Copy link
Member Author

joboet commented Jan 21, 2026

I've opened #151450 which fixes the issue on some UNIX platforms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

I-libs-api-nominated Nominated for discussion during a libs-api team meeting. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants