chenBright commented on code in PR #3637:
URL: https://github.com/apache/brpc/pull/3637#discussion_r4336644423


##########
src/bthread/sys_futex.cpp:
##########
@@ -117,14 +137,18 @@ int futex_wake_private(void* addr1, int nwake) {
     int rc = 0;
     {
         std::unique_lock<pthread_mutex_t> mu1(simu_futex.lock);
-        nwake = (nwake < simu_futex.counts)? nwake: simu_futex.counts;
-        for (int i = 0; i < nwake; ++i) {
-            if ((rc = pthread_cond_signal(&simu_futex.cond)) != 0) {
+        while (nwakedup < nwake && !simu_futex.waiters.empty()) {
+            SimuFutexWaiter* waiter = simu_futex.waiters.head()->value();
+            if ((rc = pthread_cond_signal(&waiter->cond)) != 0) {
                 errno = rc;
                 break;
-            } else {
-                ++nwakedup;
             }
+            // Commit the notification to this waiter before releasing the
+            // mutex. Later wakes cannot count it again while it is 
rescheduled,
+            // and another waiter's timeout cannot consume its notification.
+            waiter->signaled = true;
+            waiter->RemoveFromList();

Review Comment:
   butil::LinkNode::RemoveFromList() resets both links to this, so calling it 
again on a detached node is safe. I've added a guard using the existing 
signaled flag to make the lifecycle explicit and avoid the redundant removal



##########
src/bthread/sys_futex.cpp:
##########
@@ -22,29 +22,41 @@
 #include "bthread/sys_futex.h"
 #include "butil/scoped_lock.h"
 #include "butil/atomicops.h"
+#include "butil/containers/linked_list.h"
 #include <pthread.h>
 #include <unordered_map>
 
 #if defined(OS_MACOSX)
 
 namespace bthread {
 
-class SimuFutex {
-public:
-    SimuFutex() : counts(0)
-                , ref(0) {
-        pthread_mutex_init(&lock, nullptr);
+struct SimuFutexWaiter : public butil::LinkNode<SimuFutexWaiter> {
+    SimuFutexWaiter() : signaled(false) {
         pthread_cond_init(&cond, nullptr);
     }
+    ~SimuFutexWaiter() {
+        // The futex mutex is still held when the stack waiter is destroyed.
+        // A notified waiter is already detached; otherwise remove it on 
timeout
+        // or error.
+        RemoveFromList();
+        pthread_cond_destroy(&cond);
+    }

Review Comment:
   butil::LinkNode::RemoveFromList() resets both links to this, so calling it 
again on a detached node is safe. I've added a guard using the existing 
signaled flag to make the lifecycle explicit and avoid the redundant removal



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to