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


##########
sched/semaphore/sem_recover.c:
##########
@@ -100,7 +101,39 @@ void nxsem_recover(FAR struct tcb_s *tcb)
        * place.
        */
 
-      atomic_fetch_add(NXSEM_COUNT(sem), 1);
+      if (NXSEM_IS_MUTEX(sem))
+        {
+          FAR dq_entry_t *wtcb;
+          uint32_t mholder;
+          uint32_t blocks_mask = ~NXSEM_MBLOCKS_BIT;
+
+          /* The TID of the mutex holder is correct but we need to
+           * update the blocking bit. Count the remaining tcbs in the
+           * blocking list (ignoring the one that is being cancelled).
+           */
+
+          for (wtcb = dq_peek(SEM_WAITLIST(sem)); wtcb; wtcb = dq_next(wtcb))

Review Comment:
   I think you are mixing up holder and waiter lists now. For a mutex only a 
single entry can EVER be in the holder list, and in this case since tcb is 
waiting for the mutex it CANNOT be the holder. For counting semaphores this is 
different.
   
   However, for waiter list there can be multiple entries.
   
   What I said above is correct, the tcb must be removed from the semaphores 
waiter list here. Otherwise it stays there forever.
   
   It also looks like for semaphores there is a bug, the tcb being removed 
stays in the wait list forever...



-- 
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