diff --git a/include/nuttx/semaphore.h b/include/nuttx/semaphore.h index 769da8baa4b..4178a51b474 100644 --- a/include/nuttx/semaphore.h +++ b/include/nuttx/semaphore.h @@ -74,6 +74,22 @@ #define NXSEM_MHOLDER(s) ((FAR atomic_t *)&((s)->val.mholder)) +/* Make a mutex holder value from a task id. The holder field keeps it in + * the low 31 bits because bit 31 is the blocking flag, and a task id can + * be negative here: nxsched_gettid() reports -ESRCH for a context that no + * longer maps to a task, which is exactly the state a task is in while + * nxtask_exit() tears it down and its group's mutexes get their final + * lock and unlock. Storing such an id unmasked would raise the blocking + * flag by accident, and an unlock would then try to wake waiters that do + * not exist. Masking keeps a lock and unlock from the same context + * consistent with each other, whatever the id's sign. + * + * The ids -1 and -2 would alias NXSEM_MRESET and NXSEM_NO_MHOLDER, but + * nxsched_gettid() produces only valid ids and -ESRCH, which is -3. + */ + +#define NXSEM_MAKE_MHOLDER(tid) ((uint32_t)(tid) & ~NXSEM_MBLOCKING_BIT) + /* Check if holder value (TID) is not NO_HOLDER or RESET */ #define NXSEM_MACQUIRED(h) (((h) & NXSEM_NO_MHOLDER) != NXSEM_NO_MHOLDER) diff --git a/libs/libc/semaphore/sem_trywait.c b/libs/libc/semaphore/sem_trywait.c index 8a62ac9bd88..a7a8bd39894 100644 --- a/libs/libc/semaphore/sem_trywait.c +++ b/libs/libc/semaphore/sem_trywait.c @@ -152,7 +152,7 @@ int nxsem_trywait(FAR sem_t *sem) return -EAGAIN; } - new = _SCHED_GETTID(); + new = NXSEM_MAKE_MHOLDER(_SCHED_GETTID()); } else { diff --git a/libs/libc/semaphore/sem_wait.c b/libs/libc/semaphore/sem_wait.c index 36fb106bfc5..20b73a02f95 100644 --- a/libs/libc/semaphore/sem_wait.c +++ b/libs/libc/semaphore/sem_wait.c @@ -180,7 +180,7 @@ int nxsem_wait(FAR sem_t *sem) break; } - new = _SCHED_GETTID(); + new = NXSEM_MAKE_MHOLDER(_SCHED_GETTID()); } else { diff --git a/sched/semaphore/sem_post.c b/sched/semaphore/sem_post.c index 3ffa4e8e890..8018a1d869a 100644 --- a/sched/semaphore/sem_post.c +++ b/sched/semaphore/sem_post.c @@ -99,11 +99,14 @@ int nxsem_post_slow(FAR sem_t *sem) mholder = atomic_fetch_or(NXSEM_MHOLDER(sem), NXSEM_MBLOCKING_BIT); /* Mutex post from another thread is not allowed, unless - * called from nxsem_reset + * called from nxsem_reset. The comparison uses the same encoding + * as the lock side so that a context whose id is -ESRCH, which is + * what a task being torn down reports, still matches its own lock. */ DEBUGASSERT(mholder == (NXSEM_MBLOCKING_BIT | NXSEM_MRESET) || - (mholder & (~NXSEM_MBLOCKING_BIT)) == nxsched_gettid()); + (mholder & (~NXSEM_MBLOCKING_BIT)) == + NXSEM_MAKE_MHOLDER(nxsched_gettid())); blocking = NXSEM_MBLOCKING(mholder); diff --git a/sched/semaphore/sem_trywait.c b/sched/semaphore/sem_trywait.c index d8ec4b3d96c..40f2e10275c 100644 --- a/sched/semaphore/sem_trywait.c +++ b/sched/semaphore/sem_trywait.c @@ -88,7 +88,7 @@ int nxsem_trywait_slow(FAR sem_t *sem) break; } - new = nxsched_gettid(); + new = NXSEM_MAKE_MHOLDER(nxsched_gettid()); } else { diff --git a/sched/semaphore/sem_wait.c b/sched/semaphore/sem_wait.c index e4c065bfb47..a8391b13b9f 100644 --- a/sched/semaphore/sem_wait.c +++ b/sched/semaphore/sem_wait.c @@ -119,9 +119,13 @@ int nxsem_wait_slow(FAR sem_t *sem) mholder = atomic_fetch_or(NXSEM_MHOLDER(sem), NXSEM_MBLOCKING_BIT); - /* Avoid mutex recursion, which is not allowed. */ + /* Avoid mutex recursion, which is not allowed. The comparison uses + * the lock side's encoding so that ids of either sign compare the + * way they were stored. + */ - DEBUGASSERT((mholder & (~NXSEM_MBLOCKING_BIT)) != nxsched_gettid()); + DEBUGASSERT((mholder & (~NXSEM_MBLOCKING_BIT)) != + NXSEM_MAKE_MHOLDER(nxsched_gettid())); if (NXSEM_MACQUIRED(mholder)) {