Skip to content

pal/hermit: correctly round up microseconds in Thread::sleep #129588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 26, 2024

Conversation

mkroening
Copy link
Contributor

This fixes the Hermit-related part of #129212 and thus the whole issue, since ESP-IDF is already fixed, as far as I understand.

Fixes #129212

r? @workingjubilee

CC: @stlankes

@rustbot rustbot added O-hermit Operating System: Hermit 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. labels Aug 25, 2024
@workingjubilee
Copy link
Member

Yup!

@workingjubilee
Copy link
Member

workingjubilee commented Aug 25, 2024

I assume hermit here is assessed as not having to do the "repeatedly call the underlying sleep fn" thing because it has 64-bit usleep?

What if Duration::as_micros returns a u128 greater than u64::MAX?

@workingjubilee
Copy link
Member

granted, perhaps sleeping accuracy over... uh, would it be... 500,000 years?... is not a concern.

@workingjubilee
Copy link
Member

...actually, it IS a little bit of a concern: if someone inputs a really absurd Duration, the as conversion is not saturating, it is truncating-to-bits, thus someone can request to sleep until an Age has came and went but actually wake up 0 seconds later. We should first at least use u128::min(u64::MAX as u128, micros)

@workingjubilee
Copy link
Member

That is still only relevant to vampires writing Rust code to manage their wakeup alarms for torpor, but can you imagine how cranky someone would be if they woke up a hundred thousand years early?

Probably we should respec Duration in such a way that people don't have to worry about this absurdity, or introduce a new type and make thread::sleep take a generic, but until then...

@mkroening
Copy link
Contributor Author

Thanks for the quick review!

I pushed a commit to saturate the conversion. :)

@workingjubilee
Copy link
Member

Cool, thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 26, 2024

📌 Commit edeefc5 has been approved by workingjubilee

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 26, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 26, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#129190 (Add f16 and f128 to tests/ui/consts/const-float-bits-conv.rs)
 - rust-lang#129377 (Add implementations for `unbounded_shl`/`unbounded_shr`)
 - rust-lang#129539 (link to Future::poll from the Poll docs)
 - rust-lang#129588 (pal/hermit: correctly round up microseconds in `Thread::sleep`)
 - rust-lang#129592 (Remove cfg(test) from library/core)
 - rust-lang#129597 (mv `build_reduced_graph_for_external_crate_res` into Resolver)
 - rust-lang#129600 (Tie `impl_trait_overcaptures` lint to Rust 2024)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 26, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#129190 (Add f16 and f128 to tests/ui/consts/const-float-bits-conv.rs)
 - rust-lang#129377 (Add implementations for `unbounded_shl`/`unbounded_shr`)
 - rust-lang#129539 (link to Future::poll from the Poll docs)
 - rust-lang#129588 (pal/hermit: correctly round up microseconds in `Thread::sleep`)
 - rust-lang#129592 (Remove cfg(test) from library/core)
 - rust-lang#129597 (mv `build_reduced_graph_for_external_crate_res` into Resolver)
 - rust-lang#129600 (Tie `impl_trait_overcaptures` lint to Rust 2024)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit d0b3c3a into rust-lang:master Aug 26, 2024
6 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 26, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 26, 2024
Rollup merge of rust-lang#129588 - hermit-os:sleep-micros, r=workingjubilee

pal/hermit: correctly round up microseconds in `Thread::sleep`

This fixes the Hermit-related part of rust-lang#129212 and thus the whole issue, since ESP-IDF is already fixed, as far as I understand.

Fixes rust-lang#129212

r? `@workingjubilee`

CC: `@stlankes`
@mkroening mkroening deleted the sleep-micros branch December 13, 2024 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-hermit Operating System: Hermit S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

multiple platforms incorrectly impl thread::sleep with Duration::as_micros
4 participants