Crash when I add mutex

Hello,

I have an HTTPS server application that uses FreeRTOS.
To protect the access to the useful data to generate the dynamic HTTP files, I added a mutex protection:

static SemaphoreHandle_t readerMutex;
static SemaphoreHandle_t writerMutex;
static int readersCounter;

static void initReaderWriter(void){
    readerMutex = xSemaphoreCreateMutex();
    if(readerMutex == NULL){
        while(1);
    }
    
    writerMutex = xSemaphoreCreateMutex();
    if(readerMutex == NULL){
        while(1);
    }
}

// ****************
static void openReader(void){
    xSemaphoreTake(readerMutex, portMAX_DELAY); // portMAX_DELAY = 0xffffffffUL 
        readersCounter++;
        if(readersCounter == 1){ // if first reader
            xSemaphoreTake(writerMutex, portMAX_DELAY);
        }
    xSemaphoreGive(readerMutex);
}

static void closeReader(void){
    xSemaphoreTake(readerMutex, portMAX_DELAY); // portMAX_DELAY = 0xffffffffUL   
        readersCounter--;
        if(readersCounter == 0){ // if last reader
            xSemaphoreGive(writerMutex);
        }    
    xSemaphoreGive(readerMutex);
}

// ********************
static void openWriter(void){
    xSemaphoreTake(writerMutex, portMAX_DELAY);
}

static void closeWriter(void){
    xSemaphoreGive(writerMutex);
}

openWriter/openWriter are used for HTTP POST requests and openReader/closeReader are used for HTTP GET requests.
When I make HTTP GET requests with a single HTTPS connection, there is no problem but when there are 2 connections, FreeRTOS crashes. If I do not implement this mutex protection, FreeRTOS does not crash.
FreeRTOS does not crash when code is executed between the open/close functions… this is very strange.
I tried increasing the memory allocation for tasks but it’s the same thing.
I have other tasks that use mutexes and there is no problem.

What is the problem ?

can you determine exactly where FreeRTOS “crashes?” What exactly do you mean by “crash?” A fault or a deadlock?

As a side note: Your second test on readerMutex !=0 should probably be a check on writerMutex, no?

1 Like

Also, if you need to implement a true R/W lock (either a writer or arbitrarily many readers), the code is not correct as it wii allow readers to enter when a writer is active. In addition, there is deadlock potential in your implementation. I recommend to query this forum for reader/writer lock; the issue has been discussed before.

It isn’t a deadlock : program reboot at the line (in freeRTOS) :
if( pxList->pxIndex == pxItemToRemove )

Error code (PIC32MZ) :

load TLB miss/hit

I think my code to protect resource is good because this test code works :

static SemaphoreHandle_t readerMutex;
static SemaphoreHandle_t writerMutex;
static int readersCounter;

static void openReader(void){
    xSemaphoreTake(readerMutex, portMAX_DELAY); // portMAX_DELAY = 0xffffffffUL 
        readersCounter++;
        if(readersCounter == 1){ // if first reader
            xSemaphoreTake(writerMutex, portMAX_DELAY);
        }
    xSemaphoreGive(readerMutex);
}

static void closeReader(void){
    xSemaphoreTake(readerMutex, portMAX_DELAY); // portMAX_DELAY = 0xffffffffUL   
        readersCounter--;
        if(readersCounter == 0){ // if last reader
            xSemaphoreGive(writerMutex);
        }    
    xSemaphoreGive(readerMutex);
}


static SemaphoreHandle_t uartMutex;

static void uartSendStr(const char * buf){
    if(*buf == '\0'){
        return;
    }
    
    xSemaphoreTake(uartMutex, portMAX_DELAY); // portMAX_DELAY = 0xffffffffUL 
        while(*buf != '\0'){
            while (U5STAbits.UTXBF);
            U5TXREG = *buf++;
        }
    
    xSemaphoreGive(uartMutex);
    
    //taskYIELD();
}

static void freeRTOS_task(void *param){

    vTaskDelay(5000 / portTICK_PERIOD_MS);
    
    uartSendStr(param);
    
    vTaskDelay(5000 / portTICK_PERIOD_MS);
    
    while(1){
        openReader();
             uartSendStr(param);
            //vTaskDelay(1);
        closeReader();
    }
}

