From f7b859ac841e56ebdd6485a95a9f464adae02095 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 23 Dec 2020 21:09:42 +1100 Subject: [PATCH] freertos: Fix race condition using vTaskDelete() cross-core causing resource leak Causes test added in parent commit to pass. This race happens if the deleted task is running on the other CPU, and is already spinning in a critical section waiting for xTaskQueueMutex because it's about to be blocked for a resource. The "deleted" task would end up blocked, possibly indefinitely, and never actually deleted or its resources cleaned up by the idle tasks. Details: vTaskDelete() adds the target task to the xTasksWaitingTermination list, expecting it to be yielded off CPU and then cleaned up later. However as soon as vTaskDelete() releases xTaskQueueMutex, the target task runs and moves itself to the xDelayedTaskList1. Because interrupts are already disabled on that CPU, the "yield" to the other CPU sent by the vTaskDelete() comes afterward so doesn't help. --- components/freertos/tasks.c | 41 ++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c index 0d1a6313b6..deb7774d01 100644 --- a/components/freertos/tasks.c +++ b/components/freertos/tasks.c @@ -1340,6 +1340,21 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode hence xYieldPending is used to latch that a context switch is required. */ portPRE_TASK_DELETE_HOOK( pxTCB, &xYieldPending[core] ); + + if (portNUM_PROCESSORS > 1 && pxTCB == pxCurrentTCB[ !core ]) + { + /* SMP case of deleting a task running on a different core. Same issue + as a task deleting itself, but we need to send a yield to this task now + before we release xTaskQueueMutex. + + Specifically there is a case where the other core may already be spinning on + xTaskQueueMutex waiting to go into a blocked state. A check is added in + prvAddCurrentTaskToDelayedList() to prevent it from removing itself from + xTasksWaitingTermination list in this case (instead it will immediately + release xTaskQueueMutex again and be yielded before the FreeRTOS function + returns.) */ + vPortYieldOtherCore( !core ); + } } else { @@ -1372,25 +1387,6 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode configASSERT( xTaskGetSchedulerState() != taskSCHEDULER_SUSPENDED ); portYIELD_WITHIN_API(); } - else if ( portNUM_PROCESSORS > 1 ) - { - /* Check if the deleted task is currently running on any other core - and force a yield to take it off. - - (this includes re-checking the core that curTCB was previously - running on, in case curTCB has migrated to a different core.) - */ - taskENTER_CRITICAL( &xTaskQueueMutex ); - for(BaseType_t i = 0; i < portNUM_PROCESSORS; i++) - { - if(pxTCB == pxCurrentTCB[ i ] ) - { - vPortYieldOtherCore( i ); - break; - } - } - taskEXIT_CRITICAL( &xTaskQueueMutex ); - } else { mtCOVERAGE_TEST_MARKER(); @@ -5677,6 +5673,13 @@ static void prvAddCurrentTaskToDelayedList( const portBASE_TYPE xCoreID, const T TickType_t xTimeToWake; const TickType_t xConstTickCount = xTickCount; + if (portNUM_PROCESSORS > 1 && listIS_CONTAINED_WITHIN(&xTasksWaitingTermination, &( pxCurrentTCB[xCoreID]->xStateListItem))) { + /* vTaskDelete() has been called on this task. This would have happened from the other core while this task was spinning on xTaskQueueMutex, + so don't move the running task to the delayed list - as soon as this core re-enables interrupts this task will + be suspended permanently */ + return; + } + #if( INCLUDE_xTaskAbortDelay == 1 ) { /* About to enter a delayed list, so ensure the ucDelayAborted flag is