The first comment in vTaskPriorityDisinherit() says: “We must be the running task to be able to give the mutex back.”. But this is not true and this will lead to the FreeRTOS crash.
But let’s start from the beginning. I use FreeRTOS v8.0.0. There is a task A with priority 1, a task B with priority 2 and the idle task with priority 0 (there are more tasks but they don’t matter). There is also a mutex. The mutex is given from a ISR with xSemaphoreGiveFromISR(); portYIELD_FROM_ISR().
Task A holds the mutex and task B waits for the mutex (the priority from task A is inherit to 2). Now Task A takes the mutex a second time -> the idle task runs (The list xTaskWaitingToReceive from the mutex contains task A and B and the mutexHolder is task A. The list xDelayedTaskList1 contains task A and B). After a short time the ISR runs and the mutex is given with xSemaphoreGiveFromISR(). In xQueueGenericSendFromISR() the function prvCopyDataToQueue() is called which calls vTaskPriorityDisinherit(). This function assumes that task A is on the list pxReadyTasksLists so it calls uxListRemove( &( pxTCB->xGenericListItem ) ). But this removes the task A from the list xDelayedTaskList1. That’s the reason why FreeRTOS will crash in vListInsert().
The next what will happen is:
Task B gets the mutex (the priorty from task A is disinherit to 1).
Task B takes the mutex a second time (so far the only error is that task A is on the readyList).
Now task A runs (what is wrong because the timeout is not over and the mutex is hold by task B).
Task A is still in the function xQueueGenericReceive(). pxQueue->uxMessagesWaiting is 0 so vTaskPlaceOnEventList( &( pxQueue->xTasksWaitingToReceive ), xTicksToWait ) is called. But task A is already on the list xTasksWaitingToReceive (mutex). So it will be added a second time. Now the list is corrupted (see the following table).
| Expression | Value | Location |
|---|---|---|
| - xTasksWaitingToReceive (Mutex) | <struct> | Memory: 0x514A |
| uxNumberOfItems | 3 | Memory: 0x514A |
| pxIndex | 0x514E | Memory: 0x514C |
| - xListEnd | <struct> | Memory: 0x514E |
| xItemValue | 4294967295 | Memory: 0x514E |
| pxNext | 0x3C52 | Memory: 0x5152 |
| pxPrevious | 0x3F4C | Memory: 0x5154 |
| - xEventListItem (Task A) | <struct> | Memory: 0x3F4C |
| xItemValue | 9 | Memory: 0x3F4C |
| pxNext | 0x3F4C | Memory: 0x3F50 |
| pxPrevious | 0x3F4C | Memory: 0x3F52 |
| pvOwner | 0x3F3E | Memory: 0x3F54 |
| pvContainer | 0x514A | Memory: 0x3F56 |
| - xEventListItem (Task B) | <struct> | Memory: 0x3C52 |
| xItemValue | 8 | Memory: 0x3C52 |
| pxNext | 0x3F4C | Memory: 0x3C56 |
| pxPrevious | 0x514E | Memory: 0x3C58 |
| pvOwner | 0x3C44 | Memory: 0x3C5A |
| pvContainer | 0x514A | Memory: 0x3C5C |
The next what will happen is:
After a short time the ISR runs and the mutex is given with xSemaphoreGiveFromISR().
Task B runs an give the Mutex with xSemaphoreGive().
Task A get the mutex an run.
Task A takes the mutex -> crash in vListInsert()
You do not give a Mutex with xSemaphoreGiveFromISR(), only semaphores.
In fact, ISRs can not use Mutexes as mutexes use info about the current task, and it is true that the task that takes the mutex must be the task that then gives the mutex back, or data structures might get corrupted.
A task should not attempt to acquire a mutex it already has, unless the mutex is defined as a recursive mutex.
IF you don't follow the requirements for a function, you should't have expectations for its results.
You are right the API Reference says: "Mutex type semaphores (those created using a call to xSemaphoreCreateMutex()) must not be used with this macro."
But the FreeRTOS+IO library do it the same way. From a macro which is called from a ISR:
…
xSemaphoreGiveFromISR( pxZeroCopyState->xWriteAccessMutex, &( xHigherPriorityTaskWoken ) );
…
The FreeRTOS+IO library also take a mutex a second time.
Give a mutex with xSemaphoreGiveFromISR() don’t need informations about the current task. Which task holds the mutex is saved in pxMutexHolder. The priority disinherit is done with this information. If I don’t miss anything then to get this run the only what has to be done is adding
if( listIS_CONTAINED_WITHIN( &( pxReadyTasksLists[ pxTCB->uxPriority ] ), &( pxTCB->xGenericListItem ) ) != pdFALSE )
I will test this a little.
I have attached a file. The file contains the modified function vTaskPriorityDisinherit(). I think with this modification it should be possible to take a mutex with xSemaphoreGiveFromISR().
When I wrote the code I noticed something else.
The function xTaskRemoveFromEventList() removes the task from the eventList and add the task to the readyList if the scheduler is stopped. The function xTaskResumeAll() also removes the task from the eventList. Is this an error or did I miss something?
Normally the eventListItem still contain the old (right) pxNext and pxPrevious. So if this an error the pointer from the list are not all right. But the variable uxNumberOfItems is wrong.
As far as this being a FreeRTOS bug, I have to concur with Richard Damon, at this point (without further investigation) I don't think it is a bug, it is just that you are trying to do something that is not supported and would probably not be logical, and the documentation says not to do.
That said, it does appear that FreeRTOS+IO is doing this, so there could be a bug in FreeRTOS+IO (which is separate to FreeRTOS), so I will leave this bug report open until I have investigated that. As I recall at the time I think FreeRTOS+IO's particular usage case was deemed to be correct. I need to reaffirm that though.
As for taking a mutex twice - I'm not sure that is possible. The function may get called twice, but if the mutex is already held the second call should fail as the mutex is not there to be taken (assuming the mutex is not a recursive mutex).
I will take an action to make the documentation more explicit too.
Regards.
I has read the FreeRTOS+IO library and copied this part without reading the API References for the macro xSemaphoreGiveFromISR. The references say don’t do this so you can’t call this a FreeRTOS bug. I agree with this.
But by adding one line to vTaskPriorityDisinherit() you can use a implementation like the one from the FreeRTOS+IO library. In this implementation the FreeRTOS_write (Interrupt driven zero copy) take the mutex and the ISR (write finished) returns the mutex. By taking the mutex the second time with FreeRTOS_ioctl(portHandle, ioctlWAIT_PREVIOUS_WRITE_COMPLETE, timeout) the task can wait until the write is done.
So I would call this a feature request.
When I edited the code for vTaskPriorityDisinherit() I analyzed it and I can’t find a reason (except the fixed issue) why you should not do this. After the fix this implementation works well.
By the way, since you want to change the documentation, the macro xSemaphoreTakeFromISR() missing the information that you can’t use it with a mutex.
Regards.
I recently encountered the same problem using FreeRTOS+IO V1.0.1 with FreeRTOS V7.5.2. Looking in the archives (http://www.freertos.org/FreeRTOS_Support_Forum_Archive/September_2012/freertos_FreeRTOS_Plus_IO_LPC1769_UART_IRQ_Handler_5681883.html), this problem has been known for a couple years, but never fixed.
Attached is my patch to change FreeRTOS+IO to use a binary semaphore.
Last edit: Scott Bertin 2014-04-24
I'm not sure of the conclusion here - but in any case some mutex behaviour was changed in V8.1.2 so I don't beleive the thread is relevant any more.