ipc_task() can wake up blocking task early. (IDFGH-6939)

[pickaxprograms]

• Development Kit: none
• Module or chip used: ESP32-WROVER
• IDF version : v4.4-12-ga0404a9ed2
• Build System: idf.py
• Compiler version: xtensa-esp32-elf-gcc (crosstool-NG esp-2021r2-patch3) 8.4.0
• Operating System: Windows
• (Windows only) environment type: WSL
• Using an IDE?: No

Problem Description

If two different threads on the same core attempt an esp_ipc_call() at the same time it is possible for one to be woken early.

Steps to reproduce

In two different threads on the same core,
esp_ipc_call()
esp_ipc_call_blocking()
If the function in esp_ipc_call() takes a while to run or blocks, a second thread calling esp_ipc_call_blocking() can be woken before it should be.

Theoretical analysis:

static void IRAM_ATTR ipc_task(void* arg)
{
...
            // Original code
            if (s_ipc_wait[cpuid] == IPC_WAIT_FOR_START) {
                xSemaphoreGive(s_ipc_ack[cpuid]);
            }
            (*func)(arg);
            if (s_ipc_wait[cpuid] == IPC_WAIT_FOR_END) {
                xSemaphoreGive(s_ipc_ack[cpuid]);
            }
...
}

The first pass in ipc_task() handling the esp_ipc_call() will give the ack semaphore (as s_ipc_wait[]=IPC_WAIT_FOR_START) and then execute (*func)(arg). When the ack semaphore is given, that esp_ipc_call() can exit and the call to esp_ipc_call_blocking() can now update s_ipc_wait[] and s_ipc_ack[]. When (*func)(arg) returns, it checks the new value of s_ipc_wait[] which is now IPC_WAIT_FOR_END and then gives the new s_ipc_ack[] before the the (*func)() from esp_ipc_call_blocking() had a chance to be called. Both of these need to be cached first.

        esp_ipc_wait_t ipc_wait = s_ipc_wait[cpuid];
        SemaphoreHandle_t ack = s_ipc_ack[cpuid];

        if (ipc_wait == IPC_WAIT_FOR_START) {
            xSemaphoreGive(ack);
        }
        (*func)(arg);
        if (ipc_wait == IPC_WAIT_FOR_END) {
            xSemaphoreGive(ack);
        }

[KonstantinKondrashov]

Hi @pickaxprograms!
Thank you for noticing this. I have created a fix, it should be merged soon (backports will also be provided).