mirror of
https://github.com/espressif/esp-idf.git
synced 2024-09-19 14:26:01 -04:00
Merge branch 'fix/vtaskdeletewithcaps_leaks_memory_v5.1' into 'release/v5.1'
fix(freertos): Fixed memory leak issue in vTaskDeleteWithCaps() (v5.1) See merge request espressif/esp-idf!32401
This commit is contained in:
commit
fa8cd334b7
@ -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
|
* SPDX-License-Identifier: Apache-2.0
|
||||||
*/
|
*/
|
||||||
@ -95,6 +95,17 @@ static inline BaseType_t xPortInIsrContext(void)
|
|||||||
return xPortCheckIfInISR();
|
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()
|
||||||
|
|
||||||
#ifdef __cplusplus
|
#ifdef __cplusplus
|
||||||
}
|
}
|
||||||
#endif
|
#endif
|
||||||
|
@ -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
|
* SPDX-License-Identifier: Apache-2.0
|
||||||
*/
|
*/
|
||||||
@ -16,6 +16,8 @@
|
|||||||
#include "freertos/timers.h"
|
#include "freertos/timers.h"
|
||||||
#include "freertos/idf_additions.h"
|
#include "freertos/idf_additions.h"
|
||||||
#include "esp_heap_caps.h"
|
#include "esp_heap_caps.h"
|
||||||
|
#include "esp_log.h"
|
||||||
|
#include "freertos/portmacro.h"
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* This file contains the implementation for some the functions in
|
* This file contains the implementation for some the functions in
|
||||||
@ -78,21 +80,133 @@ err:
|
|||||||
return pdFAIL;
|
return pdFAIL;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#endif /* if ( configSUPPORT_STATIC_ALLOCATION == 1 ) */
|
||||||
|
/*----------------------------------------------------------*/
|
||||||
|
|
||||||
|
#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 )
|
void vTaskDeleteWithCaps( TaskHandle_t xTaskToDelete )
|
||||||
{
|
{
|
||||||
BaseType_t xResult;
|
/* THIS FUNCTION SHOULD NOT BE CALLED FROM AN INTERRUPT CONTEXT. */
|
||||||
StaticTask_t * pxTaskBuffer;
|
/*TODO: Update it to use portASSERT_IF_IN_ISR() instead. (IDF-10540) */
|
||||||
StackType_t * puxStackBuffer;
|
vPortAssertIfInISR();
|
||||||
|
|
||||||
xResult = xTaskGetStaticBuffers( xTaskToDelete, &puxStackBuffer, &pxTaskBuffer );
|
TaskHandle_t xCurrentTaskHandle = xTaskGetCurrentTaskHandle();
|
||||||
configASSERT( xResult == pdTRUE );
|
configASSERT( xCurrentTaskHandle != NULL );
|
||||||
|
|
||||||
/* Delete the task */
|
if( ( xTaskToDelete == NULL ) || ( xTaskToDelete == xCurrentTaskHandle ) )
|
||||||
vTaskDelete( xTaskToDelete );
|
{
|
||||||
|
/* 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 */
|
/* Should never reach here */
|
||||||
heap_caps_free( puxStackBuffer );
|
ESP_LOGE( "freertos_additions", "%s: Failed to suspend the task to be deleted", __func__ );
|
||||||
vPortFree( pxTaskBuffer );
|
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 ) ) */
|
||||||
}
|
}
|
||||||
|
|
||||||
/* ---------------------------------- Queue --------------------------------- */
|
/* ---------------------------------- Queue --------------------------------- */
|
||||||
|
@ -279,6 +279,11 @@
|
|||||||
* @brief Deletes a task previously created using xTaskCreateWithCaps() or
|
* @brief Deletes a task previously created using xTaskCreateWithCaps() or
|
||||||
* xTaskCreatePinnedToCoreWithCaps()
|
* 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
|
* @param xTaskToDelete A handle to the task to be deleted
|
||||||
*/
|
*/
|
||||||
void vTaskDeleteWithCaps( TaskHandle_t xTaskToDelete );
|
void vTaskDeleteWithCaps( TaskHandle_t xTaskToDelete );
|
||||||
|
@ -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
|
* SPDX-License-Identifier: Apache-2.0
|
||||||
*/
|
*/
|
||||||
@ -40,7 +40,17 @@ static void task_with_caps(void *arg)
|
|||||||
vTaskSuspend(NULL);
|
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;
|
TaskHandle_t task_handle = NULL;
|
||||||
StackType_t *puxStackBuffer;
|
StackType_t *puxStackBuffer;
|
||||||
@ -59,6 +69,57 @@ TEST_CASE("IDF additions: Task creation with memory caps", "[freertos]")
|
|||||||
vTaskDeleteWithCaps(task_handle);
|
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]")
|
TEST_CASE("IDF additions: Queue creation with memory caps", "[freertos]")
|
||||||
{
|
{
|
||||||
QueueHandle_t queue_handle;
|
QueueHandle_t queue_handle;
|
||||||
|
Loading…
Reference in New Issue
Block a user