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

fix(freertos): Fixed memory leak issue in vTaskDeleteWithCaps()

Browse Source 
vTaskDeleteWithCaps() leaked memory when a task uses the API to delete
itself. This commit adds a fix to avoid the memory leak.

Closes https://github.com/espressif/esp-idf/issues/14222
This commit is contained in:
Sudeep Mohanty 2024-07-23 16:19:24 +02:00
parent 5f3f68fa58
commit 987df81e58
5 changed files with 207 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
components/freertos/FreeRTOS-Kernel-SMP/portable/linux/include/freertos/portmacro.h
View File

@ -252,6 +252,12 @@ static inline BaseType_t xPortInIsrContext(void)
return xPortCheckIfInISR();
}
static inline void vPortAssertIfInISR(void)
{
/* Assert if the interrupt nesting count is > 0 */
configASSERT(xPortInIsrContext() == 0);
}
// xPortInterruptedFromISRContext() is only used in panic handler and core dump,
// both probably not relevant on POSIX sim.
//BaseType_t xPortInterruptedFromISRContext(void);
@ -309,7 +315,7 @@ extern void vPortCancelThread( void *pxTaskToDelete );
* are always a full memory barrier. ISRs are emulated as signals
* which also imply a full memory barrier.
*
* Thus, only a compilier barrier is needed to prevent the compiler
* Thus, only a compiler barrier is needed to prevent the compiler
* reordering.
*/
#define portMEMORY_BARRIER() __asm volatile( "" ::: "memory" )

13
components/freertos/FreeRTOS-Kernel/portable/linux/include/freertos/portmacro_idf.h
View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -104,6 +104,17 @@ static inline BaseType_t xPortInIsrContext(void)
return xPortCheckIfInISR();
}
static inline void vPortAssertIfInISR(void)
{
/* Assert if the interrupt nesting count is > 0 */
configASSERT(xPortInIsrContext() == 0);
}
/**
* @brief Assert if in ISR context
*/
#define portASSERT_IF_IN_ISR() vPortAssertIfInISR()
#if CONFIG_FREERTOS_ENABLE_STATIC_TASK_CLEAN_UP
/* If enabled, users must provide an implementation of vPortCleanUpTCB() */
extern void vPortCleanUpTCB ( void *pxTCB );

