Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Threading, Timers improvements #3865

Merged
merged 2 commits into from
Sep 7, 2024

Tighten the use of FuriThread* vs FuriThreadId

bd05b53
Select commit
Loading
Failed to load commit list.
Merged

Threading, Timers improvements #3865

Tighten the use of FuriThread* vs FuriThreadId
bd05b53
Select commit
Loading
Failed to load commit list.
Task list completed / task-list-completed succeeded Sep 7, 2024 in 0s

3 / 3 tasks completed

All tasks have been completed

Details

Required Tasks

Task Status
FuriTimer::can_be_removed has been replaced with a local variable to reduce the size of the structure. Passing a pointer to a local variable to a pending callback is safe in furi_timer_free, because we are waiting for that pending call to finish before returning from the caller. This change reduces the memory usage of FuriTimer while keeping the blocking behaviour of furi_timer_free. Incomplete
Since currently FuriThread*, FuriThreadId and TaskHandle_t are aliases, the use of TLS to store a pointer to FuriThread is not required anymore. I replaced all the instances of this and changed FreeRTOS config not to allocate that TLS slot. DROPPED AS IT'S UNSAFE Incomplete
MOVED TO #3881 To simplify join logic, a new FuriThreadStateStopping has been introduced, while StateStopped is now raised from the TCB cleanup. This means furi_thread_join can just wait for the state to change to Stopped. Incomplete
MOVED TO #3881 The above change comes with a bonus - thread state callbacks may now assume that upon receiving a Stopped call, it is safe to release FuriThread. Multiple call sites in the firmware have been updated to make use of this new assumption, instead of deferring their release to a pending timer call. Incomplete
MOVED TO #3881 State change callback received a new FuriThread* thread parameter - previously, callbacks assumed that they are always invoked from the thread that is changing its state, but this wasn't true for StateStarting, and with this PR, it's not true for StateStopped either. Incomplete
Several call sites mixed up FuriThread* and FuriThreadId. While this is fine in the internal thread routines, IMO shouldn't be done by other modules. Incomplete
Run a full suite of tests. Incomplete
Verify that apps start and stop. Incomplete
Verify that RPC via qFlipper still works. Incomplete
PR has description of feature/bug or link to Confluence/Jira task Completed
Description contains actions to verify feature/bugfix Completed
I've built this code, uploaded it to the device and verified feature/bugfix Completed
FuriThreadStateStopped behavior change(need more testing) Incomplete
everything else (ready for merge) Incomplete