FWS-1372 Fix os_eventq_put() to wake up task only if EVQ_WAIT flag is set

[vrahane]

• This was causing a task waiting on a semaphore to wakeup on scheduling an event

[wes3]

Well this is quite the good catch I must say. Unless I am mistaken this can cause a number of issues that would be hard to find/debug. Again, unless I am mistaken, if code uses os_time_delay(), a task can get woken up before the delay expires if an event is posted. I cannot see anything wrong with this fix.

[kasjer]

While this does not break anything I don't think it solves the problem.
evq->evq_task is only set when task calls os_eventq_get() this is blocking call so it's strange that same task can later call os_sem_pend().
For me it more likely looks like memory corruption of task struct.

[vrahane]

I think I did not do a good job at explaining this. So, what is happening is that we have the default task waiting on a semaphore. The task actually wakes up when os_eventq_put() is called which should not happen, it should only wakeup when os_sem_release() is called on the pending semaphore. So, what this change does is it forces the scheduler to be blocked on the semaphore.
The above code did the right thing back in 2015-2016 before we added os_eventq_poll(). You will have to go back into incubator-mynewt-core to look at the exact code. The functionality broke when OS_TASK_SLEEP flag was added I believe.

[andrzej-kaczmarek]

So, what this change does is it forces the scheduler to be blocked on the semaphore even though an event is scheduled if thats what the task has been waiting for.

This is exactly what's impossible here: task cannot be blocked on semaphore and wait for an event at the same time.

The condition you modify is only executed if evq->evq_task != NULL and this is only valid if task is blocked on os_eventq_get. If you have task blocked on a semaphore which is also set as evq_task in some eventq then something is broken elsewhere.

[wes3]

I read the code in more detail (which I should have done before approving admittedly) and yes you are correct. Checking evq_task first will guarantee the above situation does not occur. This is good :-) Thanks guys

[vrahane]

Thank you, makes complete sense. We got a bit carried away by the semaphore behavior which contributed to the issue.

[vrahane]

Closing this, I tried reproducing it with semaphore tokens initialized to 0 before pending which was causing our issue.