POSIX Simulator: Don't yield non-FreeRTOS threads

[johnboiles]

POSIX Simulator: Don't yield non-FreeRTOS threads

Description

This is a followup to #1223. I think it's correct to not call vTaskSwitchContext from non-FreeRTOS threads but I'm low confidence on this one. Feedback is welcome.

I just noticed a hang in my test suite and it looks like my test suite thread got suspended by FreeRTOS:

Thread 1 (Thread 0x7fea4bf8abc0 (LWP 6178)):
#0  0x00007fea4b298d61 in __futex_abstimed_wait_common64 (private=3, cancel=true, abstime=0x0, op=393, expected=0, futex_word=0x7fea4401479c) at ./nptl/futex-internal.c:57
#1  __futex_abstimed_wait_common (cancel=true, private=3, abstime=0x0, clockid=0, expected=0, futex_word=0x7fea4401479c) at ./nptl/futex-internal.c:87
#2  __GI___futex_abstimed_wait_cancelable64 (futex_word=futex_word@entry=0x7fea4401479c, expected=expected@entry=0, clockid=clockid@entry=0, abstime=abstime@entry=0x0, private=private@entry=0) at ./nptl/futex-internal.c:139
#3  0x00007fea4b29b7dd in __pthread_cond_wait_common (abstime=0x0, clockid=0, mutex=0x7fea44014740, cond=0x7fea44014770) at ./nptl/pthread_cond_wait.c:503
#4  ___pthread_cond_wait (cond=0x7fea44014770, mutex=0x7fea44014740) at ./nptl/pthread_cond_wait.c:627
#5  0x00005581c12ceca3 in event_wait (ev=0x7fea44014740) at /home/runner/work/embedded-app/embedded-app/lib/Simulator/stubs/freertos-posix/src/Source/portable/ThirdParty/GCC/Posix/utils/wait_for_event.c:84
#6  0x00005581c12cea26 in prvSuspendSelf (thread=0x7fea4bf45fe8) at /home/runner/work/embedded-app/embedded-app/lib/Simulator/stubs/freertos-posix/src/Source/portable/ThirdParty/GCC/Posix/port.c:614
#7  0x00005581c12ce9f8 in prvSwitchThread (pxThreadToResume=0x5581ce8c5b08, pxThreadToSuspend=0x7fea4bf45fe8) at /home/runner/work/embedded-app/embedded-app/lib/Simulator/stubs/freertos-posix/src/Source/portable/ThirdParty/GCC/Posix/port.c:592
#8  0x00005581c12ce649 in prvPortYieldFromISR () at /home/runner/work/embedded-app/embedded-app/lib/Simulator/stubs/freertos-posix/src/Source/portable/ThirdParty/GCC/Posix/port.c:387
#9  0x00005581c12ce65e in vPortYield () at /home/runner/work/embedded-app/embedded-app/lib/Simulator/stubs/freertos-posix/src/Source/portable/ThirdParty/GCC/Posix/port.c:395
#10 0x00005581c12d1537 in prvAddNewTaskToReadyList (pxNewTCB=0x5581ce8b2530) at /home/runner/work/embedded-app/embedded-app/lib/Simulator/stubs/freertos-posix/src/Source/tasks.c:1098
#11 0x00005581c12d11ac in xTaskCreate (pxTaskCode=0x5581c0f800f1 <_FUN(void*)>, pcName=0x7ffd493ff470 "D:TestTelem", usStackDepth=2048, pvParameters=0x5581ce8c0d40, uxPriority=1, pxCreatedTask=0x0) at /home/runner/work/embedded-app/embedded-app/lib/Simulator/stubs/freertos-posix/src/Source/tasks.c:804
#12 0x00005581c12d684d in xTaskCreatePinnedToCore (pvTaskCode=0x5581c0f800f1 <_FUN(void*)>, pcName=0x7ffd493ff470 "D:TestTelem", usStackDepth=2048, pvParameters=0x5581ce8c0d40, uxPriority=1, pvCreatedTask=0x0, xCoreID=1) at /home/runner/work/embedded-app/embedded-app/lib/Simulator/stubs/freertos-posix/src/Esp32Shim.c:7
#13 0x00005581c0f801fb in DispatchQueue::dispatchOneOff(std::function<void ()> const&, unsigned long, unsigned long, long, char const*) (op=..., threadStack=2048, priority=1, core=1, name=0x5581c12e9e56 "TestTelem") at /home/runner/work/embedded-app/embedded-app/src/Misc/Dispatch.cpp:174
#14 0x00005581c0ef6863 in CATCH2_INTERNAL_TEST_0 () at /home/runner/work/embedded-app/embedded-app/test/CatchTests/TestTelemetry.cpp:36
#15 0x00005581c117595a in Catch::(anonymous namespace)::TestInvokerAsFunction::invoke (this=0x5581ce895840) at /home/runner/work/embedded-app/embedded-app/lib/Catch2/catch_amalgamated.cpp:7133
#16 0x00005581c11a9157 in Catch::TestCaseHandle::invoke (this=0x5581ce8c0f30) at /home/runner/work/embedded-app/embedded-app/lib/Catch2/catch_amalgamated.hpp:7163
#17 0x00005581c1170b9c in Catch::RunContext::invokeActiveTestCase (this=0x7ffd49400680) at /home/runner/work/embedded-app/embedded-app/lib/Catch2/catch_amalgamated.cpp:6154
#18 0x00005581c117089d in Catch::RunContext::runCurrentTest (this=0x7ffd49400680) at /home/runner/work/embedded-app/embedded-app/lib/Catch2/catch_amalgamated.cpp:6117
#19 0x00005581c116e8e6 in Catch::RunContext::runTest (this=0x7ffd49400680, testCase=...) at /home/runner/work/embedded-app/embedded-app/lib/Catch2/catch_amalgamated.cpp:5814
#20 0x00005581c1[158](https://github.com/sindarin-inc/embedded-app/actions/runs/13169223544/job/36756300303#step:11:159)953 in Catch::(anonymous namespace)::TestGroup::execute (this=0x7ffd49400670) at /home/runner/work/embedded-app/embedded-app/lib/Catch2/catch_amalgamated.cpp:1253
#21 0x00005581c1[159](https://github.com/sindarin-inc/embedded-app/actions/runs/13169223544/job/36756300303#step:11:160)e9c in Catch::Session::runInternal (this=0x7ffd49400950) at /home/runner/work/embedded-app/embedded-app/lib/Catch2/catch_amalgamated.cpp:1473
#22 0x00005581c11599a6 in Catch::Session::run (this=0x7ffd49400950) at /home/runner/work/embedded-app/embedded-app/lib/Catch2/catch_amalgamated.cpp:1405
#23 0x00005581c11ba6df in Catch::Session::run<char> (this=0x7ffd49400950, argc=6, argv=0x7ffd49400c18) at /home/runner/work/embedded-app/embedded-app/lib/Catch2/catch_amalgamated.hpp:4937
#24 0x00005581c116aef5 in main (argc=6, argv=0x7ffd49400c18) at /home/runner/work/embedded-app/embedded-app/lib/Catch2/catch_amalgamated.cpp:4784

