Giving Semaphore Limit+1 can cause limit+1 takes

[nslowell]

Describe the bug
It appears that if a single thread's loop includes waiting on k_sem_take() of a semaphore with limit X, if another routine (like an IRQ function) calls k_sem_give() X+1 times, the thread will be able to run X+1 times even though it should only run X times.

The true example: I have a single looping thread calling k_sem_take() on a semaphore with a max of 1. When the count is 0, this thread is put to sleep. An IRQ runs and ends up calling k_sem_give() once, which does not actually increment the semaphore count but finds the sleeping thread and makes it "ready to run". However, before the thread is switched in, the IRQ calls k_sem_give() a second time. Because the thread isn't found sleeping, this time the semaphore count is incremented. The looping thread is finally swapped in and executes the first time due to it's being made "ready". When it reaches the k_sem_take() call, because the count is still 1, it is able to loop an additional time.

To Reproduce
Steps to reproduce the behavior:

1. Create a semaphore with max count of 1
2. Create a thread that loops by calling k_sem_take() on the semaphore, waiting K_FOREVER
3. Create an IRQ that calls k_sem_give() twice when executing
4. Cause the IRQ to fire
5. Observe the thread loops twice

Expected behavior
I would expect the semaphore limit to take affect when k_sem_give() is called twice in an IRQ before the "taking" thread can execute.

Impact
This was causing the thread to loop an extra time, causing issues like processing on empty data or old data b/c no new data had been received from a new interrupt.

[aescolar]

CC @andyross @andrewboie

[andyross]

This is a priority inversion bug in your app, I think. The behavior per specification sounds correct to me: in the high priority process (which happens to be an ISR, but could be anything) the first call to k_sem_give() unpends the waiting thread and makes it runnable, leaving the semaphore count at zero. The second call sees a semaphore with a count of zero and increments it, so the low priority context returns synchronously the second time it calls k_sem_take(). Two calls to give, two calls to take, the count never goes above one. That's all correct as I see it.

What you are expecting is that no other (higher priority) process can run between the point where k_sem_give() wakes you up and where k_sem_take() returns. But nothing in the system guarantees that.