Update task notification scheduler suspension usage

Dazza0 commented 8 months ago

Description

https://github.com/FreeRTOS/FreeRTOS-Kernel/issues/612 fixed an issue with ulTaskGenericNotifyTake() and xTaskGenericNotifyWait() being non deterministic by ensuring that those functions call prvAddCurrentTaskToDelayedList() while the scheduler was suspended. The merged fix would suspend the scheduler while still in a critical section, then exit the critical section while the scheduler was suspended:

taskENTER_CRITICAL();
vTaskSuspendAll();
taskEXIT_CRITICAL();
prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE );
xTaskResumeAll();

The method above technically works but is a bit unintuitive due to interleaving critical sections and scheduler suspension.

This PR updates the behavior described above by wrapping the critical section in a scheduler suspension block:

vTaskSuspendAll();
taskENTER_CRITICAL();
// Check if block required
taskEXIT_CRITICAL();
if (blocking) {
    prvAddCurrentTaskToDelayedList( xTicksToWait, pdTRUE );
}
xTaskResumeAll();

This method is more intuitive. As an added bonus, it also allows the SMP implementation of vTaskSuspendAll() to be simplified (as we can assume the vTaskSuspendAll() is now never called in a critical section). As such, an extra assert has been added to vTaskSuspendAll() to ensure that it is never called in a critical section.

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

Related Issue

See https://github.com/FreeRTOS/FreeRTOS/pull/1177 for CMock test updates
Related to https://github.com/FreeRTOS/FreeRTOS-Kernel/issues/612

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

Dazza0 commented 8 months ago

@chinglee-iot I've updated the CMock unit tests as well. PTAL.

sonarcloud[bot] commented 8 months ago

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

2 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

codecov[bot] commented 8 months ago

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (57a5ed7) 93.55% compared to head (63c8d73) 93.52%.

Files	Patch %	Lines
tasks.c	89.65%	0 Missing and 3 partials :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #982 +/- ## ========================================== - Coverage 93.55% 93.52% -0.03% ========================================== Files 6 6 Lines 3197 3199 +2 Branches 887 889 +2 ========================================== + Hits 2991 2992 +1 Misses 92 92 - Partials 114 115 +1 ``` | [Flag](https://app.codecov.io/gh/FreeRTOS/FreeRTOS-Kernel/pull/982/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/FreeRTOS/FreeRTOS-Kernel/pull/982/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS) | `93.52% <89.65%> (-0.03%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=FreeRTOS#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

FreeRTOS / FreeRTOS-Kernel