diff options
Diffstat (limited to 'rtic-time')
| -rw-r--r-- | rtic-time/CHANGELOG.md | 3 | ||||
| -rw-r--r-- | rtic-time/src/half_period_counter.rs | 26 | ||||
| -rw-r--r-- | rtic-time/tests/half_period_time_counter.rs | 4 |
3 files changed, 22 insertions, 11 deletions
diff --git a/rtic-time/CHANGELOG.md b/rtic-time/CHANGELOG.md index 8a99633a..24b93227 100644 --- a/rtic-time/CHANGELOG.md +++ b/rtic-time/CHANGELOG.md @@ -16,6 +16,9 @@ For each category, *Added*, *Changed*, *Fixed* add new entries at the top! ### Fixed +- Fix race condition in `half_period_counter::calculate_now`. + This sadly required a minor API change. + ## v1.1.0 - 2023-12-04 ### Added diff --git a/rtic-time/src/half_period_counter.rs b/rtic-time/src/half_period_counter.rs index 0aaec5e2..753b75ab 100644 --- a/rtic-time/src/half_period_counter.rs +++ b/rtic-time/src/half_period_counter.rs @@ -108,7 +108,7 @@ //! //! fn now() -> u64 { //! rtic_time::half_period_counter::calculate_now( -//! HALF_PERIOD_COUNTER.load(Ordering::Relaxed), +//! || HALF_PERIOD_COUNTER.load(Ordering::Relaxed), //! || timer_get_value(), //! ) //! } @@ -191,19 +191,27 @@ impl_timer_ops!(u128); /// /// # Arguments /// -/// * `half_periods` - The period counter value. If read from an atomic, can use `Ordering::Relaxed`. +/// * `half_periods` - A closure/function that when called produces the period counter value. If read from an atomic, can use `Ordering::Relaxed`. /// * `timer_value` - A closure/function that when called produces the current timer value. -pub fn calculate_now<P, T, F, O>(half_periods: P, timer_value: F) -> O +pub fn calculate_now<P, T, F1, F2, O>(half_periods: F1, timer_value: F2) -> O where T: TimerValue, O: From<P> + From<T> + TimerOps, - F: FnOnce() -> T, + F1: FnOnce() -> P, + F2: FnOnce() -> T, { - // Important: half_period **must** be read first. - // Otherwise we have another mathematical race condition. - let half_periods = O::from(half_periods); - compiler_fence(Ordering::Acquire); - let timer_value = O::from(timer_value()); + // This is timing critical; for fast-overflowing timers (like the 1MHz 16-bit timers on STM32), + // it could lead to erroneous behavior if preempted in between the two reads. + // Hence the critical section. + let (half_periods, timer_value) = critical_section::with(|_| { + // Important: half_periods **must** be read first. + // Otherwise the mathematical principle that prevents + // the race condition does not work. + let half_periods = O::from(half_periods()); + compiler_fence(Ordering::Acquire); + let timer_value = O::from(timer_value()); + (half_periods, timer_value) + }); // Credits to the `time-driver` of `embassy-stm32`. // diff --git a/rtic-time/tests/half_period_time_counter.rs b/rtic-time/tests/half_period_time_counter.rs index ceffbea2..ee501b43 100644 --- a/rtic-time/tests/half_period_time_counter.rs +++ b/rtic-time/tests/half_period_time_counter.rs @@ -5,7 +5,7 @@ macro_rules! do_test_u8 { let periods: u32 = $periods; let counter: u8 = $counter; let expected: u32 = $expected; - let actual: u32 = calculate_now(periods, || counter); + let actual: u32 = calculate_now(|| periods, || counter); assert_eq!( actual, expected, "Expected: ({} | {}) => {}, got: {}", @@ -19,7 +19,7 @@ macro_rules! do_test_u16 { let periods: u16 = $periods; let counter: u16 = $counter; let expected: u32 = $expected; - let actual: u32 = calculate_now(periods, || counter); + let actual: u32 = calculate_now(|| periods, || counter); assert_eq!( actual, expected, "Expected: ({} | {}) => {}, got: {}", |