Fix ChannelMultiplexer to properly handle base channel creation failure.

remoting/protocol/channel_multiplexer.cc

Index: remoting/protocol/channel_multiplexer.cc
diff --git a/remoting/protocol/channel_multiplexer.cc b/remoting/protocol/channel_multiplexer.cc
index 3ad87bcf0d80ac60a4ee08a8eeb8f2552549f5e1..39cfc27bc526da7cca06c2ca13fb11dfe3701df2 100644
--- a/remoting/protocol/channel_multiplexer.cc
+++ b/remoting/protocol/channel_multiplexer.cc
@@ -9,7 +9,9 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/location.h"
+#include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
+#include "base/thread_task_runner_handle.h"
 #include "net/base/net_errors.h"
 #include "net/socket/stream_socket.h"
 #include "remoting/protocol/util.h"
@@ -364,7 +366,7 @@ ChannelMultiplexer::ChannelMultiplexer(ChannelFactory* factory,
     : base_channel_factory_(factory),
       base_channel_name_(base_channel_name),
       next_channel_id_(0),
-      destroyed_flag_(NULL) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
 }
 
 ChannelMultiplexer::~ChannelMultiplexer() {
@@ -374,9 +376,6 @@ ChannelMultiplexer::~ChannelMultiplexer() {
   // Cancel creation of the base channel if it hasn't finished.
   if (base_channel_factory_)
     base_channel_factory_->CancelChannelCreation(base_channel_name_);
-
-  if (destroyed_flag_)
-    *destroyed_flag_ = true;
 }
 
 void ChannelMultiplexer::CreateStreamChannel(
@@ -425,32 +424,40 @@ void ChannelMultiplexer::OnBaseChannelReady(
   base_channel_factory_ = NULL;
   base_channel_ = socket.Pass();
 
-  if (!base_channel_.get()) {
-    // Notify all callers that we can't create any channels.
-    for (std::list<PendingChannel>::iterator it = pending_channels_.begin();
-         it != pending_channels_.end(); ++it) {
-      it->callback.Run(scoped_ptr<net::StreamSocket>());
-    }
-    pending_channels_.clear();
-    return;
+  if (base_channel_.get()) {
+    // Initialize reader and writer.
+    reader_.Init(base_channel_.get(),
+                 base::Bind(&ChannelMultiplexer::OnIncomingPacket,
+                            base::Unretained(this)));
+    writer_.Init(base_channel_.get(),
+                 base::Bind(&ChannelMultiplexer::OnWriteFailed,
+                            base::Unretained(this)));
   }
 
-  // Initialize reader and writer.
-  reader_.Init(base_channel_.get(),
-               base::Bind(&ChannelMultiplexer::OnIncomingPacket,
-                          base::Unretained(this)));
-  writer_.Init(base_channel_.get(),
-               base::Bind(&ChannelMultiplexer::OnWriteFailed,
-                          base::Unretained(this)));
+  DoCreatePendingChannels();
+}
 
-  // Now create all pending channels.
-  for (std::list<PendingChannel>::iterator it = pending_channels_.begin();
-       it != pending_channels_.end(); ++it) {
-    it->callback.Run(GetOrCreateChannel(it->name)->CreateSocket());
-  }
-  pending_channels_.clear();
+void ChannelMultiplexer::DoCreatePendingChannels() {
+  if (pending_channels_.empty())
+    return;
+
+  // Every time this function is called it connects a single channel and posts a

[Wez, 2012/09/25 23:34:15]

typo: Remove the "a"

[Sergey Ulanov, 2012/09/26 00:46:45]

replaced "tasks" with "task" instead. 

+  // separate tasks to connect other channels. This is necessary because the
+  // callback may destroy the multiplexer or somehow else modify
+  // |pending_channels_| list (e.g. call CancelChannelCreation()).
+  base::ThreadTaskRunnerHandle::Get()->PostTask(
+      FROM_HERE, base::Bind(&ChannelMultiplexer::DoCreatePendingChannels,
+                            weak_factory_.GetWeakPtr()));
+
+  PendingChannel c = pending_channels_.front();
+  pending_channels_.erase(pending_channels_.begin());
+  scoped_ptr<net::StreamSocket> socket;
+  if (base_channel_.get())
+    socket = GetOrCreateChannel(c.name)->CreateSocket();
+  c.callback.Run(socket.Pass());
 }
 
+
 ChannelMultiplexer::MuxChannel* ChannelMultiplexer::GetOrCreateChannel(
     const std::string& name) {
   // Check if we already have a channel with the requested name.
@@ -467,15 +474,19 @@ ChannelMultiplexer::MuxChannel* ChannelMultiplexer::GetOrCreateChannel(
 
 
 void ChannelMultiplexer::OnWriteFailed(int error) {
-  bool destroyed = false;
-  destroyed_flag_ = &destroyed;
   for (std::map<std::string, MuxChannel*>::iterator it = channels_.begin();
        it != channels_.end(); ++it) {
+    base::ThreadTaskRunnerHandle::Get()->PostTask(
+        FROM_HERE, base::Bind(&ChannelMultiplexer::NotifyWriteFailed,
+                              weak_factory_.GetWeakPtr(), it->second->name()));
+  }
+}
+
+void ChannelMultiplexer::NotifyWriteFailed(const std::string& name) {
+  std::map<std::string, MuxChannel*>::iterator it = channels_.find(name);
+  if (it != channels_.end()) {
     it->second->OnWriteFailed();
-    if (destroyed)
-      return;
   }
-  destroyed_flag_ = NULL;
 }
 
 void ChannelMultiplexer::OnIncomingPacket(scoped_ptr<MultiplexPacket> packet,