Eliminate a potential race in IPC::ChannelProxy

ipc/ipc_channel_proxy.cc

 // Copyright (c) 2012 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 "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/debug/trace_event.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 125 matching lines @@
 
 //------------------------------------------------------------------------------
 
 ChannelProxy::Context::Context(Listener* listener,
                                base::SingleThreadTaskRunner* ipc_task_runner)
     : listener_task_runner_(base::ThreadTaskRunnerHandle::Get()),
       listener_(listener),
       ipc_task_runner_(ipc_task_runner),
       channel_connected_called_(false),
       message_filter_router_(new MessageFilterRouter()),
-      peer_pid_(base::kNullProcessId) {
+      peer_pid_(base::kNullProcessId),
+      channel_is_connected_(false) {
   DCHECK(ipc_task_runner_.get());
 }
 
 ChannelProxy::Context::~Context() {
 }
 
 void ChannelProxy::Context::ClearIPCTaskRunner() {
   ipc_task_runner_ = NULL;
 }
 
@@ skipping 36 matching lines @@
       FROM_HERE, base::Bind(&Context::OnDispatchMessage, this, message));
   return true;
 }
 
 // Called on the IPC::Channel thread
 void ChannelProxy::Context::OnChannelConnected(int32 peer_pid) {
   // Add any pending filters.  This avoids a race condition where someone
   // creates a ChannelProxy, calls AddFilter, and then right after starts the
   // peer process.  The IO thread could receive a message before the task to add
   // the filter is run on the IO thread.
+  channel_is_connected_ = true;
   OnAddFilter();
 
   // We cache off the peer_pid so it can be safely accessed from both threads.
   peer_pid_ = channel_->peer_pid();

[jam, 2014/02/28 00:41:42]

nit: if you move these two lines to the two of the method, then you can avoid
adding the new member variable and instead use peer_pid_ instead as the signal

   for (size_t i = 0; i < filters_.size(); ++i)
     filters_[i]->OnChannelConnected(peer_pid);
 
   // See above comment about using listener_task_runner_ here.
   listener_task_runner_->PostTask(
       FROM_HERE, base::Bind(&Context::OnDispatchConnected, this));
 }
 
 // Called on the IPC::Channel thread
 void ChannelProxy::Context::OnChannelError() {
@@ skipping 54 matching lines @@
   if (!channel_.get()) {
     OnChannelClosed();
     return;
   }
   if (!channel_->Send(message.release()))
     OnChannelError();
 }
 
 // Called on the IPC::Channel thread
 void ChannelProxy::Context::OnAddFilter() {
+  if (!channel_is_connected_)
+    return;
+
   std::vector<scoped_refptr<MessageFilter> > new_filters;
   {
     base::AutoLock auto_lock(pending_filters_lock_);
     new_filters.swap(pending_filters_);
   }
 
   for (size_t i = 0; i < new_filters.size(); ++i) {
     filters_.push_back(new_filters[i]);
 
     message_filter_router_->AddFilter(new_filters[i].get());
@@ skipping 221 matching lines @@
   Channel* channel = context_.get()->channel_.get();
   // Channel must have been created first.
   DCHECK(channel) << context_.get()->channel_id_;
   return channel->GetPeerEuid(peer_euid);
 }
 #endif
 
 //-----------------------------------------------------------------------------
 
 }  // namespace IPC