From f57097239b8c8c330dcd70f3e5355bd3f538472d Mon Sep 17 00:00:00 2001 From: Kaspar Schleiser Date: Thu, 25 Feb 2016 19:56:15 +0100 Subject: [PATCH] sys: xtimer: fix some race conditions --- sys/include/xtimer.h | 5 +---- sys/xtimer/xtimer_core.c | 41 ++++++++++++++++++++++------------------ 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/sys/include/xtimer.h b/sys/include/xtimer.h index 488f50fa3eb49..0e2b7cf9815b8 100644 --- a/sys/include/xtimer.h +++ b/sys/include/xtimer.h @@ -251,11 +251,8 @@ void xtimer_set(xtimer_t *timer, uint32_t offset); * @note this function runs in O(n) with n being the number of active timers * * @param[in] timer ptr to timer structure that will be removed - * - * @return 1 on success - * @return 0 when timer was not active */ -int xtimer_remove(xtimer_t *timer); +void xtimer_remove(xtimer_t *timer); /** * @brief receive a message blocking but with timeout diff --git a/sys/xtimer/xtimer_core.c b/sys/xtimer/xtimer_core.c index 852194a7a9b42..c78f1f19b4f0e 100644 --- a/sys/xtimer/xtimer_core.c +++ b/sys/xtimer/xtimer_core.c @@ -40,6 +40,7 @@ static xtimer_t *long_list_head = NULL; static void _add_timer_to_list(xtimer_t **list_head, xtimer_t *timer); static void _add_timer_to_long_list(xtimer_t **list_head, xtimer_t *timer); static void _shoot(xtimer_t *timer); +static void _remove(xtimer_t *timer); static inline void _lltimer_set(uint32_t target); static uint32_t _time_left(uint32_t target, uint32_t reference); @@ -94,7 +95,10 @@ void _xtimer_set64(xtimer_t *timer, uint32_t offset, uint32_t long_offset) xtimer_set(timer, (uint32_t) offset); } else { - xtimer_remove(timer); + int state = disableIRQ(); + if (_is_set(timer)) { + _remove(timer); + } _xtimer_now64(&timer->target, &timer->long_target); timer->target += offset; @@ -103,7 +107,6 @@ void _xtimer_set64(xtimer_t *timer, uint32_t offset, uint32_t long_offset) timer->long_target++; } - int state = disableIRQ(); _add_timer_to_long_list(&long_list_head, timer); restoreIRQ(state); DEBUG("xtimer_set64(): added longterm timer (long_target=%" PRIu32 " target=%" PRIu32 ")\n", @@ -172,13 +175,17 @@ int _xtimer_set_absolute(xtimer_t *timer, uint32_t target) return 0; } + unsigned state = disableIRQ(); + if (_is_set(timer)) { + _remove(timer); + } + timer->target = target; timer->long_target = _long_cnt; if (target < now) { timer->long_target++; } - unsigned state = disableIRQ(); if ( !_this_high_period(target) ) { DEBUG("xtimer_set_absolute(): the timer doesn't fit into the low-level timer's mask.\n"); _add_timer_to_long_list(&long_list_head, timer); @@ -239,14 +246,8 @@ static int _remove_timer_from_list(xtimer_t **list_head, xtimer_t *timer) return 0; } -int xtimer_remove(xtimer_t *timer) +static void _remove(xtimer_t *timer) { - if (!_is_set(timer)) { - return 0; - } - - unsigned state = disableIRQ(); - int res = 0; if (timer_list_head == timer) { uint32_t next; timer_list_head = timer->next; @@ -260,17 +261,21 @@ int xtimer_remove(xtimer_t *timer) _lltimer_set(next); } else { - res = _remove_timer_from_list(&timer_list_head, timer) || - _remove_timer_from_list(&overflow_list_head, timer) || - _remove_timer_from_list(&long_list_head, timer); + if (!_remove_timer_from_list(&timer_list_head, timer)) { + if (!_remove_timer_from_list(&overflow_list_head, timer)) { + _remove_timer_from_list(&long_list_head, timer); + } + } } +} - timer->target = 0; - timer->long_target = 0; - +void xtimer_remove(xtimer_t *timer) +{ + int state = disableIRQ(); + if (_is_set(timer)) { + _remove(timer); + } restoreIRQ(state); - - return res; } static uint32_t _time_left(uint32_t target, uint32_t reference)