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

Merged
schkovich merged 6 commits from 2527_mutex_owner into develop 2025-06-19 08:57:08 -05:00
schkovich commented 2025-06-16 10:11:09 -05:00 (Migrated from github.com)

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

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:

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

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
lurch (Migrated from github.com) reviewed 2025-06-17 02:42:26 -05:00
@ -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) {
lurch (Migrated from github.com) commented 2025-06-17 02:42:25 -05:00

Is "other call" here supposed to be "other core" ?

Is "other call" here supposed to be "other core" ?
kilograham (Migrated from github.com) reviewed 2025-06-17 08:21:07 -05:00
@ -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) {
kilograham (Migrated from github.com) commented 2025-06-17 08:21:07 -05:00

yes

yes
schkovich (Migrated from github.com) reviewed 2025-06-17 11:22:48 -05:00
@ -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) {
schkovich (Migrated from github.com) commented 2025-06-17 11:22:48 -05:00

Maybe: ...to own the mutex concurrently.

Maybe: ...to own the mutex concurrently.
kilograham (Migrated from github.com) reviewed 2025-06-18 08:45:18 -05:00
@ -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) {
kilograham (Migrated from github.com) commented 2025-06-18 08:45:18 -05:00

maybe i improved it a little, idk

maybe i improved it a little, idk
kilograham (Migrated from github.com) approved these changes 2025-06-18 17:04:31 -05:00
Sign in to join this conversation.
No reviewers
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-raspberrypi/pico-sdk!2528
No description provided.