Threading, Timers improvements

[CookiePLMonster]

What's new

This PR introduces several changes to timers and threads:

1. 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.
2. Numerous changes have been made to FuriThread.
   • 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
   • MOVED TO FuriThread: Improve state callbacks #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.
   • MOVED TO FuriThread: Improve state callbacks #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.
   • MOVED TO FuriThread: Improve state callbacks #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.
3. 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.

Thanks to the removal of a struct member in FuriTimer and a now-useless TLS slot, this change saves a bit of heap and pool space. I suspect the code should be a bit smaller too, but I haven't compared the sizes.

Before:

>: free
Free heap size: 146328
Total heap size: 186064
Minimum heap size: 137768
Maximum heap block: 146240
Pool free: 1368
Maximum pool block: 1340

>: free_blocks
A 2000B470 S 32
A 2000B4B8 S 146240

After:

>: free
Free heap size: 146392
Total heap size: 186064
Minimum heap size: 137872
Maximum heap block: 146344
Pool free: 1416
Maximum pool block: 1344

>: free_blocks
A 2000B410 S 24
A 2000B470 S 146312

Verification

• Run a full suite of tests.
• Verify that apps start and stop.
• Verify that RPC via qFlipper still works.

Checklist (For Reviewer)

• PR has description of feature/bug or link to Confluence/Jira task
• Description contains actions to verify feature/bugfix
• I've built this code, uploaded it to the device and verified feature/bugfix

[skotopes]

@CookiePLMonster there is a catch: under CPU starvation you will never receive FuriThreadStateStopped signal.
I'd propose to split this PR into 2 parts:

• FuriThreadStateStopped behavior change(need more testing)
• everything else (ready for merge)

[CookiePLMonster]

there is a catch: under CPU starvation you will never receive FuriThreadStateStopped signal.

Before this PR, in CPU starved scenarios FuriThread::is_active was never set to false, so furi_thread_join never returned regardless. However, with my PR this also means that things waiting for the Stopped thread state callback will not receive the call.

I'm not sure if this is a bad catch in itself, as the main use case (waiting inside furi_thread_join) is essentially unaffected by it, but if you prefer I can split the PR into two, sure. However, ideally I would still like to keep the change to FuriThreadStateCallback in this PR, since it is safe to keep even without the new Stopped behaviour.

EDIT:
On second thought maybe it's better to split, because both FuriThreadStateCallback and FuriThreadStateStopping state should have technically bumped the API version., and I didn't do that... While the remaining changes are purely internal and don't need a version bump.

[skotopes]

@CookiePLMonster in general what you did is a movement in the right direction. It just we were having deadlock problems in specific conditions in a code that looks exactly like yours right now(I'm the person who added this ugly hack for that problem), I need some time to add additional unit_tests to make sure that we are not stepping into same shit.

Current implementation is not perfect, but it forces your main thread to give some time to idle thread to release TCB. Which your implementation not doing

[CookiePLMonster]

Current implementation is not perfect, but it forces your main thread to give some time to idle thread to release TCB. Which your implementation not doing

Ah I see, before my changes, join calls in the pended state change calls gave some time, but my changes removed those entirely.

Could a yield before vTaskDelete help a bit? Probably not, as it'd be yielding too early.

[CookiePLMonster]

Split off to #3881.