131
components/freertos/esp_additions/idf_additions.c
View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -21,6 +21,8 @@
#include "freertos/timers.h"
#include "freertos/idf_additions.h"
#include "esp_heap_caps.h"
#include "esp_log.h"
#include "freertos/portmacro.h"
/* -------------------------------------------- Creation With Memory Caps ------------------------------------------- */
@ -81,21 +83,128 @@ err:
#if ( configSUPPORT_STATIC_ALLOCATION == 1 )
static void prvTaskDeleteWithCapsTask( void * pvParameters )
{
TaskHandle_t xTaskToDelete = ( TaskHandle_t ) pvParameters;
/* The task to be deleted must not be running */
configASSERT( eRunning != eTaskGetState( xTaskToDelete ) );
/* Delete the WithCaps task */
vTaskDeleteWithCaps( xTaskToDelete );
/* Delete the temporary clean up task */
vTaskDelete( NULL );
}
void vTaskDeleteWithCaps( TaskHandle_t xTaskToDelete )
{
BaseType_t xResult;
StaticTask_t * pxTaskBuffer;
StackType_t * puxStackBuffer;
/* THIS FUNCTION SHOULD NOT BE CALLED FROM AN INTERRUPT CONTEXT. */
/*TODO: Update it to use portASSERT_IF_IN_ISR() instead. (IDF-10540) */
vPortAssertIfInISR();
xResult = xTaskGetStaticBuffers( xTaskToDelete, &puxStackBuffer, &pxTaskBuffer );
configASSERT( xResult == pdTRUE );
TaskHandle_t xCurrentTaskHandle = xTaskGetCurrentTaskHandle();
configASSERT( xCurrentTaskHandle != NULL );
/* Delete the task */
vTaskDelete( xTaskToDelete );
if( ( xTaskToDelete == NULL ) || ( xTaskToDelete == xCurrentTaskHandle ) )
{
/* The WithCaps task is deleting itself. While, the task can put itself on the
* xTasksWaitingTermination list via the vTaskDelete() call, the idle
* task will not free the task TCB and stack memories we created statically
* during xTaskCreateWithCaps() or xTaskCreatePinnedToCoreWithCaps(). This
* task will never be rescheduled once it is on the xTasksWaitingTermination
* list and will not be able to clear the memories. Therefore, it will leak memory.
*
* To avoid this, we create a new "temporary clean up" task to delete the current task.
* This task is created at the priority of the task to be deleted with the same core
* affitinty. Its limited purpose is to delete the self-deleting task created WithCaps.
*
* This approach has the following problems -
* 1. Once a WithCaps task deletes itself via vTaskDeleteWithCaps(), it may end up in the
* suspended tasks lists for a short time before being deleted. This can give an incorrect
* picture about the system state.
*
* 2. This approach is wasteful and can be error prone. The temporary clean up task will need
* system resources to get scheduled and cleanup the WithCaps task. It can be a problem if the system
* has several self-deleting WithCaps tasks.
*
* TODO: A better approach could be either -
*
* 1. Delegate memory management to the application/user. This way the kernel needn't bother about freeing
* the memory (like other static memory task creation APIs like xTaskCreateStatic()) (IDF-10521)
*
* 2. Have a post deletion hook/callback from the IDLE task to notify higher layers when it is safe to
* perform activities such as clearing up the TCB and stack memories. (IDF-10522) */
if( xTaskCreatePinnedToCore( ( TaskFunction_t ) prvTaskDeleteWithCapsTask, "prvTaskDeleteWithCapsTask", configMINIMAL_STACK_SIZE, xCurrentTaskHandle, uxTaskPriorityGet( xTaskToDelete ), NULL, xPortGetCoreID() ) != pdFAIL )
{
/* Although the current task should get preemted immediately when prvTaskDeleteWithCapsTask is created,
* for safety, we suspend the current task and wait for prvTaskDeleteWithCapsTask to delete it. */
vTaskSuspend( xTaskToDelete );
/* Free the memory buffers */
heap_caps_free( puxStackBuffer );
vPortFree( pxTaskBuffer );
/* Should never reach here */
ESP_LOGE( "freertos_additions", "%s: Failed to suspend the task to be deleted", __func__ );
abort();
}
else
{
/* Failed to create the task to delete the current task. */
ESP_LOGE( "freertos_additions", "%s: Failed to create the task to delete the current task", __func__ );
abort();
}
}
#if ( configNUM_CORES > 1 )
else if( eRunning == eTaskGetState( xTaskToDelete ) )
{
/* The WithCaps task is running on another core.
* We suspend the task first and then delete it. */
vTaskSuspend( xTaskToDelete );
/* Wait for the task to be suspended */
while( eRunning == eTaskGetState( xTaskToDelete ) )
{
portYIELD_WITHIN_API();
}
BaseType_t xResult;
StaticTask_t * pxTaskBuffer;
StackType_t * puxStackBuffer;
xResult = xTaskGetStaticBuffers( xTaskToDelete, &puxStackBuffer, &pxTaskBuffer );
configASSERT( xResult == pdTRUE );
configASSERT( puxStackBuffer != NULL );
configASSERT( pxTaskBuffer != NULL );
/* Delete the task */
vTaskDelete( xTaskToDelete );
/* Free the memory buffers */
heap_caps_free( puxStackBuffer );
vPortFree( pxTaskBuffer );
}
#endif /* if ( configNUM_CORES > 1 ) */
else
{
/* The WithCaps task is not running and is being deleted
* from another task's context. */
configASSERT( eRunning != eTaskGetState( xTaskToDelete ) );
BaseType_t xResult;
StaticTask_t * pxTaskBuffer;
StackType_t * puxStackBuffer;
xResult = xTaskGetStaticBuffers( xTaskToDelete, &puxStackBuffer, &pxTaskBuffer );
configASSERT( xResult == pdTRUE );
configASSERT( puxStackBuffer != NULL );
configASSERT( pxTaskBuffer != NULL );
/* We can delete the task and free the memory buffers. */
vTaskDelete( xTaskToDelete );
/* Free the memory buffers */
heap_caps_free( puxStackBuffer );
vPortFree( pxTaskBuffer );
} /* if( ( xTaskToDelete == NULL ) || ( xTaskToDelete == xCurrentTaskHandle ) ) */
}
#endif /* if ( configSUPPORT_STATIC_ALLOCATION == 1 ) */

5
components/freertos/esp_additions/include/freertos/idf_additions.h
View File

