core/mutex: use thread_yield_higher() in mutex_unlock()#20890
core/mutex: use thread_yield_higher() in mutex_unlock()#20890benpicco merged 1 commit intoRIOT-OS:masterfrom
Conversation
Using `sched_switch()` in `mutex_unlock()` can result in crashes when `mutex_unlock()` is called from IRQ context. This however is a common pattern in RIOT to wake up a thread from IRQ. The reason for the crash is that `sched_switch()` assumes `thread_get_active()` to always return a non-`NULL` value. But when thread-less idle is used, no thread may be active after the last runnable thread exited. Using `thread_yield_higher()` instead solves the issue, as `thread_yield_higher()` is safe to call from IRQ context without an active thread. This fixes RIOT-OS#20812
|
With this patch masterthis PR |
|
That is expected: The use of |
IMHO that's even wrong - if the high prio thread is running, calling Only yielding to higher priority threads is much more sound. |
Neither The hot path is no waiter with a mutex anyway, and that is actually optimised anyway (with no call to either). So this would be net positive from a performance PoV IMO. |
Contribution description
Using
sched_switch()inmutex_unlock()can result in crashes whenmutex_unlock()is called from IRQ context. This however is a common pattern in RIOT to wake up a thread from IRQ. The reason for the crash is thatsched_switch()assumesthread_get_active()to always return a non-NULLvalue. But when thread-less idle is used, no thread may be active after the last runnable thread exited. Usingthread_yield_higher()instead solves the issue, asthread_yield_higher()is safe to call from IRQ context without an active thread.Testing procedure
As described in #20812 (also add the
assert(active_thread != NULL);tosched_switch()to not rely on undefined behavior triggering the crash, but to crash reliably on invalid state).In addition to fixing the crash, it should not introduce regressions. I'm not sure how good the test coverage for mutex is, but at least the following passed:
for test in tests/core/mutex_*; do make BOARD=nucleo-f767zi -C $test flash test; doneIssues/PRs references
This fixes #20812
Alternative to #20878