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


##########
sched/pthread/pthread_condbroadcast.c:
##########
@@ -68,22 +67,25 @@ int pthread_cond_broadcast(FAR pthread_cond_t *cond)
     }
   else
     {
+      int wcnt = atomic_read(COND_WAIT_COUNT(cond));
+
       /* Loop until all of the waiting threads have been restarted. */
 
-      while (cond->wait_count > 0)
+      while (wcnt > 0)
         {
-          /* If the value is less than zero (meaning that one or more
-           * thread is waiting), then post the condition semaphore.
-           * Only the highest priority waiting thread will get to execute
-           */
+          if (atomic_cmpxchg(COND_WAIT_COUNT(cond), &wcnt, wcnt - 1))
+            {
+              /* If the value is less than zero (meaning that one or more
+               * thread is waiting), then post the condition semaphore.
+               * Only the highest priority waiting thread will get to execute
+               */
 
-          ret = -nxsem_post(&cond->sem);
+              ret = -nxsem_post(&cond->sem);
 
-          /* Increment the semaphore count (as was done by the
-           * above post).
-           */
+              /* Decrement the waiter count */
 
-          cond->wait_count--;
+              wcnt--;

Review Comment:
   No, atomic_cmpxchg updates cond->wait_count but the local copy does not get 
modified. Either we need to re-read it here or do the decrement. Otherwise the 
while-loop will not work correctly.



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