search

FreeRTOS / FreeRTOS-Kernel

FreeRTOS kernel files only, submoduled into https://github.com/FreeRTOS/FreeRTOS and various other repos.
https://www.FreeRTOS.org
MIT License
2.62k stars 1.09k forks source link

[BUG] Atomic_*s are not atomic! #936

Closed sebunger closed 7 months ago

sebunger commented 8 months ago

In atomic.h, ATOMIC_ENTER_CRITICAL and ATOMIC_EXIT_CRITICAL are defined differently depending on whether portSET_INTERRUPT_MASK_FROM_ISR is defined! However, in FreeRTOS.h this macro is stubbed out if it is not already defined by the port.

This means that on all ports that do not, in fact, define their own (and functional) portSET_INTERRUPT_MASK_FROM_ISR, the atomics will end up silently not being atomic resulting in who-knows-what sort of bugs.

This is particularly egregious since the template port does not even mention that macro in its template portmacro.h!

There’s a possibly related but not quite identical post that dates back 7 years. So this may have been there a while? Unfortunately I can’t seem to link to the post since I just signed up, but the topic number is 6295.

There's a bit more discussion along with possible avenues for fixes in the forum: https://forums.freertos.org/t/atomic-s-are-not-atomic/18809

jefftenney commented 8 months ago

I did a quick test with MSP430. That port does not define portSET_INTERRUPT_MASK_FROM_ISR(). I added some code to call Atomic_Increment_u32() and then checked the compiled code (asm file produced by compiler). The compiled code does not mask interrupts and does not provide atomic behavior. So this is a good catch.

From the forum post it seems adding a new symbol portHAS_NESTED_INTERRUPTS would allow a simple fix to atomic.h.

chinglee-iot commented 7 months ago

@sebunger Thank you for reporting this issue. This issue is addressed in PR #940. Feel free to reopen this issue for further problem about this issue.