sys/xtimer/xtimer.c: _mutex_timeout() bug fix

[JulianHolzwarth]

HAS "REMOVE ME" COMMIT TO SHOW BUG
This includes pr: #10872
Tracking: pr #11660

Contribution description

Fixes BUG.

The function _mutex_timeout removes a thread from the waiting list of a mutex and unblocks the thread (sets status to STATUS_PENDING) when the thread is waiting for the mutex. This is used for xtimer_mutex_lock_timeout a mutex lock function with timeout. This pr fixes a bug that happens because the function _mutex_timeout never checks if the mutex has a waiting list. This pr adds this check.

When the Timer triggers after the mutex is acquired but before the timer is removed.. You can see the bug with the "REMOVE ME" commit (asserts when the mutex is acquired and the timer triggers after).
The Function (_mutex_timeout) does nothing when the thread got the mutex (and no thread is waiting for it mutex->queue.next == MUTEX_LOCKED ) and when the mutex is free (mutex->queue.next == NULL).

list_remove should not be called with a mutex where mutex->queue.next == MUTEX_LOCKED.

Testing procedure

BOARD=native make -C tests/xtimer_mutex_lock_timeout/ flash test
output:

...
> mutex_timeout_long_unlocked
mutex_timeout_long_unlocked
starting test: xtimer mutex lock timeout
OK

> mutex_timeout_long_locked
mutex_timeout_long_locked
starting test: xtimer mutex lock timeout
OK

> mutex_timeout_long_locked_low
mutex_timeout_long_locked_low
starting test: xtimer mutex lock timeout with thread
threads = 2
THREAD low prio: start
MAIN THREAD: calling xtimer_mutex_lock_timeout
OK
threads = 3
MAIN THREAD: waiting for created thread to end
THREAD low prio: exiting low
threads = 2

Revert the Revert "REMOVE ME" commit to see it work after fix.
output:

...
> mutex_timeout_long_unlocked
mutex_timeout_long_unlocked
starting test: xtimer mutex lock timeout
OK

> mutex_timeout_long_locked
mutex_timeout_long_locked
starting test: xtimer mutex lock timeout
OK

> mutex_timeout_long_locked_low
mutex_timeout_long_locked_low
starting test: xtimer mutex lock timeout with thread
threads = 2
THREAD low prio: start
MAIN THREAD: calling xtimer_mutex_lock_timeout
OK
threads = 3
MAIN THREAD: waiting for created thread to end
THREAD low prio: exiting low
threads = 2

Revert to REMOVE ME commit and run the test to see the bug.
output:

...
> mutex_timeout_long_unlocked
mutex_timeout_long_unlocked
starting test: xtimer mutex lock timeout
sys/xtimer/xtimer.c:252 => 0x565b8323
*** RIOT kernel panic:
FAILED ASSERTION.

*** halted.

Issues/PRs references

Depends on PR #11807
Tracking PR #11660
Includes PR #10872
my old pr fixing same bug: #11543

[JulianHolzwarth]

rebase because of #12008 got merged

[MichelRottleuthner]

Finally found the time to test this one. I could reproduce the bug and verify that the fix is working. Nice one.
After struggling a bit to understand how the mutex/queue handling works here and what is implied by that, I wonder If we should just add some comments explaining what happens there to make it easier for the next one that looks at this code.

[JulianHolzwarth]

@MichelRottleuthner I will create a function for removing a thread from a mutex list ( this is what the _mutex_timeout() function is doing now) and I will add more comments in there.
Can I rebase and remove the "remove me" commits?

[MichelRottleuthner]

Can I rebase and remove the "remove me" commits?

Sure go ahead.

[JulianHolzwarth]

@MichelRottleuthner I rebased. And removed REMOVE_ME commits

[MichelRottleuthner]

PR was tested and changes are valid -> ACK