Don't call the other wait set impl dispatcher methods under mutex either.

mojo/edk/system/dispatcher.cc

 // Copyright 2013 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/dispatcher.h"
 
 #include "base/logging.h"
 #include "mojo/edk/system/configuration.h"
 #include "mojo/edk/system/data_pipe_consumer_dispatcher.h"
 #include "mojo/edk/system/data_pipe_producer_dispatcher.h"
@@ skipping 258 matching lines @@
     uint64_t num_bytes,
     MojoMapBufferFlags flags,
     std::unique_ptr<PlatformSharedBufferMapping>* mapping) {
   MutexLocker locker(&mutex_);
   if (is_closed_)
     return MOJO_RESULT_INVALID_ARGUMENT;
 
   return MapBufferImplNoLock(offset, num_bytes, flags, mapping);
 }
 
+// Note: The following three wait set methods don't lock |mutex_|, and leave
+// everything for |WaitSet...Impl()| to do. (We could just make these methods
+// virtual, but we prefer to have a separate "impl" methods for consistency.)
 MojoResult Dispatcher::WaitSetAdd(
     UserPointer<const MojoWaitSetAddOptions> options,
     Handle&& handle,
     MojoHandleSignals signals,
     uint64_t cookie) {
-  MutexLocker locker(&mutex_);
-  if (is_closed_)
-    return MOJO_RESULT_INVALID_ARGUMENT;
-
-  return WaitSetAddImplNoLock(options, std::move(handle), signals, cookie);
+  return WaitSetAddImpl(options, std::move(handle), signals, cookie);
 }
 
 MojoResult Dispatcher::WaitSetRemove(uint64_t cookie) {
-  MutexLocker locker(&mutex_);
-  if (is_closed_)
-    return MOJO_RESULT_INVALID_ARGUMENT;
-
-  return WaitSetRemoveImplNoLock(cookie);
+  return WaitSetRemoveImpl(cookie);
 }
 
 MojoResult Dispatcher::WaitSetWait(MojoDeadline deadline,
                                    UserPointer<uint32_t> num_results,
                                    UserPointer<MojoWaitSetResult> results,
                                    UserPointer<uint32_t> max_results) {
-  // Note: This doesn't lock |mutex_|, and leaves everything for
-  // |WaitSetWaitImpl()| to do. (We could just make this method virtual, but we
-  // prefer to have a separate "impl" method for consistency.)
   return WaitSetWaitImpl(deadline, num_results, results, max_results);
 }
 
 HandleSignalsState Dispatcher::GetHandleSignalsState() const {
   MutexLocker locker(&mutex_);
   if (is_closed_)
     return HandleSignalsState();
 
   return GetHandleSignalsStateImplNoLock();
 }
@@ skipping 187 matching lines @@
     uint64_t /*offset*/,
     uint64_t /*num_bytes*/,
     MojoMapBufferFlags /*flags*/,
     std::unique_ptr<PlatformSharedBufferMapping>* /*mapping*/) {
   mutex_.AssertHeld();
   DCHECK(!is_closed_);
   // By default, not supported. Only needed for buffer dispatchers.
   return MOJO_RESULT_INVALID_ARGUMENT;
 }
 
-MojoResult Dispatcher::WaitSetAddImplNoLock(
+// Note that the following three methods are *not* called under |mutex_| and
+// |is_closed_| hasn't been checked. However, since we'll return
+// |MOJO_RESULT_INVALID_ARGUMENT| regardless of the value of |is_closed_| (these
+// methods are only needed for wait set dispatchers), we don't need to lock
+// |mutex_| and check |is_closed_|.
+MojoResult Dispatcher::WaitSetAddImpl(
     UserPointer<const MojoWaitSetAddOptions> /*options*/,
     Handle&& /*handle*/,
     MojoHandleSignals /*signals*/,
     uint64_t /*cookie*/) {
-  mutex_.AssertHeld();
-  DCHECK(!is_closed_);
-  // By default, not supported. Only needed for wait set dispatchers.
+  // See note above.
   return MOJO_RESULT_INVALID_ARGUMENT;
 }
 
-MojoResult Dispatcher::WaitSetRemoveImplNoLock(uint64_t /*cookie*/) {
-  mutex_.AssertHeld();
-  DCHECK(!is_closed_);
-  // By default, not supported. Only needed for wait set dispatchers.
+MojoResult Dispatcher::WaitSetRemoveImpl(uint64_t /*cookie*/) {
+  // See note above |WaitSetAddImpl()|.
   return MOJO_RESULT_INVALID_ARGUMENT;
 }
 
 MojoResult Dispatcher::WaitSetWaitImpl(
     MojoDeadline /*deadline*/,
     UserPointer<uint32_t> /*num_results*/,
     UserPointer<MojoWaitSetResult> /*results*/,
     UserPointer<uint32_t> /*max_results*/) {
-  // Note that this is *not* called under |mutex_| and |is_closed_| hasn't been
-  // checked. But since we'll return |MOJO_RESULT_INVALID_ARGUMENT| in either
-  // case (by default, this is not supported: it's only needed for wait set
-  // dispatchers), we don't need to lock |mutex_| and check |is_closed_|.
+  // See note above |WaitSetAddImpl()|.
   return MOJO_RESULT_INVALID_ARGUMENT;
 }
 
 HandleSignalsState Dispatcher::GetHandleSignalsStateImplNoLock() const {
   mutex_.AssertHeld();
   DCHECK(!is_closed_);
   // By default, waiting isn't supported. Only dispatchers that can be waited on
   // will do something nontrivial.
   return HandleSignalsState();
 }
@@ skipping 94 matching lines @@
   // want to remove or weaken).
   MutexLocker locker(&mutex_);
 #endif
 
   return EndSerializeAndCloseImplNoLock(channel, destination, actual_size,
                                         platform_handles);
 }
 
 }  // namespace system
 }  // namespace mojo