Take extra lock in base::WaitableEvent::WaitMany.

base/synchronization/waitable_event_posix.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <algorithm>
 #include <vector>
 
 #include "base/logging.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/synchronization/condition_variable.h"
@@ skipping 179 matching lines @@
       const bool return_value = sw.fired();
 
       // We can't acquire @lock_ before releasing the SyncWaiter lock (because
       // of locking order), however, in between the two a signal could be fired
       // and @sw would accept it, however we will still return false, so the
       // signal would be lost on an auto-reset WaitableEvent. Thus we call
       // Disable which makes sw::Fire return false.
       sw.Disable();
       sw.lock()->Release();
 
+      // This is a bug that has been enshrined in the interface of
+      // WaitableEvent now: |Dequeue| is called even when |sw.fired()| is true,
+      // even though it'll always return false in that case. However, taking
+      // the lock ensures that |Signal| has completed before we return and
+      // means that a WaitableEvent can synchronise its own destruction.
       kernel_->lock_.Acquire();
       kernel_->Dequeue(&sw, &sw);
       kernel_->lock_.Release();
 
       return return_value;
     }
 
     if (finite_time) {
       const TimeDelta max_wait(end_time - current_time);
       sw.cv()->TimedWait(max_wait);
@@ skipping 73 matching lines @@
   // remove our SyncWaiter from the wait-list
   for (size_t i = 0; i < count; ++i) {
     if (raw_waitables[i] != signaled_event) {
       raw_waitables[i]->kernel_->lock_.Acquire();
         // There's no possible ABA issue with the address of the SyncWaiter here
         // because it lives on the stack. Thus the tag value is just the pointer
         // value again.
         raw_waitables[i]->kernel_->Dequeue(&sw, &sw);
       raw_waitables[i]->kernel_->lock_.Release();
     } else {
+      // By taking this lock here we ensure that |Signal| has completed by the

[willchan no longer on Chromium, 2014/08/05 17:26:27]

Non-obvious on a first read that Signal() is holding this lock. Please add that
explanation.

[yhirano, 2014/08/06 11:49:49]

Done.

+      // time we return. This matches the behaviour of |Wait| and |TimedWait|.
+      raw_waitables[i]->kernel_->lock_.Acquire();
+      raw_waitables[i]->kernel_->lock_.Release();
       signaled_index = i;
     }
   }
 
   return signaled_index;
 }
 
 // -----------------------------------------------------------------------------
 // If return value == 0:
 //   The locks of the WaitableEvents have been taken in order and the Waiter has
@@ skipping 95 matching lines @@
       return true;
     }
   }
 
   return false;
 }
 
 // -----------------------------------------------------------------------------
 
 }  // namespace base