Eliminate deferred destruction of AssociatedGroupController

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 922cb6ce6111ae3ef702178457fb0671d969ad7b..65510f04ee3d517a58835515cb60444622b61645 100644
--- a/mojo/public/cpp/bindings/lib/multiplex_router.cc
+++ b/mojo/public/cpp/bindings/lib/multiplex_router.cc
@@ -285,8 +285,8 @@ MultiplexRouter::MultiplexRouter(
     bool set_interface_id_namesapce_bit,
     ScopedMessagePipeHandle message_pipe,
     scoped_refptr<base::SingleThreadTaskRunner> runner)
-    : AssociatedGroupController(base::ThreadTaskRunnerHandle::Get()),
-      set_interface_id_namespace_bit_(set_interface_id_namesapce_bit),
+    : set_interface_id_namespace_bit_(set_interface_id_namesapce_bit),
+      task_runner_(runner),
       header_validator_(this),
       connector_(std::move(message_pipe),
                  Connector::MULTI_THREADED_SEND,
@@ -298,14 +298,14 @@ MultiplexRouter::MultiplexRouter(
       encountered_error_(false),
       paused_(false),
       testing_mode_(false) {
+  DCHECK(task_runner_->BelongsToCurrentThread());
   // Always participate in sync handle watching, because even if it doesn't
   // expect sync requests during sync handle watching, it may still need to
   // dispatch messages to associated endpoints on a different thread.
   connector_.AllowWokenUpBySyncWatchOnSameThread();
   connector_.set_incoming_receiver(&header_validator_);
   connector_.set_connection_error_handler(
-      base::Bind(&MultiplexRouter::OnPipeConnectionError,
-                 base::Unretained(this)));

[yzshen1, 2016/07/26 21:21:15]

It seems safe to use base::Unretained() here. The master interface endpoint will
hold a ref to the multiplex router and it will call CloseMessagePipe or
PassMessagePipe before it releases that ref.

If we let the connector holds a ref it is easier to leak. (I think it already
leaks if we call PassMessagePipe() because it doesn't reset the connection error
handler.)

So maybe it makes sense to keep base::Unretained() and add comments to
MultiplexRouter header file saying that CloseMessagePipe or PassMessageMessage
must be called on the master thread before destruction?

[Ken Rockot(use gerrit already), 2016/07/26 22:07:00]

Done

+      base::Bind(&MultiplexRouter::OnPipeConnectionError, this));
 }
 
 MultiplexRouter::~MultiplexRouter() {
@@ -590,6 +590,10 @@ void MultiplexRouter::OnPipeConnectionError() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   scoped_refptr<MultiplexRouter> protector(this);
+
+  // We explcitly clear the error handler since it has a ref to |this|.
+  connector_.set_connection_error_handler(base::Closure());
+
   base::AutoLock locker(lock_);
 
   encountered_error_ = true;