async_context_threadsafe_background: fix incorrect mutex assertion in cross-core execute_sync() (#2528)

* async_context_threadsafe_background: fix incorrect mutex assertion in cross-core execute_sync()

In multicore configurations, `async_context_threadsafe_background_execute_sync()`
contained an overly strict assertion used during cross-core calls:

```c
hard_assert(!recursive_mutex_enter_count(&self->lock_mutex));
```

This check fails whenever the `lock_mutex` is held — regardless of *who*
owns it — even in valid situations where the async core is processing background
work.

The assertion does **not check ownership**, only that the `enter_count` is zero,
which leads to false-positive failures on valid cross-core calls.

This patch replaces the enter-count check with a core-aware assertion:

```c
hard_assert(self->lock_mutex.owner != calling_core);
```

This ensures the current core does not recursively hold the mutex, preventing
deadlocks while allowing valid usage where the *other* core owns the lock.

The patch ensures that both `get_core_num()` and `hard_assert()` remain inlined
as in the original implementation, preserving the performance characteristics
under `-Os` and `RelWithDebInfo` builds.

Fixes #2527

Signed-off-by: Goran Mišković <schkovich@users.noreply.github.com>

* fix indents

* Update async_context_threadsafe_background.c

Use pre-existing mutex owner method; add a comment

* oops

* typo

* Update async_context_threadsafe_background.c

---------

Signed-off-by: Goran Mišković <schkovich@users.noreply.github.com>
Co-authored-by: Graham Sanderson <graham.sanderson@raspberrypi.com>
This commit is contained in:
Goran Miskovic 2025-06-19 15:57:07 +02:00 committed by GitHub
parent a24bc13301
commit b1ca434581
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -138,8 +138,15 @@ static void lock_release(async_context_threadsafe_background_t *self) {
uint32_t async_context_threadsafe_background_execute_sync(async_context_t *self_base, uint32_t (*func)(void *param), void *param) {
async_context_threadsafe_background_t *self = (async_context_threadsafe_background_t*)self_base;
#if ASYNC_CONTEXT_THREADSAFE_BACKGROUND_MULTI_CORE
if (self_base->core_num != get_core_num()) {
hard_assert(!recursive_mutex_enter_count(&self->lock_mutex));
const uint calling_core = get_core_num();
if (self_base->core_num != calling_core) {
// This core must not hold the lock mutex, or this cross-core execute would deadlock. It is fine if the other core holds it.
// It would be a strange set of circumstances for it to do so, hence the hard_assert
// Note that this read of the owner is not synchronized with the other core; however we only
// care about it being set to `calling_core`, which the other core will not transition
// it either from or to.
hard_assert(recursive_mutex_owner(&self->lock_mutex) != calling_core);
sync_func_call_t call = {0};
call.worker.do_work = handle_sync_func_call;
call.func = func;