alex/esp-idf
alex/esp-idf
1
0
Fork 0
mirror of https://github.com/espressif/esp-idf.git synced 2024-10-05 20:47:46 -04:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

Merge branch 'bugfix/freertos_event_group_unblock_race_condition_v4.3' into 'release/v4.3'

Browse Source 
FreeRTOS: Fix event group task list race condition (v4.3)

See merge request espressif/esp-idf!19104
This commit is contained in:
Mahavir Jain 2022-07-27 12:25:39 +08:00
commit 949a6a8d3f
4 changed files with 54 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
components/freertos/event_groups.c
View File

@ -535,6 +535,10 @@ BaseType_t xMatchFound = pdFALSE;
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. */
taskENTER_CRITICAL( &pxEventBits->eventGroupMux );
/* 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();
{
traceEVENT_GROUP_SET_BITS( xEventGroup, uxBitsToSet );
@ -606,6 +610,8 @@ BaseType_t xMatchFound = pdFALSE;
bit was set in the control word. */
pxEventBits->uxEventBits &= ~uxBitsToClear;
}
/* Release the previously held task list spinlock, then release the event group spinlock. */
vTaskReleaseEventListLock();
taskEXIT_CRITICAL( &pxEventBits->eventGroupMux );
return pxEventBits->uxEventBits;
@ -620,6 +626,10 @@ const List_t *pxTasksWaitingForBits = &( pxEventBits->xTasksWaitingForBits );
traceEVENT_GROUP_DELETE( xEventGroup );
taskENTER_CRITICAL( &pxEventBits->eventGroupMux );
/* 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();
{
while( listCURRENT_LIST_LENGTH( pxTasksWaitingForBits ) > ( UBaseType_t ) 0 )
{
@ -629,6 +639,8 @@ const List_t *pxTasksWaitingForBits = &( pxEventBits->xTasksWaitingForBits );
xTaskRemoveFromUnorderedEventList( pxTasksWaitingForBits->xListEnd.pxNext, eventUNBLOCKED_DUE_TO_BIT_SET );
}
}
/* Release the previously held task list spinlock. */
vTaskReleaseEventListLock();
taskEXIT_CRITICAL( &pxEventBits->eventGroupMux );
#if( ( configSUPPORT_DYNAMIC_ALLOCATION == 1 ) && ( configSUPPORT_STATIC_ALLOCATION == 0 ) )

17
components/freertos/include/freertos/task.h
View File

@ -2369,6 +2369,23 @@ void vTaskPlaceOnUnorderedEventList( List_t * pxEventList, const TickType_t xIte
*/
void vTaskPlaceOnEventListRestricted( List_t * const pxEventList, TickType_t xTicksToWait, const BaseType_t xWaitIndefinitely ) PRIVILEGED_FUNCTION;
/*
* 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 );
/*
* THIS FUNCTION MUST NOT BE USED FROM APPLICATION CODE. IT IS AN
* INTERFACE WHICH IS FOR THE EXCLUSIVE USE OF THE SCHEDULER.

12
components/freertos/tasks.c
View File

@ -3563,6 +3563,18 @@ UBaseType_t i, uxTargetCPU;
}
/*-----------------------------------------------------------*/
void vTaskTakeEventListLock( void )
{
/* We call the tasks.c critical section macro to take xTaskQueueMutex */
taskENTER_CRITICAL(&xTaskQueueMutex);
}
void vTaskReleaseEventListLock( void )
{
/* We call the tasks.c critical section macro to release xTaskQueueMutex */
taskEXIT_CRITICAL(&xTaskQueueMutex);
}
BaseType_t xTaskRemoveFromUnorderedEventList( ListItem_t * pxEventListItem, const TickType_t xItemValue )
{
TCB_t *pxUnblockedTCB;

18
components/freertos/test/test_freertos_eventgroups.c
View File

@ -7,6 +7,7 @@
#include "freertos/event_groups.h"
#include "driver/timer.h"
#include "unity.h"
#include "test_utils.h"
#ifdef CONFIG_IDF_TARGET_ESP32S2
#define int_clr_timers int_clr
@ -40,7 +41,8 @@ static void task_event_group_call_response(void *param)
printf("Task %d done\n", task_num);
xSemaphoreGive(done_sem);
vTaskDelete(NULL);
// Wait to be deleted
vTaskSuspend(NULL);
}
TEST_CASE("FreeRTOS Event Groups", "[freertos]")
@ -48,6 +50,8 @@ TEST_CASE("FreeRTOS Event Groups", "[freertos]")
eg = xEventGroupCreate();
done_sem = xSemaphoreCreateCounting(NUM_TASKS, 0);
TaskHandle_t task_handles[NUM_TASKS];
/* Note: task_event_group_call_response all have higher priority than this task, so on this core
they will always preempt this task.
@ -55,7 +59,7 @@ TEST_CASE("FreeRTOS Event Groups", "[freertos]")
or they get out of sync.
*/
for (int c = 0; c < NUM_TASKS; c++) {
xTaskCreatePinnedToCore(task_event_group_call_response, "tsk_call_resp", 4096, (void *)c, configMAX_PRIORITIES - 1, NULL, c % portNUM_PROCESSORS);
xTaskCreatePinnedToCore(task_event_group_call_response, "tsk_call_resp", 4096, (void *)c, configMAX_PRIORITIES - 1, &task_handles[c], c % portNUM_PROCESSORS);
}
/* Tasks all start instantly, but this task will resume running at the same time as the higher priority tasks on the
@ -66,15 +70,19 @@ TEST_CASE("FreeRTOS Event Groups", "[freertos]")
/* signal all tasks with "CALL" bit... */
xEventGroupSetBits(eg, BIT_CALL);
/* Only wait for 1 tick, the wakeup should be immediate... */
TEST_ASSERT_EQUAL_HEX16(ALL_RESPONSE_BITS, xEventGroupWaitBits(eg, ALL_RESPONSE_BITS, true, true, 1));
/* Wait until all tasks have set their respective response bits */
TEST_ASSERT_EQUAL_HEX16(ALL_RESPONSE_BITS, xEventGroupWaitBits(eg, ALL_RESPONSE_BITS, true, true, portMAX_DELAY));
}
/* Ensure all tasks cleaned up correctly */
/* Ensure all tasks have suspend themselves */
for (int c = 0; c < NUM_TASKS; c++) {
TEST_ASSERT( xSemaphoreTake(done_sem, 100/portTICK_PERIOD_MS) );
}
for (int c = 0; c < NUM_TASKS; c++) {
test_utils_task_delete(task_handles[c]);
}
vSemaphoreDelete(done_sem);
vEventGroupDelete(eg);
}