Merge branch 'fix/assert_fail_in_xtaskdeletewithcaps' into 'master'

Fixed occational assert failure in vTaskDeleteWithCaps()

Closes IDFGH-13294

See merge request espressif/esp-idf!33468

components/freertos/esp_additions/idf_additions.c

@@ -81,15 +81,44 @@ err:
 
 #if ( configSUPPORT_STATIC_ALLOCATION == 1 )
 
+    static void prvTaskDeleteWithCaps( TaskHandle_t xTaskToDelete )
+    {
+        /* 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();
+        }
+
+        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;
 
-        /* The task to be deleted must not be running */
-        configASSERT( eRunning != eTaskGetState( xTaskToDelete ) );
-
         /* Delete the WithCaps task */
-        vTaskDeleteWithCaps( xTaskToDelete );
+        prvTaskDeleteWithCaps( xTaskToDelete );
 
         /* Delete the temporary clean up task */
         vTaskDelete( NULL );
@@ -98,7 +127,7 @@ err:
     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();
@@ -151,60 +180,8 @@ err:
             }
         }
 
-        #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 ) */
@@ -262,8 +239,8 @@ err:
 
     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;
 
@@ -335,8 +312,8 @@ err:
 
     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
@@ -408,8 +385,8 @@ err:
     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;
 

components/freertos/test_apps/freertos/misc/test_idf_additions.c

@@ -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)

components/freertos/test_apps/freertos/sdkconfig.ci.psram

@@ -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