IPC & LibEvent fix

* Allow IPC::Listeners to send a message on OnChannelConnected.
* Fix a bug in MessagePumpLibevent::WatchFileDescriptor causing a read-after-free.

BUG=22451

Review URL: http://codereview.chromium.org/209061

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@26946 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 86 insertions, 4 deletions

diff --git a/base/message_pump_libevent.cc b/base/message_pump_libevent.cc
index a940081..e6d2cea 100644
--- a/base/message_pump_libevent.cc
+++ b/base/message_pump_libevent.cc
@@ -171,6 +171,23 @@ bool MessagePumpLibevent::WatchFileDescriptor(int fd,
     should_delete_event = false;
     // Ownership is transferred to the controller.
     evt.reset(new event);
+  } else {
+    // It's illegal to use this function to listen on 2 separate fds with the
+    // same |controller|.
+    if (EVENT_FD(evt.get()) != fd) {
+      NOTREACHED() << "FDs don't match" << EVENT_FD(evt.get()) << "!=" << fd;
+      return false;
+    }
+
+    // Make sure we don't pick up any funky internal libevent masks.
+    int old_interest_mask = evt.get()->ev_events &
+        (EV_READ | EV_WRITE | EV_PERSIST);
+
+    // Combine old/new event masks.
+    event_mask |= old_interest_mask;
+
+    // Must disarm the event before we can reuse it.
+    event_del(evt.get());
   }
 
   // Set current interest mask and message pump for this event.
diff --git a/base/message_pump_libevent.h b/base/message_pump_libevent.h
index a2a4e1c..8e2f77c 100644
--- a/base/message_pump_libevent.h
+++ b/base/message_pump_libevent.h
@@ -68,9 +68,9 @@ class MessagePumpLibevent : public MessagePump {
   };
 
   // Have the current thread's message loop watch for a a situation in which
-  // reading/writing to the FD can be performed without Blocking.
+  // reading/writing to the FD can be performed without blocking.
   // Callers must provide a preallocated FileDescriptorWatcher object which
-  // can later be used to manage the Lifetime of this event.
+  // can later be used to manage the lifetime of this event.
   // If a FileDescriptorWatcher is passed in which is already attached to
   // an event, then the effect is cumulative i.e. after the call |controller|
   // will watch both the previous event and the new one.
diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index c1f4564..24d11f4 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -913,8 +913,6 @@ void Channel::ChannelImpl::OnFileCanReadWithoutBlocking(int fd) {
   // This gives us a chance to kill the client if the incoming handshake
   // is invalid.
   if (send_server_hello_msg) {
-    // This should be our first write so there's no chance we can block here...
-    DCHECK(is_blocked_on_write_ == false);
     ProcessOutgoingMessages();
   }
 }
diff --git a/ipc/ipc_tests.cc b/ipc/ipc_tests.cc
index 7094308..43a368c 100644
--- a/ipc/ipc_tests.cc
+++ b/ipc/ipc_tests.cc
@@ -286,6 +286,73 @@ TEST_F(IPCChannelTest, ChannelProxyTest) {
   thread.Stop();
 }
 
+class ChannelListenerWithOnConnectedSend : public IPC::Channel::Listener {
+ public:
+  virtual void OnChannelConnected(int32 peer_pid) {
+    SendNextMessage();
+  }
+
+  virtual void OnMessageReceived(const IPC::Message& message) {
+    IPC::MessageIterator iter(message);
+
+    iter.NextInt();
+    const std::string data = iter.NextString();
+    const std::string big_string = iter.NextString();
+    EXPECT_EQ(kLongMessageStringNumBytes - 1, big_string.length());
+    SendNextMessage();
+  }
+
+  virtual void OnChannelError() {
+    // There is a race when closing the channel so the last message may be lost.
+    EXPECT_LE(messages_left_, 1);
+    MessageLoop::current()->Quit();
+  }
+
+  void Init(IPC::Message::Sender* s) {
+    sender_ = s;
+    messages_left_ = 50;
+  }
+
+ private:
+  void SendNextMessage() {
+    if (--messages_left_ == 0) {
+      MessageLoop::current()->Quit();
+    } else {
+      Send(sender_, "Foo");
+    }
+  }
+
+  IPC::Message::Sender* sender_;
+  int messages_left_;
+};
+
+TEST_F(IPCChannelTest, SendMessageInChannelConnected) {
+  // This tests the case of a listener sending back an event in it's
+  // OnChannelConnected handler.
+
+  ChannelListenerWithOnConnectedSend channel_listener;
+  // Setup IPC channel.
+  IPC::Channel channel(kTestClientChannel, IPC::Channel::MODE_SERVER,
+                       &channel_listener);
+  channel_listener.Init(&channel);
+  channel.Connect();
+
+  base::ProcessHandle process_handle = SpawnChild(TEST_CLIENT, &channel);
+  ASSERT_TRUE(process_handle);
+
+  Send(&channel, "hello from parent");
+
+  // Run message loop.
+  MessageLoop::current()->Run();
+
+  // Close Channel so client gets its OnChannelError() callback fired.
+  channel.Close();
+
+  // Cleanup child process.
+  EXPECT_TRUE(base::WaitForSingleProcess(process_handle, 5000));
+  base::CloseProcessHandle(process_handle);
+}
+
 MULTIPROCESS_TEST_MAIN(RunTestClient) {
   MessageLoopForIO main_message_loop;
   MyChannelListener channel_listener;