[mojo] Ports EDK

mojo/edk/system/wait_set_dispatcher.cc

 // Copyright 2015 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 "mojo/edk/system/wait_set_dispatcher.h"
 
 #include <stdint.h>
 
 #include <algorithm>
 #include <utility>
@@ skipping 21 matching lines @@
   WaitSetDispatcher* const dispatcher_;
 };
 
 WaitSetDispatcher::WaitState::WaitState() {}
 
 WaitSetDispatcher::WaitState::~WaitState() {}
 
 WaitSetDispatcher::WaitSetDispatcher()
     : waiter_(new WaitSetDispatcher::Waiter(this)) {}
 
-WaitSetDispatcher::~WaitSetDispatcher() {
-  DCHECK(waiting_dispatchers_.empty());
-  DCHECK(awoken_queue_.empty());
-  DCHECK(processed_dispatchers_.empty());
-}
-
 Dispatcher::Type WaitSetDispatcher::GetType() const {
   return Type::WAIT_SET;
 }
 
-void WaitSetDispatcher::CloseImplNoLock() {
-  lock().AssertAcquired();
-  for (const auto& entry : waiting_dispatchers_)
-    entry.second.dispatcher->RemoveAwakable(waiter_.get(), nullptr);
-  waiting_dispatchers_.clear();
+MojoResult WaitSetDispatcher::Close() {
+  {
+    base::AutoLock lock(lock_);

[Anand Mistry (off Chromium), 2016/01/28 02:26:25]

Why not just hold lock_ for the entire function. It's difficult to reason, but
this code was written under that assumption, so there might be races now.

+    if (is_closed_)
+      return MOJO_RESULT_INVALID_ARGUMENT;
+    is_closed_ = true;
+  }
+
+  {
+    base::AutoLock locker(awakable_lock_);
+    awakable_list_.CancelAll();
+  }
+
+  {
+    base::AutoLock lock(lock_);
+    for (const auto& entry : waiting_dispatchers_)
+      entry.second.dispatcher->RemoveAwakable(waiter_.get(), nullptr);
+    waiting_dispatchers_.clear();
+  }
 
   base::AutoLock locker(awoken_lock_);
   awoken_queue_.clear();
   processed_dispatchers_.clear();
+
+  return MOJO_RESULT_OK;
 }
 
-MojoResult WaitSetDispatcher::AddWaitingDispatcherImplNoLock(
+MojoResult WaitSetDispatcher::AddWaitingDispatcher(
     const scoped_refptr<Dispatcher>& dispatcher,
     MojoHandleSignals signals,
     uintptr_t context) {
-  lock().AssertAcquired();
   if (dispatcher == this)
     return MOJO_RESULT_INVALID_ARGUMENT;
 
+  base::AutoLock lock(lock_);
+
+  if (is_closed_)
+    return MOJO_RESULT_INVALID_ARGUMENT;
+
   uintptr_t dispatcher_handle = reinterpret_cast<uintptr_t>(dispatcher.get());
   auto it = waiting_dispatchers_.find(dispatcher_handle);
   if (it != waiting_dispatchers_.end()) {
     return MOJO_RESULT_ALREADY_EXISTS;
   }
 
   const MojoResult result = dispatcher->AddAwakable(waiter_.get(), signals,
                                                     dispatcher_handle, nullptr);
   if (result == MOJO_RESULT_INVALID_ARGUMENT) {
     // Dispatcher is closed.
     return result;
   } else if (result != MOJO_RESULT_OK) {
     WakeDispatcher(result, dispatcher_handle);
   }
 
   WaitState state;
   state.dispatcher = dispatcher;
   state.context = context;
   state.signals = signals;
   bool inserted = waiting_dispatchers_.insert(
       std::make_pair(dispatcher_handle, state)).second;
   DCHECK(inserted);
 
   return MOJO_RESULT_OK;
 }
 
-MojoResult WaitSetDispatcher::RemoveWaitingDispatcherImplNoLock(
+MojoResult WaitSetDispatcher::RemoveWaitingDispatcher(
     const scoped_refptr<Dispatcher>& dispatcher) {
-  lock().AssertAcquired();
   uintptr_t dispatcher_handle = reinterpret_cast<uintptr_t>(dispatcher.get());
-  auto it = waiting_dispatchers_.find(dispatcher_handle);
-  if (it == waiting_dispatchers_.end())
-    return MOJO_RESULT_NOT_FOUND;
+  {
+    base::AutoLock lock(lock_);

[Anand Mistry (off Chromium), 2016/01/28 02:26:25]

Need to handle the closed state. This can return "not found" when the wait set
is being closed, which isn't right.

+    auto it = waiting_dispatchers_.find(dispatcher_handle);
+    if (it == waiting_dispatchers_.end())
+      return MOJO_RESULT_NOT_FOUND;
 
-  dispatcher->RemoveAwakable(waiter_.get(), nullptr);
-  // At this point, it should not be possible for |waiter_| to be woken with
-  // |dispatcher|.
-  waiting_dispatchers_.erase(it);
+    dispatcher->RemoveAwakable(waiter_.get(), nullptr);
+    // At this point, it should not be possible for |waiter_| to be woken with
+    // |dispatcher|.
+    waiting_dispatchers_.erase(it);
+  }
 
   base::AutoLock locker(awoken_lock_);
   int num_erased = 0;
   for (auto it = awoken_queue_.begin(); it != awoken_queue_.end();) {
     if (it->first == dispatcher_handle) {
       it = awoken_queue_.erase(it);
       num_erased++;
     } else {
       ++it;
     }
   }
   // The dispatcher should only exist in the queue once.
   DCHECK_LE(num_erased, 1);
   processed_dispatchers_.erase(
       std::remove(processed_dispatchers_.begin(), processed_dispatchers_.end(),
                   dispatcher_handle),
       processed_dispatchers_.end());
 
   return MOJO_RESULT_OK;
 }
 
-MojoResult WaitSetDispatcher::GetReadyDispatchersImplNoLock(
+MojoResult WaitSetDispatcher::GetReadyDispatchers(
     uint32_t* count,
     DispatcherVector* dispatchers,
     MojoResult* results,
     uintptr_t* contexts) {
-  lock().AssertAcquired();
+  base::AutoLock lock(lock_);
+
   dispatchers->clear();
 
   // Re-queue any already retrieved dispatchers. These should be the dispatchers
   // that were returned on the last call to this function. This loop is
   // necessary to preserve the logically level-triggering behaviour of waiting
   // in Mojo. In particular, if no action is taken on a signal, that signal
   // continues to be satisfied, and therefore a |MojoWait()| on that
   // handle/signal continues to return immediately.
   std::deque<uintptr_t> pending;
   {
@@ skipping 52 matching lines @@
     num_woken++;
   }
 
   *count = num_woken;
   if (!num_woken)
     return MOJO_RESULT_SHOULD_WAIT;
 
   return MOJO_RESULT_OK;
 }
 
-void WaitSetDispatcher::CancelAllAwakablesNoLock() {
-  lock().AssertAcquired();
-  base::AutoLock locker(awakable_lock_);
-  awakable_list_.CancelAll();
+HandleSignalsState WaitSetDispatcher::GetHandleSignalsState() const {

[Anand Mistry (off Chromium), 2016/01/28 02:26:25]

Need to handle the closed state. Without it, it's possible to add an awakable to
a closed wait set.

+  HandleSignalsState rv;
+  rv.satisfiable_signals = MOJO_HANDLE_SIGNAL_READABLE;
+  base::AutoLock locker(awoken_lock_);
+  if (!awoken_queue_.empty() || !processed_dispatchers_.empty())
+    rv.satisfied_signals = MOJO_HANDLE_SIGNAL_READABLE;
+  return rv;
 }
 
-MojoResult WaitSetDispatcher::AddAwakableImplNoLock(
-    Awakable* awakable,
-    MojoHandleSignals signals,
-    uintptr_t context,
-    HandleSignalsState* signals_state) {
-  lock().AssertAcquired();
-
-  HandleSignalsState state(GetHandleSignalsStateImplNoLock());
+MojoResult WaitSetDispatcher::AddAwakable(Awakable* awakable,
+                                          MojoHandleSignals signals,
+                                          uintptr_t context,
+                                          HandleSignalsState* signals_state) {
+  HandleSignalsState state(GetHandleSignalsState());
   if (state.satisfies(signals)) {
     if (signals_state)
       *signals_state = state;
     return MOJO_RESULT_ALREADY_EXISTS;
   }
   if (!state.can_satisfy(signals)) {
     if (signals_state)
       *signals_state = state;
     return MOJO_RESULT_FAILED_PRECONDITION;
   }
 
   base::AutoLock locker(awakable_lock_);
   awakable_list_.Add(awakable, signals, context);
   return MOJO_RESULT_OK;
 }
 
-void WaitSetDispatcher::RemoveAwakableImplNoLock(
-    Awakable* awakable,
-    HandleSignalsState* signals_state) {
-  lock().AssertAcquired();
+void WaitSetDispatcher::RemoveAwakable(Awakable* awakable,
+                                       HandleSignalsState* signals_state) {
   base::AutoLock locker(awakable_lock_);
   awakable_list_.Remove(awakable);
   if (signals_state)
-    *signals_state = GetHandleSignalsStateImplNoLock();
+    *signals_state = GetHandleSignalsState();
 }
 
-HandleSignalsState WaitSetDispatcher::GetHandleSignalsStateImplNoLock() const {
-  lock().AssertAcquired();
-  HandleSignalsState rv;
-  rv.satisfiable_signals = MOJO_HANDLE_SIGNAL_READABLE;
-  base::AutoLock locker(awoken_lock_);
-  if (!awoken_queue_.empty() || !processed_dispatchers_.empty())
-    rv.satisfied_signals = MOJO_HANDLE_SIGNAL_READABLE;
-  return rv;
+bool WaitSetDispatcher::BeginTransit() {
+  // You can't transfer wait sets!
+  return false;
 }
 
-scoped_refptr<Dispatcher>
-WaitSetDispatcher::CreateEquivalentDispatcherAndCloseImplNoLock() {
-  lock().AssertAcquired();
-  LOG(ERROR) << "Attempting to serialize WaitSet";
-  CloseImplNoLock();
-  return new WaitSetDispatcher();
+WaitSetDispatcher::~WaitSetDispatcher() {
+  DCHECK(waiting_dispatchers_.empty());
+  DCHECK(awoken_queue_.empty());
+  DCHECK(processed_dispatchers_.empty());
 }
 
 void WaitSetDispatcher::WakeDispatcher(MojoResult result, uintptr_t context) {
   {
     base::AutoLock locker(awoken_lock_);
 
     if (result == MOJO_RESULT_ALREADY_EXISTS)
       result = MOJO_RESULT_OK;
 
     awoken_queue_.push_back(std::make_pair(context, result));
   }
 
   base::AutoLock locker(awakable_lock_);
   HandleSignalsState signals_state;
   signals_state.satisfiable_signals = MOJO_HANDLE_SIGNAL_READABLE;
   signals_state.satisfied_signals = MOJO_HANDLE_SIGNAL_READABLE;
   awakable_list_.AwakeForStateChange(signals_state);
 }
 
 }  // namespace edk
 }  // namespace mojo