Skip to content

Commit cc21978

Browse files
authored
Rollup merge of rust-lang#129232 - ivmarkov:master, r=workingjubilee
Fix `thread::sleep` Duration-handling for ESP-IDF Addresses the ESP-IDF specific aspect of rust-lang#129212 #### A short summary of the problems addressed by this PR: ================================================ 1. **Problem 1** - the current implementation of `std::thread::sleep` does not properly round up the passed `Duration` As per the documentation of `std::thread::sleep`, the implementation should sleep _at least_ for the provided duration, but not less. Since the minimum supported resolution of the `usleep` syscall which is used with ESP-IDF is one microsecond, this means that we need to round-up any sub-microsecond nanos to one microsecond. Moreover, in the edge case where the user had passed a duration of < 1000 nanos (i.e. less than one microsecond), the current implementation will _not_ sleep _at all_. This is addressed by this PR. 2. **Problem 2** - the implementation of `usleep` on the ESP-IDF can overflow if the passed number of microseconds is >= `u32::MAX - 1_000_000` This is also addressed by this PR. Extra details for Problem 2: `u32::MAX - 1_000_000` is chosen to accommodate for the longest possible systick on the ESP IDF which is 1000ms. The systick duration is selected when compiling the ESP IDF FreeRTOS task scheduler itself, so we can't know it from within `STD`. The default systick duration is 10ms, and might be lowered down to 1ms. (Making it longer I have never seen, but in theory it can go up to a 1000ms max, even if obviously a one second systick is unrealistic - but we are paranoid in the PR.) While the overflow is reported upstream in the ESP IDF repo[^1], I still believe we should workaround it in the Rust wrappers as well, because it might take time until it is fixed, and they might not fix it for all released ESP IDF versions. For big durations, rather than calling `usleep` repeatedly on the ESP-IDF in chunks of `u32::MAX - 1_000_000`us, it might make sense to call instead with 1_000_000us (one second) as this is the max period that seems to be agreed upon as a safe max period in the `usleep` POSIX spec. On the other hand, that might introduce less precision (as we need to call more times `usleep` in a loop) and, we would be fighting a theoretical problem only, as I have big doubts the ESP IDF will stop supporting durations higher than 1_000_000us - ever - because of backwards compatibility with code which already calls `usleep` on the ESP IDF with bigger durations. [^1]: espressif/esp-idf#14390
2 parents d37ebfe + 0b0dad4 commit cc21978

File tree

1 file changed

+24
-6
lines changed

1 file changed

+24
-6
lines changed

Diff for: std/src/sys/pal/unix/thread.rs

+24-6
Original file line numberDiff line numberDiff line change
@@ -267,14 +267,32 @@ impl Thread {
267267

268268
#[cfg(target_os = "espidf")]
269269
pub fn sleep(dur: Duration) {
270-
let mut micros = dur.as_micros();
271-
unsafe {
272-
while micros > 0 {
273-
let st = if micros > u32::MAX as u128 { u32::MAX } else { micros as u32 };
270+
// ESP-IDF does not have `nanosleep`, so we use `usleep` instead.
271+
// As per the documentation of `usleep`, it is expected to support
272+
// sleep times as big as at least up to 1 second.
273+
//
274+
// ESP-IDF does support almost up to `u32::MAX`, but due to a potential integer overflow in its
275+
// `usleep` implementation
276+
// (https://github.com/espressif/esp-idf/blob/d7ca8b94c852052e3bc33292287ef4dd62c9eeb1/components/newlib/time.c#L210),
277+
// we limit the sleep time to the maximum one that would not cause the underlying `usleep` implementation to overflow
278+
// (`portTICK_PERIOD_MS` can be anything between 1 to 1000, and is 10 by default).
279+
const MAX_MICROS: u32 = u32::MAX - 1_000_000 - 1;
280+
281+
// Add any nanoseconds smaller than a microsecond as an extra microsecond
282+
// so as to comply with the `std::thread::sleep` contract which mandates
283+
// implementations to sleep for _at least_ the provided `dur`.
284+
// We can't overflow `micros` as it is a `u128`, while `Duration` is a pair of
285+
// (`u64` secs, `u32` nanos), where the nanos are strictly smaller than 1 second
286+
// (i.e. < 1_000_000_000)
287+
let mut micros = dur.as_micros() + if dur.subsec_nanos() % 1_000 > 0 { 1 } else { 0 };
288+
289+
while micros > 0 {
290+
let st = if micros > MAX_MICROS as u128 { MAX_MICROS } else { micros as u32 };
291+
unsafe {
274292
libc::usleep(st);
275-
276-
micros -= st as u128;
277293
}
294+
295+
micros -= st as u128;
278296
}
279297
}
280298

0 commit comments

Comments
 (0)