pussuw commented on code in PR #16194:
URL: https://github.com/apache/nuttx/pull/16194#discussion_r2058968249


##########
sched/semaphore/sem_waitirq.c:
##########
@@ -72,30 +72,54 @@ void nxsem_wait_irq(FAR struct tcb_s *wtcb, int errcode)
 {
   FAR struct tcb_s *rtcb = this_task();
   FAR sem_t *sem = wtcb->waitobj;
+  const bool mutex = NXSEM_IS_MUTEX(sem);
 
   /* It is possible that an interrupt/context switch beat us to the punch
    * and already changed the task's state.
    */
 
-  DEBUGASSERT(sem != NULL && atomic_read(NXSEM_COUNT(sem)) < 0);
+  DEBUGASSERT(sem != NULL &&
+              (mutex || atomic_read(NXSEM_COUNT(sem)) < 0) &&
+              (!mutex ||
+               (atomic_read(NXSEM_MHOLDER(sem)) & NXSEM_MBLOCKS_BIT)));
+
+  /* Mutex is never interrupted by a signal */
+
+  if (mutex && errcode == EINTR)
+    {
+      return;
+    }
 
   /* Restore the correct priority of all threads that hold references
    * to this semaphore.
    */
 
   nxsem_canceled(wtcb, sem);
 
-  /* And increment the count on the semaphore.  This releases the count
-   * that was taken by sem_post().  This count decremented the semaphore
-   * count to negative and caused the thread to be blocked in the first
-   * place.
+  /* Remove task from waiting list */
+
+  dq_rem((FAR dq_entry_t *)wtcb, SEM_WAITLIST(sem));
+
+  /* This restores the value to what it was before the previous sem_wait.
+   * This caused the thread to be blocked in the first place.
    */
 
-  atomic_fetch_add(NXSEM_COUNT(sem), 1);
+  if (mutex)

Review Comment:
   To prevent races the new holder should maybe be set here rather than in 
nxsem_wait



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to