@ -362,6 +362,11 @@ uint8_t * pxTaskGetStackStart( TaskHandle_t xTask );
* @brief Deletes a task previously created using xTaskCreateWithCaps() or
* xTaskCreatePinnedToCoreWithCaps()
*
* @note It is recommended to use this API to delete tasks from another task's
* context, rather than self-deletion. When the task is being deleted, it is vital
* to ensure that it is not running on another core. This API must not be called
* from an interrupt context.
*
* @param xTaskToDelete A handle to the task to be deleted
*/
void vTaskDeleteWithCaps( TaskHandle_t xTaskToDelete );

65
components/freertos/test_apps/freertos/misc/test_idf_additions.c
View File

@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: 2023 Espressif Systems (Shanghai) CO LTD
* SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD
*
* SPDX-License-Identifier: Apache-2.0
*/
@ -40,7 +40,17 @@ static void task_with_caps(void *arg)
vTaskSuspend(NULL);
}
TEST_CASE("IDF additions: Task creation with memory caps", "[freertos]")
static void task_with_caps_self_delete(void *arg)
{
/* Wait for the unity task to indicate that this task should delete itself */
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
/* Although it is not recommended to self-delete a task with memory caps but this
* is done intentionally to test for memory leaks */
vTaskDeleteWithCaps(NULL);
}
TEST_CASE("IDF additions: Task creation with memory caps and deletion from another task", "[freertos]")
{
TaskHandle_t task_handle = NULL;
StackType_t *puxStackBuffer;
@ -59,6 +69,57 @@ TEST_CASE("IDF additions: Task creation with memory caps", "[freertos]")
vTaskDeleteWithCaps(task_handle);
}
TEST_CASE("IDF additions: Task creation with memory caps and self deletion", "[freertos]")
{
TaskHandle_t task_handle = NULL;
StackType_t *puxStackBuffer;
StaticTask_t *pxTaskBuffer;
// Create a task with caps
TEST_ASSERT_EQUAL(pdPASS, xTaskCreatePinnedToCoreWithCaps(task_with_caps_self_delete, "task", 4096, (void *)xTaskGetCurrentTaskHandle(), UNITY_FREERTOS_PRIORITY + 1, &task_handle, UNITY_FREERTOS_CPU, OBJECT_MEMORY_CAPS));
TEST_ASSERT_NOT_EQUAL(NULL, task_handle);
// Get the task's memory
TEST_ASSERT_EQUAL(pdTRUE, xTaskGetStaticBuffers(task_handle, &puxStackBuffer, &pxTaskBuffer));
TEST_ASSERT(esp_ptr_in_dram(puxStackBuffer));
TEST_ASSERT(esp_ptr_in_dram(pxTaskBuffer));
// Notify the task to delete itself
xTaskNotifyGive(task_handle);
}
#if ( CONFIG_FREERTOS_NUMBER_OF_CORES > 1 )
static void task_with_caps_running_on_other_core(void *arg)
{
/* Notify the unity task that this task is running on the other core */
xTaskNotifyGive((TaskHandle_t)arg);
/* We make sure that this task is running on the other core */
while (1) {
;
}
}
TEST_CASE("IDF additions: Task creation with memory caps and deletion from another core", "[freertos]")
{
TaskHandle_t task_handle = NULL;
StackType_t *puxStackBuffer;
StaticTask_t *pxTaskBuffer;
// Create a task with caps on the other core
TEST_ASSERT_EQUAL(pdPASS, xTaskCreatePinnedToCoreWithCaps(task_with_caps_running_on_other_core, "task", 4096, (void *)xTaskGetCurrentTaskHandle(), UNITY_FREERTOS_PRIORITY + 1, &task_handle, !UNITY_FREERTOS_CPU, OBJECT_MEMORY_CAPS));
TEST_ASSERT_NOT_EQUAL(NULL, task_handle);
// Get the task's memory
TEST_ASSERT_EQUAL(pdTRUE, xTaskGetStaticBuffers(task_handle, &puxStackBuffer, &pxTaskBuffer));
TEST_ASSERT(esp_ptr_in_dram(puxStackBuffer));
TEST_ASSERT(esp_ptr_in_dram(pxTaskBuffer));
// Wait for the created task to start running
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
// Delete the task from another core
vTaskDeleteWithCaps(task_handle);
}
#endif // CONFIG_FREERTOS_NUMBER_OF_CORES > 1
TEST_CASE("IDF additions: Queue creation with memory caps", "[freertos]")
{
QueueHandle_t queue_handle;