Merge branch 'contrib/github_pr_11215_v4.4' into 'release/v4.4'

improve thread safety in esp_timer (GitHub PR) (v4.4)

See merge request espressif/esp-idf!23541
pull/12186/head
Marius Vikhammer 2023-06-08 16:00:14 +08:00
commit 3cba50e0a1
1 zmienionych plików z 62 dodań i 26 usunięć

Wyświetl plik

@ -145,18 +145,30 @@ esp_err_t IRAM_ATTR esp_timer_start_once(esp_timer_handle_t timer, uint64_t time
if (timer == NULL) { if (timer == NULL) {
return ESP_ERR_INVALID_ARG; return ESP_ERR_INVALID_ARG;
} }
if (!is_initialized() || timer_armed(timer)) { if (!is_initialized()) {
return ESP_ERR_INVALID_STATE; return ESP_ERR_INVALID_STATE;
} }
int64_t alarm = esp_timer_get_time() + timeout_us; int64_t alarm = esp_timer_get_time() + timeout_us;
esp_timer_dispatch_t dispatch_method = timer->flags & FL_ISR_DISPATCH_METHOD; esp_timer_dispatch_t dispatch_method = timer->flags & FL_ISR_DISPATCH_METHOD;
esp_err_t err;
timer_list_lock(dispatch_method); timer_list_lock(dispatch_method);
timer->alarm = alarm;
timer->period = 0; /* Check if the timer is armed once the list is locked.
* Otherwise another task may arm the timer inbetween the check
* and us locking the list, resulting in us inserting the
* timer to s_timers a second time. This will create a loop
* in s_timers. */
if (timer_armed(timer)) {
err = ESP_ERR_INVALID_STATE;
} else {
timer->alarm = alarm;
timer->period = 0;
#if WITH_PROFILING #if WITH_PROFILING
timer->times_armed++; timer->times_armed++;
#endif #endif
esp_err_t err = timer_insert(timer, false); err = timer_insert(timer, false);
}
timer_list_unlock(dispatch_method); timer_list_unlock(dispatch_method);
return err; return err;
} }
@ -166,20 +178,27 @@ esp_err_t IRAM_ATTR esp_timer_start_periodic(esp_timer_handle_t timer, uint64_t
if (timer == NULL) { if (timer == NULL) {
return ESP_ERR_INVALID_ARG; return ESP_ERR_INVALID_ARG;
} }
if (!is_initialized() || timer_armed(timer)) { if (!is_initialized()) {
return ESP_ERR_INVALID_STATE; return ESP_ERR_INVALID_STATE;
} }
period_us = MAX(period_us, esp_timer_impl_get_min_period_us()); period_us = MAX(period_us, esp_timer_impl_get_min_period_us());
int64_t alarm = esp_timer_get_time() + period_us; int64_t alarm = esp_timer_get_time() + period_us;
esp_timer_dispatch_t dispatch_method = timer->flags & FL_ISR_DISPATCH_METHOD; esp_timer_dispatch_t dispatch_method = timer->flags & FL_ISR_DISPATCH_METHOD;
esp_err_t err;
timer_list_lock(dispatch_method); timer_list_lock(dispatch_method);
timer->alarm = alarm;
timer->period = period_us; /* Check if the timer is armed once the list is locked to avoid a data race */
if (timer_armed(timer)) {
err = ESP_ERR_INVALID_STATE;
} else {
timer->alarm = alarm;
timer->period = period_us;
#if WITH_PROFILING #if WITH_PROFILING
timer->times_armed++; timer->times_armed++;
timer->times_skipped = 0; timer->times_skipped = 0;
#endif #endif
esp_err_t err = timer_insert(timer, false); err = timer_insert(timer, false);
}
timer_list_unlock(dispatch_method); timer_list_unlock(dispatch_method);
return err; return err;
} }
@ -189,10 +208,22 @@ esp_err_t IRAM_ATTR esp_timer_stop(esp_timer_handle_t timer)
if (timer == NULL) { if (timer == NULL) {
return ESP_ERR_INVALID_ARG; return ESP_ERR_INVALID_ARG;
} }
if (!is_initialized() || !timer_armed(timer)) { if (!is_initialized()) {
return ESP_ERR_INVALID_STATE; return ESP_ERR_INVALID_STATE;
} }
return timer_remove(timer); esp_timer_dispatch_t dispatch_method = timer->flags & FL_ISR_DISPATCH_METHOD;
esp_err_t err;
timer_list_lock(dispatch_method);
/* Check if the timer is armed once the list is locked to avoid a data race */
if (!timer_armed(timer)) {
err = ESP_ERR_INVALID_STATE;
} else {
err = timer_remove(timer);
}
timer_list_unlock(dispatch_method);
return err;
} }
esp_err_t esp_timer_delete(esp_timer_handle_t timer) esp_err_t esp_timer_delete(esp_timer_handle_t timer)
@ -200,22 +231,27 @@ esp_err_t esp_timer_delete(esp_timer_handle_t timer)
if (timer == NULL) { if (timer == NULL) {
return ESP_ERR_INVALID_ARG; return ESP_ERR_INVALID_ARG;
} }
if (timer_armed(timer)) {
return ESP_ERR_INVALID_STATE;
}
// A case for the timer with ESP_TIMER_ISR:
// This ISR timer was removed from the ISR list in esp_timer_stop() or in timer_process_alarm() -> LIST_REMOVE(it, list_entry)
// and here this timer will be added to another the TASK list, see below.
// We do this because we want to free memory of the timer in a task context instead of an isr context.
int64_t alarm = esp_timer_get_time(); int64_t alarm = esp_timer_get_time();
esp_err_t err;
timer_list_lock(ESP_TIMER_TASK); timer_list_lock(ESP_TIMER_TASK);
timer->flags &= ~FL_ISR_DISPATCH_METHOD;
timer->event_id = EVENT_ID_DELETE_TIMER; /* Check if the timer is armed once the list is locked to avoid a data race */
timer->alarm = alarm; if (timer_armed(timer)) {
timer->period = 0; err = ESP_ERR_INVALID_STATE;
timer_insert(timer, false); } else {
// A case for the timer with ESP_TIMER_ISR:
// This ISR timer was removed from the ISR list in esp_timer_stop() or in timer_process_alarm() -> LIST_REMOVE(it, list_entry)
// and here this timer will be added to another the TASK list, see below.
// We do this because we want to free memory of the timer in a task context instead of an isr context.
timer->flags &= ~FL_ISR_DISPATCH_METHOD;
timer->event_id = EVENT_ID_DELETE_TIMER;
timer->alarm = alarm;
timer->period = 0;
err = timer_insert(timer, false);
}
timer_list_unlock(ESP_TIMER_TASK); timer_list_unlock(ESP_TIMER_TASK);
return ESP_OK; return err;
} }
static IRAM_ATTR esp_err_t timer_insert(esp_timer_handle_t timer, bool without_update_alarm) static IRAM_ATTR esp_err_t timer_insert(esp_timer_handle_t timer, bool without_update_alarm)