flipperdevices / flipperzero-firmware

Flipper Zero firmware source code
https://flipperzero.one
GNU General Public License v3.0
12.84k stars 2.73k forks source link

Idle priority threads that only yield can lock up serial USB if qFlipper is in use #3908

Closed CookiePLMonster closed 1 month ago

CookiePLMonster commented 1 month ago

Describe the bug.

This bug report only mentions the Direct Draw debug app, since it's the easiest to reproduce this issue on that app - however, appears to happen in any other app that is running at an idle priority and only uses yielding to relinquish CPU time.

Attempting to run qFlipper when a Direct Draw debug app runs, then terminating it, and attempting to either:

returns in these actions locking up and failing. ufbt cli gets stuck at

scons: Entering directory `H:\dev\flipper-zero\.ufbt\current\scripts\ufbt'
python H:\dev\flipper-zero\.ufbt\current\scripts/serial_cli.py -p auto
--- Miniterm on COM6  230400,8,N,1 ---
--- Quit: Ctrl+] | Menu: Ctrl+T | Help: Ctrl+T followed by Ctrl+H ---

and never shows the Flipper logo prompt, deployments halt, qFlipper cannot detect the Flipper.

Terminating the app "unstucks" the connections. Also this does not happen with all serial connections, CLI can be started as many times as I wish - the state only locks up after running qFlipper.

As far as I can tell, this is not related to direct drawing, rather to the thread priorities again - as replacing furi_thread_yield() with furi_delay_tick(2) resolves this issue. However, since the app thread is running at an Idle priority, it should make space for any other task that needs CPU time, I think?

Reproduction

  1. Connect the Flipper to the PC.
  2. Run a Direct Draw debug app.
  3. Run ufbt cli - notice the command goes through and it shows a Flipper prompt.
  4. Terminate the CLI connection.
  5. Run qFlipper, and exit it as soon as it shows the mirrored screen.
  6. Attempt to run ufbt cli again - notice it gets stuck at --- Quit: Ctrl+] | Menu: Ctrl+T | Help: Ctrl+T followed by Ctrl+H ---.
  7. Terminate the Direct Draw debug app. Notice the CLI session gets unstuck.

Target

f7 running on stable 1.0.1 firmware

Logs

No response

Anything else?

No response

CookiePLMonster commented 1 month ago

Upon closer inspection, this appears to be caused by the FreeRTOS idle task being starved by this idle task: https://freertos.org/Documentation/02-Kernel/02-Kernel-features/01-Tasks-and-co-routines/03-Task-priorities

Each task is assigned a priority from 0 to ( configMAX_PRIORITIES - 1 ), where configMAX_PRIORITIES is defined within FreeRTOSConfig.h. Low priority numbers denote low priority tasks. The idle task has priority zero (tskIDLE_PRIORITY).

Furi threads at Idle priority have priority 1, not 0, so yielding from them will never give time to the idle task. Hence, these values

typedef enum {
    FuriThreadPriorityNone = 0, /**< Uninitialized, choose system default */
    FuriThreadPriorityIdle = 1, /**< Idle priority */

may need fixing - perhaps PriorityNone should become -1, or those Furi priorities need to be bumped one notch up, and priority - 1 should be passed to FreeRTOS? In fact FuriThreadPriorityNone is unused currently, it's probably only a 'reasonable default'.

EDIT: furi_thread_start does UBaseType_t priority = thread->priority ? thread->priority : FuriThreadPriorityNormal;, so the notion of "None" priority should just go IMO and furi_thread_alloc should default the priority field to FuriThreadPriorityNormal instead.