freertos: Fix event group task list race condition

FreeRTOS synchronization primitives (e.g., queues, eventgroups) use various event lists (i.e., task lists) to track what
tasks are blocked on a current primitive. Usually these event lists are accessed via one of the event lists functions
(such as vTask[PlaceOn|RemoveFrom]UnorderedEventList()), which in turn ensure that the global task list spinlock
(xTaskQueueMutex) is taken when accessing these lists.

However, some functions in event_groups.c manually traverse their event lists. Thus if a tick interrupt occurs on
another core during traversal and that tick interrupt unblocks a task on the event list being traversed, the event list
will be corrupted.

This commit modifies the following event_groups.c functions so that they take the global task list lock before
traversing their event list.

- xEventGroupSetBits()
- vEventGroupDelete()
This commit is contained in:
Darian Leung 2022-07-19 11:20:13 +08:00
parent f0a3ccc732
commit d465316f0b
3 changed files with 49 additions and 0 deletions

View File

@ -600,6 +600,10 @@ EventBits_t xEventGroupSetBits( EventGroupHandle_t xEventGroup,
pxListEnd = listGET_END_MARKER( pxList ); /*lint !e826 !e740 !e9087 The mini list structure is used as the list end to save RAM. This is checked and valid. */ pxListEnd = listGET_END_MARKER( pxList ); /*lint !e826 !e740 !e9087 The mini list structure is used as the list end to save RAM. This is checked and valid. */
#ifdef ESP_PLATFORM // IDF-3755 #ifdef ESP_PLATFORM // IDF-3755
taskENTER_CRITICAL(); taskENTER_CRITICAL();
/* The critical section above only takes the event groups spinlock. However, we are about to traverse a task list.
* Thus we need call the function below to take the task list spinlock located in tasks.c. Not doing so will risk
* the task list's being changed while be are traversing it. */
vTaskTakeEventListLock();
#else #else
vTaskSuspendAll(); vTaskSuspendAll();
#endif // ESP_PLATFORM #endif // ESP_PLATFORM
@ -675,6 +679,8 @@ EventBits_t xEventGroupSetBits( EventGroupHandle_t xEventGroup,
pxEventBits->uxEventBits &= ~uxBitsToClear; pxEventBits->uxEventBits &= ~uxBitsToClear;
} }
#ifdef ESP_PLATFORM // IDF-3755 #ifdef ESP_PLATFORM // IDF-3755
/* Release the previously held task list spinlock, then release the event group spinlock. */
vTaskReleaseEventListLock();
taskEXIT_CRITICAL(); taskEXIT_CRITICAL();
#else #else
( void ) xTaskResumeAll(); ( void ) xTaskResumeAll();
@ -693,6 +699,12 @@ void vEventGroupDelete( EventGroupHandle_t xEventGroup )
// IDF-3755 // IDF-3755
taskENTER_CRITICAL(); taskENTER_CRITICAL();
#ifdef ESP_PLATFORM
/* The critical section above only takes the event groups spinlock. However, we are about to traverse a task list.
* Thus we need call the function below to take the task list spinlock located in tasks.c. Not doing so will risk
* the task list's being changed while be are traversing it. */
vTaskTakeEventListLock();
#endif
{ {
while( listCURRENT_LIST_LENGTH( pxTasksWaitingForBits ) > ( UBaseType_t ) 0 ) while( listCURRENT_LIST_LENGTH( pxTasksWaitingForBits ) > ( UBaseType_t ) 0 )
{ {
@ -702,6 +714,10 @@ void vEventGroupDelete( EventGroupHandle_t xEventGroup )
vTaskRemoveFromUnorderedEventList( pxTasksWaitingForBits->xListEnd.pxNext, eventUNBLOCKED_DUE_TO_BIT_SET ); vTaskRemoveFromUnorderedEventList( pxTasksWaitingForBits->xListEnd.pxNext, eventUNBLOCKED_DUE_TO_BIT_SET );
} }
} }
#ifdef ESP_PLATFORM
/* Release the previously held task list spinlock. */
vTaskReleaseEventListLock();
#endif
taskEXIT_CRITICAL(); taskEXIT_CRITICAL();
#if ( ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) && ( configSUPPORT_STATIC_ALLOCATION == 0 ) ) #if ( ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) && ( configSUPPORT_STATIC_ALLOCATION == 0 ) )

View File

@ -3367,6 +3367,25 @@ void vTaskPlaceOnEventListRestricted( List_t * const pxEventList,
TickType_t xTicksToWait, TickType_t xTicksToWait,
const BaseType_t xWaitIndefinitely ) PRIVILEGED_FUNCTION; const BaseType_t xWaitIndefinitely ) PRIVILEGED_FUNCTION;
#ifdef ESP_PLATFORM
/*
* THIS FUNCTION MUST NOT BE USED FROM APPLICATION CODE. IT IS AN
* INTERFACE WHICH IS FOR THE EXCLUSIVE USE OF THE SCHEDULER.
*
* This function is a wrapper to take the "xTaskQueueMutex" spinlock of tasks.c.
* This lock is taken whenver any of the task lists or event lists are
* accessed/modified, such as when adding/removing tasks to/from the delayed
* task list or various event lists.
*
* This functions is meant to be called by xEventGroupSetBits() and
* vEventGroupDelete() as both those functions will access event lists (instead
* of delegating the entire responsibility to one of vTask...EventList()
* functions).
*/
void vTaskTakeEventListLock( void );
void vTaskReleaseEventListLock( void );
#endif // ESP_PLATFORM
/* /*
* THIS FUNCTION MUST NOT BE USED FROM APPLICATION CODE. IT IS AN * THIS FUNCTION MUST NOT BE USED FROM APPLICATION CODE. IT IS AN
* INTERFACE WHICH IS FOR THE EXCLUSIVE USE OF THE SCHEDULER. * INTERFACE WHICH IS FOR THE EXCLUSIVE USE OF THE SCHEDULER.

View File

@ -3733,6 +3733,20 @@ BaseType_t xTaskRemoveFromEventList( const List_t * const pxEventList )
} }
/*-----------------------------------------------------------*/ /*-----------------------------------------------------------*/
#ifdef ESP_PLATFORM
void vTaskTakeEventListLock( void )
{
/* We call the tasks.c critical section macro to take xTaskQueueMutex */
taskENTER_CRITICAL();
}
void vTaskReleaseEventListLock( void )
{
/* We call the tasks.c critical section macro to release xTaskQueueMutex */
taskEXIT_CRITICAL();
}
#endif // ESP_PLATFORM
void vTaskRemoveFromUnorderedEventList( ListItem_t * pxEventListItem, void vTaskRemoveFromUnorderedEventList( ListItem_t * pxEventListItem,
const TickType_t xItemValue ) const TickType_t xItemValue )
{ {