Fix a logical race when adding an awakable to a wait set.

If the wait set is being woken on one thread and have an awakable added
on another (i.e. during MojoWait on the wait set), it is possible for
the awakable to be added after checking the signals state, but before
adding to the awakables list. In this case, the added awakable won't get
woken up. The fix is to ensure that checking the signals state and
adding to the awakables list is an atomic operation.

Review URL: https://codereview.chromium.org/1708953002

Cr-Commit-Position: refs/heads/master@{#376089}

1 files changed, 6 insertions, 1 deletions

diff --git a/mojo/edk/system/wait_set_dispatcher.cc b/mojo/edk/system/wait_set_dispatcher.cc
index 7d59f32..a083ce9 100644
--- a/mojo/edk/system/wait_set_dispatcher.cc
+++ b/mojo/edk/system/wait_set_dispatcher.cc
@@ -246,6 +246,12 @@ MojoResult WaitSetDispatcher::AddAwakable(Awakable* awakable,
                                           uintptr_t context,
                                           HandleSignalsState* signals_state) {
   base::AutoLock lock(lock_);
+  // |awakable_lock_| is acquired here instead of immediately before adding to
+  // |awakable_list_| because we need to check the signals state and add to
+  // |awakable_list_| as an atomic operation. If the pair isn't atomic, it is
+  // possible for the signals state to change after it is checked, but before
+  // the awakable is added. In that case, the added awakable won't be signalled.
+  base::AutoLock awakable_locker(awakable_lock_);
   HandleSignalsState state(GetHandleSignalsStateNoLock());
   if (state.satisfies(signals)) {
     if (signals_state)
@@ -258,7 +264,6 @@ MojoResult WaitSetDispatcher::AddAwakable(Awakable* awakable,
     return MOJO_RESULT_FAILED_PRECONDITION;
   }
 
-  base::AutoLock locker(awakable_lock_);
   awakable_list_.Add(awakable, signals, context);
   return MOJO_RESULT_OK;
 }