From c41c02872fdd2773f8de1566f7578d65ed8275c0 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Mon, 18 Dec 2017 21:58:15 +0800 Subject: [PATCH] freertos: prvCheckTasksWaitingTermination bugfix Bugfix to prevent a self deleting no affinity task's memory from being freed by the idle task of the other core before the self deleting no affinity task is able to context switch out. prvCheckTasksWaitingTermination now checks if the task is still on pxCurrentTCB before freeing task memory. --- components/freertos/tasks.c | 13 +++++++++---- .../freertos/test/test_freertos_task_delete.c | 17 +++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c index ff5c46bb4..e95a8b04b 100644 --- a/components/freertos/tasks.c +++ b/components/freertos/tasks.c @@ -3643,10 +3643,15 @@ static void prvCheckTasksWaitingTermination( void ) /* We only want to kill tasks that ran on this core because e.g. _xt_coproc_release needs to be called on the core the process is pinned on, if any */ ListItem_t *target = listGET_HEAD_ENTRY(&xTasksWaitingTermination); - for( ; target != listGET_END_MARKER(&xTasksWaitingTermination); target = listGET_NEXT(target) ){ - int coreid = (( TCB_t * )listGET_LIST_ITEM_OWNER(target))->xCoreID; - if(coreid == core || coreid == tskNO_AFFINITY){ //Find first item not pinned to other core - pxTCB = ( TCB_t * )listGET_LIST_ITEM_OWNER(target); + for( ; target != listGET_END_MARKER(&xTasksWaitingTermination); target = listGET_NEXT(target) ){ //Walk the list + TCB_t *tgt_tcb = ( TCB_t * )listGET_LIST_ITEM_OWNER(target); + int affinity = tgt_tcb->xCoreID; + //Self deleting tasks are added to Termination List before they switch context. Ensure they aren't still currently running + if( pxCurrentTCB[core] == tgt_tcb || (portNUM_PROCESSORS > 1 && pxCurrentTCB[!core] == tgt_tcb) ){ + continue; //Can't free memory of task that is still running + } + if(affinity == core || affinity == tskNO_AFFINITY){ //Find first item not pinned to other core + pxTCB = tgt_tcb; break; } } diff --git a/components/freertos/test/test_freertos_task_delete.c b/components/freertos/test/test_freertos_task_delete.c index 72f5cc85a..7c3394cbe 100644 --- a/components/freertos/test/test_freertos_task_delete.c +++ b/components/freertos/test/test_freertos_task_delete.c @@ -17,6 +17,7 @@ #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "esp_heap_caps.h" +#include "rom/ets_sys.h" #include "unity.h" @@ -24,6 +25,8 @@ #define DELAY_TICKS 2 #define HEAP_CAPS (MALLOC_CAP_INTERNAL|MALLOC_CAP_DEFAULT) +#define DELAY_US_ITERATIONS 1000 + static void tsk_self_del(void *param) { @@ -35,6 +38,13 @@ static void tsk_extern_del(void *param) vTaskDelay(portMAX_DELAY); //Await external deletion } +static void tsk_self_del_us_delay(void *param) +{ + uint32_t delay = (uint32_t)param; + ets_delay_us(delay); + vTaskDelete(NULL); +} + TEST_CASE("FreeRTOS Delete Tasks", "[freertos]") { /* -------------- Test vTaskDelete() on currently running tasks ----------------*/ @@ -63,4 +73,11 @@ TEST_CASE("FreeRTOS Delete Tasks", "[freertos]") } TEST_ASSERT_EQUAL(before_heap, heap_caps_get_free_size(HEAP_CAPS)); +/* Test self deleting no affinity task is not removed by idle task of other core before context switch */ + for(int i = 0; i < DELAY_US_ITERATIONS; i+= 10){ + vTaskDelay(1); //Sync to next tick interrupt + xTaskCreatePinnedToCore(tsk_self_del_us_delay, "delay", 1024, (void *)i, UNITY_FREERTOS_PRIORITY - 1, NULL, tskNO_AFFINITY); + ets_delay_us(10); //Busy wait to ensure no affinity task runs on opposite core + } + }