void freeRTOS_taskInit(void){
    TaskHandle_t handle;

    uartMutex = xSemaphoreCreateMutex();
    if(uartMutex== NULL){
        while(1);
    }
    
    readerMutex = xSemaphoreCreateMutex();
    if(readerMutex == NULL){
        while(1);
    }
    
    writerMutex = xSemaphoreCreateMutex();
    if(readerMutex == NULL){
        while(1);
    }
    
    if(
        xTaskCreate(freeRTOS_task, "Task1",
        configMINIMAL_STACK_SIZE + 1000, "Task1\r\n",
        1, &handle) != pdPASS)
    {
        while(1);
    }
    
    if(
        xTaskCreate(freeRTOS_task, "Task2",
        configMINIMAL_STACK_SIZE + 1000, "Task2\r\n",
        1, &handle) != pdPASS)
    {
        while(1);
    }
}

One possible problem is that muteces can only be released from the same thread that claimed it which may not be guaranteed in your implementation. Try using a binary semaphore for the writer…

When there is a writer, I don’t want to allow a reader. And when there is a reader, I don’t want to allow a writer.
… Where is the problem in my code ?

My application crashes even if there is no writter : I do only HTTP GET requests.

Are you sure the release should be in the same thread? Where is it written in the documentation?
… if it’s true, it must be the cause

It is the very definition of a mutex that this must hold.

You may want to check the return values of your take/give calls (always a good practice).

Just a wild guess if this causes a reboot: The crash could be caused by a data corruption.
Did you define configASSERT and also enable stack overflow checking for development/debugging to catch possible fatal errors or data corruptions ?
Are the stack sizes of your tasks large enough ?

overflow checking doesn’t display error

It tried this : Readers/Writer Lock - FreeRTOS
… I have no more crashes but web server generate many HTTP timeout for requests.

I have less timeout problems with this method but I still have more than if I don’t activate mutex protection :

static SemaphoreHandle_t mutex;
static int readersCounter;

static void openReader(void){
    xSemaphoreTake(mutex, portMAX_DELAY); // portMAX_DELAY = 0xffffffffUL 
        readersCounter++;
    xSemaphoreGive(mutex);
}

static void closeReader(void){
    xSemaphoreTake(mutex, portMAX_DELAY); // portMAX_DELAY = 0xffffffffUL   
        readersCounter--;
    xSemaphoreGive(mutex);
}

static void openWriter(void){
    while(1){
        xSemaphoreTake(mutex, portMAX_DELAY); // portMAX_DELAY = 0xffffffffUL 
        if(readersCounter == 0){
            break;
        }
        xSemaphoreGive(mutex);
    }
}

static void closeWriter(void){
    xSemaphoreGive(mutex);
}


void freeRTOS_init(void){
    mutex = xSemaphoreCreateMutex();
    if(readerMutex == NULL){
        while(1);
    }
}

I think the cause is that the RAM of the embedded web server is too small… but I don’t understand why the last method works better.

One issue you will have is if a higher priority task tries to openWriter(), it will keep on taking the mutex, see that there are readers, give it, then immediately take it again, and not allow the reader to run.

You want a second semaphore (not a mutex) that is taken when ever you bump the readersCounter from 0 to 1, and then given when you bump it down from 1 to 0 that the writer can block on to wait till it has a chance of successfully taking a write lock.

I don’t undertand all : you suggest me to use the code of my first
message in changing ?

writerMutex = xSemaphoreCreateMutex();

by

writerMutex = xSemaphoreCreateBinary();

well, the code with the semaphore looks ok now, but the timeouts may be explainable by Richards observations. A lot of read requests will starve the writers… you may need to add additional mechanisms that will allow no more readers when a write request is pending, but since http requests are always read-write, you do not gain a whole lot…

With this modifications, I did somes tests and it works fine.
I thank you :slight_smile:

Not only will read requests starve the writer, but a higher trying to get the lock can starve the readers that have the lock from getting any CPU time because it is just polling for the lock to be ready.

A “Good” Read/Write lock takes a bit of thought to do well, and you need to define how you want to manage cases which could cause starvation, and which starvations are important to you.

I found a FreeRTOS add-on to manage “one-writer, several readers” : freertos-addons/read_write_lock.c at master · michaelbecker/freertos-addons · GitHub

It confirms that my method is good : it correspond to “prefer reader” mode… but I will change to “prefer writer” mode, it is better for my application.