From 376107b2ae9058d172443b8fef8256c286ba5121 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Tue, 5 Nov 2019 21:49:13 +0800 Subject: [PATCH 1/2] freertos: Fix configASSERT thread safety This commit fixes thread safety issues with configASSERT() calls regarding the value of uxSchedulerSuspended. A false negative occurs if a context switch to the opposite core occurs in between the getting the core ID and the assesment. Closes https://github.com/espressif/esp-idf/issues/4230 --- components/freertos/tasks.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c index b19ba84a6..72ebefbde 100644 --- a/components/freertos/tasks.c +++ b/components/freertos/tasks.c @@ -1302,7 +1302,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode //No mux; no harm done if this misfires. The deleted task won't get scheduled anyway. if( pxTCB == pxCurrentTCB[ core ] ) //If task was currently running on this core { - configASSERT( uxSchedulerSuspended[ core ] == 0 ); + configASSERT( xTaskGetSchedulerState() == taskSCHEDULER_RUNNING ) /* The pre-delete hook is primarily for the Windows simulator, in which Windows specific clean up operations are performed, @@ -1337,7 +1337,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode configASSERT( pxPreviousWakeTime ); configASSERT( ( xTimeIncrement > 0U ) ); - configASSERT( uxSchedulerSuspended[ xPortGetCoreID() ] == 0 ); + configASSERT( xTaskGetSchedulerState() == taskSCHEDULER_RUNNING ); taskENTER_CRITICAL(&xTaskQueueMutex); // vTaskSuspendAll(); @@ -1435,7 +1435,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode /* A delay time of zero just forces a reschedule. */ if( xTicksToDelay > ( TickType_t ) 0U ) { - configASSERT( uxSchedulerSuspended[ xPortGetCoreID() ] == 0 ); + configASSERT( xTaskGetSchedulerState() == taskSCHEDULER_RUNNING ); taskENTER_CRITICAL(&xTaskQueueMutex); // vTaskSuspendAll(); { @@ -1818,7 +1818,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode if( xSchedulerRunning != pdFALSE ) { /* The current task has just been suspended. */ - configASSERT( uxSchedulerSuspended[ xPortGetCoreID() ] == 0 ); + configASSERT( xTaskGetSchedulerState() == taskSCHEDULER_RUNNING ); portYIELD_WITHIN_API(); } else @@ -2214,7 +2214,7 @@ BaseType_t xAlreadyYielded = pdFALSE; /* If uxSchedulerSuspended[ xPortGetCoreID() ] is zero then this function does not match a previous call to vTaskSuspendAll(). */ - configASSERT( uxSchedulerSuspended[ xPortGetCoreID() ] ); + configASSERT( xTaskGetSchedulerState() == taskSCHEDULER_SUSPENDED ); /* It is possible that an ISR caused a task to be removed from an event list while the scheduler was suspended. If this was the case then the removed task will have been added to the xPendingReadyList. Once the From 1b53af2e88492a57d6e74107e4ee3463a18c4e29 Mon Sep 17 00:00:00 2001 From: Mahavir Jain Date: Tue, 12 Nov 2019 22:36:30 +0530 Subject: [PATCH 2/2] freertos: modify configASSERTs around scheduler state check Regression introduced in commit 79e74e5d5f948e585bec586db7f482a2f2df50dc It is possible that some FreeRTOS APIs are invoked prior to scheduler start condition (e.g. flash initialization in unicore mode). In that condition these asserts should not trigger (scheduler state being yet to be started), hence changes per this fix. --- components/freertos/tasks.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/components/freertos/tasks.c b/components/freertos/tasks.c index 72ebefbde..5294b5d20 100644 --- a/components/freertos/tasks.c +++ b/components/freertos/tasks.c @@ -1302,7 +1302,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode //No mux; no harm done if this misfires. The deleted task won't get scheduled anyway. if( pxTCB == pxCurrentTCB[ core ] ) //If task was currently running on this core { - configASSERT( xTaskGetSchedulerState() == taskSCHEDULER_RUNNING ) + configASSERT( xTaskGetSchedulerState() != taskSCHEDULER_SUSPENDED ) /* The pre-delete hook is primarily for the Windows simulator, in which Windows specific clean up operations are performed, @@ -1337,7 +1337,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode configASSERT( pxPreviousWakeTime ); configASSERT( ( xTimeIncrement > 0U ) ); - configASSERT( xTaskGetSchedulerState() == taskSCHEDULER_RUNNING ); + configASSERT( xTaskGetSchedulerState() != taskSCHEDULER_SUSPENDED ); taskENTER_CRITICAL(&xTaskQueueMutex); // vTaskSuspendAll(); @@ -1435,7 +1435,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode /* A delay time of zero just forces a reschedule. */ if( xTicksToDelay > ( TickType_t ) 0U ) { - configASSERT( xTaskGetSchedulerState() == taskSCHEDULER_RUNNING ); + configASSERT( xTaskGetSchedulerState() != taskSCHEDULER_SUSPENDED ); taskENTER_CRITICAL(&xTaskQueueMutex); // vTaskSuspendAll(); { @@ -1818,7 +1818,7 @@ static void prvAddNewTaskToReadyList( TCB_t *pxNewTCB, TaskFunction_t pxTaskCode if( xSchedulerRunning != pdFALSE ) { /* The current task has just been suspended. */ - configASSERT( xTaskGetSchedulerState() == taskSCHEDULER_RUNNING ); + configASSERT( xTaskGetSchedulerState() != taskSCHEDULER_SUSPENDED ); portYIELD_WITHIN_API(); } else @@ -2212,9 +2212,9 @@ BaseType_t xTaskResumeAll( void ) TCB_t *pxTCB; BaseType_t xAlreadyYielded = pdFALSE; - /* If uxSchedulerSuspended[ xPortGetCoreID() ] is zero then this function does not match a + /* If scheduler state is `taskSCHEDULER_RUNNING` then this function does not match a previous call to vTaskSuspendAll(). */ - configASSERT( xTaskGetSchedulerState() == taskSCHEDULER_SUSPENDED ); + configASSERT( xTaskGetSchedulerState() != taskSCHEDULER_RUNNING ); /* It is possible that an ISR caused a task to be removed from an event list while the scheduler was suspended. If this was the case then the removed task will have been added to the xPendingReadyList. Once the