Protect RefCount manipulation of InterfaceEndpoint with MultiplexRouter lock

mojo/public/cpp/bindings/lib/multiplex_router.cc

Index: mojo/public/cpp/bindings/lib/multiplex_router.cc
diff --git a/mojo/public/cpp/bindings/lib/multiplex_router.cc b/mojo/public/cpp/bindings/lib/multiplex_router.cc
index 2d8c53e33994243be49eebb5b417f1b1764157ea..af50a2eb315e9781b134f86e87e30fea4afdd0f1 100644
--- a/mojo/public/cpp/bindings/lib/multiplex_router.cc
+++ b/mojo/public/cpp/bindings/lib/multiplex_router.cc
@@ -174,29 +174,27 @@ class MultiplexRouter::InterfaceEndpoint
 
   void OnHandleReady(MojoResult result) {
     DCHECK(task_runner_->BelongsToCurrentThread());
-    scoped_refptr<InterfaceEndpoint> self_protector(this);
     scoped_refptr<MultiplexRouter> router_protector(router_);
 
     // Because we never close |sync_message_event_{sender,receiver}_| before
     // destruction or set a deadline, |result| should always be MOJO_RESULT_OK.
     DCHECK_EQ(MOJO_RESULT_OK, result);
-    bool reset_sync_watcher = false;
-    {
-      MayAutoLock locker(router_->lock_.get());
 
-      bool more_to_process = router_->ProcessFirstSyncMessageForEndpoint(id_);
+    MayAutoLock locker(router_->lock_.get());
+    scoped_refptr<InterfaceEndpoint> self_protector(this);
 
-      if (!more_to_process)
-        ResetSyncMessageSignal();
+    bool more_to_process = router_->ProcessFirstSyncMessageForEndpoint(id_);
 
-      // Currently there are no queued sync messages and the peer has closed so
-      // there won't be incoming sync messages in the future.
-      reset_sync_watcher = !more_to_process && peer_closed_;
-    }
-    if (reset_sync_watcher) {
+    if (!more_to_process)
+      ResetSyncMessageSignal();
+
+    // Currently there are no queued sync messages and the peer has closed so
+    // there won't be incoming sync messages in the future.
+    if (!more_to_process && peer_closed_) {
       // If a SyncWatch() call (or multiple ones) of this interface endpoint is
       // on the call stack, resetting the sync watcher will allow it to exit
       // when the call stack unwinds to that frame.
+      MayAutoUnlock unlocker(router_->lock_.get());

[yzshen1, 2017/02/09 17:16:50]

|sync_watcher_| is used exclusively on the |task_runner_|'s thread, so it is
safe to use inside/outside of lock. In this case, we may just directly reset
while holding the lock to save unlock/re-lock cost. Lock contention shouldn't be
an issue here.

[tzik, 2017/02/09 22:41:22]

Done.

       sync_watcher_.reset();
     }
   }