Quality RTOS & Embedded Software

 Real time embedded FreeRTOS RSS feed 
Quick Start Supported MCUs PDF Books Trace Tools Ecosystem TCP & FAT




Loading

Should prvNotifyQueueSetContainer check if the queue is locked before editing event lists?

Posted by cman12345 on February 19, 2015

In queue.c, I noticed the following description about prvLock/UnlockQueue:

~~~~~ /* * Unlocks a queue locked by a call to prvLockQueue. Locking a queue does not * prevent an ISR from adding or removing items to the queue, but does prevent * an ISR from removing tasks from the queue event lists. If an ISR finds a * queue is locked it will instead increment the appropriate queue lock count * to indicate that a task may require unblocking. When the queue in unlocked * these lock counts are inspected, and the appropriate action taken. */ static void prvUnlockQueue( Queuet * const pxQueue ) PRIVILEGEDFUNCTION; ~~~~~

Does that mean it's dangerous to edit the queue event list while running from an ISR?

If so, I'm trying to understand why there is no check to see if the queue is locked in prvNotifyQueueSetContainer where the following path can be reached:

xQueueGenericSendFromISR() -> prvNotifyQueueSetContainer() -> xTaskRemoveFromEventList()

Thanks for any help!


Should prvNotifyQueueSetContainer check if the queue is locked before editing event lists?

Posted by rtel on February 19, 2015

I've been looking at this for the last half hour, and on first inspection you could be onto something here. From the scenarios I have drawn out it does look like a race condition could occur. Also I'm running the coverage tests as I type and we don't seem to hit that condition. It will take some time to investigate further - but if there is an issue it can be corrected with a simple update that checks the queue lock before calling prvNotifyQueueSetContainer(), and does nothing but increment the lock if it is found that the lock is set.

Regards.


Should prvNotifyQueueSetContainer check if the queue is locked before editing event lists?

Posted by rtel on February 19, 2015

It look to me like the code should be like the following (test markets and asserts removed for clarity). Do you agree:

EDIT - the formatting was unreadable so instead I have attached a small file that contains the update.

Regards.

Attachments


Should prvNotifyQueueSetContainer check if the queue is locked before editing event lists?

Posted by cman12345 on February 19, 2015

Yes, that aligns with what I would expect. Convention wise, I suppose a prvNotifyQueueSetContainerFromISR() routine that does this would line up with how the other code is organized.

Looking closer, I think this patch is definitely needed because otherwise you would have race conditions in xQueueGenericReceive if you are waiting on a queue set and xQueueGenericSendFromISR is called on a queue that is part of the queue set. In this case, the task list could get edited incorrectly.

Thanks for the response, much appreciated!


Should prvNotifyQueueSetContainer check if the queue is locked before editing event lists?

Posted by rtel on February 19, 2015

Yes - that is the conclusion I came to too. The code has been updated but is still running tests so not checked in yet. However the tests are not hitting that line - so highlighting an issue in the tests.

Regards.


[ Back to the top ]    [ About FreeRTOS ]    [ Sitemap ]    [ ]




Copyright (C) Amazon Web Services, Inc. or its affiliates. All rights reserved.

Latest News

FreeRTOS kernel V10 is available for immediate download. Now MIT licensed.


FreeRTOS Partners

ARM Connected RTOS partner for all ARM microcontroller cores

IAR Partner

Microchip Premier RTOS Partner

RTOS partner of NXP for all NXP ARM microcontrollers

STMicro RTOS partner supporting ARM7, ARM Cortex-M3, ARM Cortex-M4 and ARM Cortex-M0

Texas Instruments MCU Developer Network RTOS partner for ARM and MSP430 microcontrollers

OpenRTOS and SafeRTOS