xTaskCreate calls vPortYield which may suspend the current thread. It's useful to be able to call xTaskCreate from outside of a FreeRTOS thread (and has to happen at least once before the scheduler is started). In my test I called xTaskCreate after the scheduler was started, which lead to this hang.

The alternative is to never allow xTaskCreate to be called from outside of a FreeRTOS thread after the scheduler has been started. If this is the direction, then we should probably enforce or document that somehow.

Test Steps

Call xTaskCreate outside of a FreeRTOS thread. Sometimes it locks up. I'm unsure why it's only sometimes.

Checklist:

• I have tested my changes. No regression in existing tests.
• I have modified and/or added unit-tests to cover the code changes in this Pull Request.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[aggarg]

Would you please help me understand how this code can create a problem because xTaskGetCurrentTaskHandle will always return FreeRTOS task only.

In any case, you should not call FreeRTOS APIs from non-FreeRTOS threads. So your following suggestions seems reasonable to me:

The alternative is to never allow xTaskCreate to be called from outside of a FreeRTOS thread after the scheduler has been started. 

[johnboiles]

Would you please help me understand how this code can create a problem because xTaskGetCurrentTaskHandle will always return FreeRTOS task only.

Are you sure this is the case? Looks like it just returns pxCurrentTCB so it will return whatever task the scheduler thinks is running at the moment (even when this code is not running from a FreeRTOS task).

You should not call FreeRTOS APIs from non-FreeRTOS threads.

I think that's a fine constraint. Just to make sure I understand, are the rules:

• Only *FromISR methods can be called from outside of FreeRTOS tasks
• Except vTaskStartScheduler(); and vTaskEndScheduler();
• Except xTaskCreate but it can only be called before vTaskStartScheduler();

I wonder how can we crash/assert loudly if these rules are broken. Maybe we have an assert in vPortYield if it's ever called from a non-FreeRTOS thread? Looks like that's the first part of the above trace that gets into port.c anyways.

[aggarg]

Are you sure this is the case? Looks like it just returns pxCurrentTCB so it will return whatever task the scheduler thinks is running at the moment (even when this code is not running from a FreeRTOS task).

pxCurrentTCB is a FreeRTOS variable and it is set by the FreeRTOS to the currently running FreeRTOS task. If you create a thread using pthread_create, FreeRTOS is not aware about it and therefore, cannot set pxCurrentTCB to your thread.

Except xTaskCreate but it can only be called before vTaskStartScheduler();

It is okay and a good general practice to create FreeRTOS objects (task, mutex, queue, event group etc.) before starting the scheduler.

[aggarg]

I am closing this PR. Feel free to reopen if you need anything else.

[johnboiles]

@aggarg What do you think about the idea of putting an assert in vPortYield so that it is obvious if this is ever called from a non-FreeRTOS thread? I think that would be better than locking up the program. I'm happy to make a PR for that if you're cool with it.

[aggarg]

@johnboiles I assume you are talking about something like configASSERT( prvIsFreeRTOSThread() == pdTRUE );. If yes, that sounds good to me. Please raise a PR.

[johnboiles]

Done in #1247