Skip to content

Commit

Permalink
Merge branch 'fix/assert_fail_in_xtaskdeletewithcaps' into 'master'
Browse files Browse the repository at this point in the history 
Fixed occational assert failure in vTaskDeleteWithCaps()

Closes IDFGH-13294

See merge request espressif/esp-idf!33468
  • Loading branch information
@sudeep-mohanty
sudeep-mohanty committed Sep 20, 2024
2 parents 19d512e + c36674e commit c01512f
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 65 deletions.
107 changes: 42 additions & 65 deletions components/freertos/esp_additions/idf_additions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,44 @@

#if ( configSUPPORT_STATIC_ALLOCATION == 1 )

static void prvTaskDeleteWithCapsTask( void * pvParameters )
static void prvTaskDeleteWithCaps( TaskHandle_t xTaskToDelete )
{
TaskHandle_t xTaskToDelete = ( TaskHandle_t ) pvParameters;
/* Return value unused if asserts are disabled */
BaseType_t __attribute__( ( unused ) ) xResult;
StaticTask_t * pxTaskBuffer;
StackType_t * puxStackBuffer;

/* The task to be deleted must not be running.
* So we suspend the task before deleting it. */
vTaskSuspend( xTaskToDelete );

/* Wait for the task to be suspended */
while( eRunning == eTaskGetState( xTaskToDelete ) )
{
taskYIELD();
}

/* The task to be deleted must not be running */
configASSERT( eRunning != eTaskGetState( xTaskToDelete ) );

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 );
}

static void prvTaskDeleteWithCapsTask( void * pvParameters )
{
TaskHandle_t xTaskToDelete = ( TaskHandle_t ) pvParameters;

/* Delete the WithCaps task */
vTaskDeleteWithCaps( xTaskToDelete );
prvTaskDeleteWithCaps( xTaskToDelete );

/* Delete the temporary clean up task */
vTaskDelete( NULL );
Expand All @@ -98,7 +127,7 @@
void vTaskDeleteWithCaps( TaskHandle_t xTaskToDelete )
{
/* THIS FUNCTION SHOULD NOT BE CALLED FROM AN INTERRUPT CONTEXT. */
/*TODO: Update it to use portASSERT_IF_IN_ISR() instead. (IDF-10540) */
/* TODO: Update it to use portASSERT_IF_IN_ISR() instead. (IDF-10540) */
vPortAssertIfInISR();

TaskHandle_t xCurrentTaskHandle = xTaskGetCurrentTaskHandle();
Expand Down Expand Up @@ -151,60 +180,8 @@
}
}

#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();
}

// Return value unused if asserts are disabled
BaseType_t __attribute__((unused)) 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 ) );

// Return value unused if asserts are disabled
BaseType_t __attribute__((unused)) 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 ) ) */
/* Delete the WithCaps task */
prvTaskDeleteWithCaps( xTaskToDelete );
}

#endif /* if ( configSUPPORT_STATIC_ALLOCATION == 1 ) */
Expand Down Expand Up @@ -262,8 +239,8 @@

void vQueueDeleteWithCaps( QueueHandle_t xQueue )
{
// Return value unused if asserts are disabled
BaseType_t __attribute__((unused)) xResult;
/* Return value unused if asserts are disabled */
BaseType_t __attribute__( ( unused ) ) xResult;
StaticQueue_t * pxQueueBuffer;
uint8_t * pucQueueStorageBuffer;

Expand Down Expand Up @@ -335,8 +312,8 @@

void vSemaphoreDeleteWithCaps( SemaphoreHandle_t xSemaphore )
{
// Return value unused if asserts are disabled
BaseType_t __attribute__((unused)) xResult;
/* Return value unused if asserts are disabled */
BaseType_t __attribute__( ( unused ) ) xResult;
StaticSemaphore_t * pxSemaphoreBuffer;

/* Retrieve the buffer used to create the semaphore before deleting it
Expand Down Expand Up @@ -408,8 +385,8 @@
void vStreamBufferGenericDeleteWithCaps( StreamBufferHandle_t xStreamBuffer,
BaseType_t xIsMessageBuffer )
{
// Return value unused if asserts are disabled
BaseType_t __attribute__((unused)) xResult;
/* Return value unused if asserts are disabled */
BaseType_t __attribute__( ( unused ) ) xResult;
StaticStreamBuffer_t * pxStaticStreamBuffer;
uint8_t * pucStreamBufferStorageArea;

Expand Down
28 changes: 28 additions & 0 deletions components/freertos/test_apps/freertos/misc/test_idf_additions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,34 @@ TEST_CASE("IDF additions: Task creation with memory caps and self deletion", "[f
xTaskNotifyGive(task_handle);
}

#if CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY

TEST_CASE("IDF additions: Task creation with SPIRAM memory caps and self deletion stress test", "[freertos]")
{
#define TEST_NUM_TASKS 5
#define TEST_NUM_ITERATIONS 1000
TaskHandle_t task_handle[TEST_NUM_TASKS];
StackType_t *puxStackBuffer;
StaticTask_t *pxTaskBuffer;

for (int j = 0; j < TEST_NUM_ITERATIONS; j++) {
for (int i = 0; i < TEST_NUM_TASKS; i++) {
// Create a task with caps
TEST_ASSERT_EQUAL(pdPASS, xTaskCreateWithCaps(task_with_caps_self_delete, "task", 4096, NULL, UNITY_FREERTOS_PRIORITY, &task_handle[i], MALLOC_CAP_SPIRAM | MALLOC_CAP_8BIT));
TEST_ASSERT_NOT_EQUAL(NULL, task_handle);
// Get the task's memory
TEST_ASSERT_EQUAL(pdTRUE, xTaskGetStaticBuffers(task_handle[i], &puxStackBuffer, &pxTaskBuffer));
}

for (int i = 0; i < TEST_NUM_TASKS; i++) {
// Notify the task to delete itself
xTaskNotifyGive(task_handle[i]);
}
}
}

#endif /* CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY */

#if ( CONFIG_FREERTOS_NUMBER_OF_CORES > 1 )

static void task_with_caps_running_on_other_core(void *arg)
Expand Down
1 change: 1 addition & 0 deletions components/freertos/test_apps/freertos/sdkconfig.ci.psram
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ CONFIG_IDF_TARGET="esp32"
# Enable SPIRAM
CONFIG_SPIRAM=y
CONFIG_SPIRAM_OCCUPY_NO_HOST=y
CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY=y

# Disable encrypted flash reads/writes to save IRAM in this build configuration
CONFIG_SPI_FLASH_ENABLE_ENCRYPTED_READ_WRITE=n
Expand Down

0 comments on commit c01512f

Please sign in to comment.