[BUG] Incorrect argument to portRESET_READY_PRIORITY macro in vTaskPriorityDisinheritAfterTimeout()

[wirelinker]

Describe the bug
I found in vTaskPriorityDisinheritAfterTimeout(), tasks.c Line 6909,
after a uxListRemove(), there is a portRESET_READY_PRIORITY macro.

The argument be passed to the macro is pxTCB->uxPriority, which
should be uxPriorityUsedOnEntry.

Because the pxTCB was removed from pxReadyTasksLists[ uxPriorityUsedOnEntry ].
Then the portRESET_READY_PRIORITY() will be called if the pxReadyTasksLists[uxPriorityUsedOnEntry] became empty after the remove.
And pxTCB->uxPriority had already been changed to another higher/lower task priority.
That is : uxPriorityUsedOnEntry != pxTCB->uxPriority.

Thank you.

Target
Ports with optimized task selection implementation.

Host

• Host OS: Ubuntu
• Version: 24.04

To Reproduce
I have no idea how to reproduce. It only effects customized scheduler ports. I am using pi-pico
as target which is not the case.

Expected behavior
Should reset ready priority with the priority of the actually empty ready list.

Additional context

[kstribrnAmzn]

Wow...this is a great find! I'll raise a PR shortly to update this. I have a way to test this and I can post some results on Monday as I left the test board in the office.

[kstribrnAmzn]

PR - #1338

[wirelinker]

I was wondering about if I misunderstood the code.
Because this bug didn't cause any abnormal behavior on pico.
I don't even know how to reproduce the bug.
So I checked all the appearances of portRESET_READY_PRIORITY() in the file, then do the post.
I am glad to see this is helpful.

[kstribrnAmzn]

Sorry to reverse my previous stance here but there was an error in my thinking. I was thinking about this priority disinheritance bug from the perspective of a single task in isolation. When doing this it will appear bugged, but once I thought about it in the context of multiple tasks then the code is correct. Let me see if I can explain and diagram this below.

For a task to inherit priority there must be a second, higher priority task which desires the mutex. When attempting to acquire this mutex the higher priority task has the option to pass a maximum wait time to acquire the mutex. When this maximum wait time expires the task will become ready again. This means the higher priority task goes from Running (immediately before calling to acquire the mutex) to Blocked (while waiting to acquire the mutex) to Ready (after timing out). This means that when timeout causes a priority disinheritance that there are two tasks at the priority - the task holding the mutex (inherited priority aka uxPriorityUsedOnEntry) and the higher priority task desiring the mutex. When the timeout occurs the mutex holder will disinherit priority, however the mutex desiring task will transition from Blocked to Ready. This is to say we don't want to clear the ready flag for the uxPriorityUsedOnEntry priority because the mutex desiring task is Ready. Clearing the entry priority would mark the newly unblocked mutex desiring task as not ready to run which is incorrect.

This is why this has been so difficult to reproduce. The code is correct.

[kstribrnAmzn]

A timeline of the situation is below

Task A (mutex holder, base priority 2, priority 2)
Task B (mutex desireer, base priority 5, priority 5)

Timeline: 
-t1--t2--t3-->

At t1: Task A acquires the mutex as it is Running. It is presumed that Task B is blocked. All priorities remain the same.
At t2: Task B awakens, preempts Task A and attempts to acquire the mutex. It cannot since it is already held. It transitions to blocked and Task A inherits the priority of Task B (Task A priority = 5, Task B priority = 5). The ready flags will signal that only Task A and the Idle task can run.
At t3: Task B completes it wait for the mutex and times out. Task B transitions to Ready. Task A disinherits priority (priority now equal to 2). With both tasks ready, the scheduler will choose the highest priority task, Task B, to run. The ready flags will signal that Task A, B, and the idle task can run.

[kstribrnAmzn]

I'll leave this issue open for a couple of days so you can post any questions you have. After the couple of days I will close this issue.