Bug 885158 - If a message is sent on a closed IPC channel, delete it immediately. r=bent

Our old behavior was to enqueue it until the channel gets destructed.

ipc/chromium/src/chrome/common/ipc_channel.h

@@ -96,10 +96,8 @@ class Channel : public Message::Sender {
   // |message| must be allocated using operator new.  This object will be
   // deleted once the contents of the Message have been sent.
   //
-  //  FIXME bug 551500: the channel does not notice failures, so if the
-  //    renderer crashes, it will silently succeed, leaking the parameter.
-  //    At least the leak will be fixed by...
-  //
+  // If you Send() a message on a Close()'d channel, we delete the message
+  // immediately.
   virtual bool Send(Message* message);
 
 #if defined(OS_POSIX)

ipc/chromium/src/chrome/common/ipc_channel_posix.cc

@@ -31,6 +31,7 @@
 #include "chrome/common/file_descriptor_set_posix.h"
 #include "chrome/common/ipc_logging.h"
 #include "chrome/common/ipc_message_utils.h"
+#include "mozilla/ipc/ProtocolUtils.h"
 
 namespace IPC {
 
@@ -288,6 +289,7 @@ void Channel::ChannelImpl::Init(Mode mode, Listener* listener) {
   listener_ = listener;
   waiting_connect_ = true;
   processing_incoming_ = false;
+  closed_ = false;
 }
 
 bool Channel::ChannelImpl::CreatePipe(const std::wstring& channel_id,
@@ -711,6 +713,19 @@ bool Channel::ChannelImpl::Send(Message* message) {
   Logging::current()->OnSendMessage(message, L"");
 #endif
 
+  // If the channel has been closed, ProcessOutgoingMessages() is never going
+  // to pop anything off output_queue; output_queue will only get emptied when
+  // the channel is destructed.  We might as well delete message now, instead
+  // of waiting for the channel to be destructed.
+  if (closed_) {
+    if (mozilla::ipc::LoggingEnabled()) {
+      fprintf(stderr, "Can't send message %s, because this channel is closed.\n",
+              message->name());
+    }
+    delete message;
+    return false;
+  }
+
   output_queue_.push(message);
   if (!waiting_connect_) {
     if (!is_blocked_on_write_) {
@@ -825,6 +840,8 @@ void Channel::ChannelImpl::Close() {
     HANDLE_EINTR(close(*i));
   }
   input_overflow_fds_.clear();
+
+  closed_ = true;
 }
 
 //------------------------------------------------------------------------------

ipc/chromium/src/chrome/common/ipc_channel_posix.h

@@ -117,6 +117,9 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
   // problems.  TODO(darin): make this unnecessary
   bool processing_incoming_;
 
+  // This flag is set after we've closed the channel.
+  bool closed_;
+
   ScopedRunnableMethodFactory<ChannelImpl> factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ChannelImpl);

ipc/chromium/src/chrome/common/ipc_channel_win.cc

@@ -15,6 +15,7 @@
 #include "chrome/common/chrome_counters.h"
 #include "chrome/common/ipc_logging.h"
 #include "chrome/common/ipc_message_utils.h"
+#include "mozilla/ipc/ProtocolUtils.h"
 
 namespace IPC {
 //------------------------------------------------------------------------------
@@ -68,6 +69,7 @@ void Channel::ChannelImpl::Init(Mode mode, Listener* listener) {
   listener_ = listener;
   waiting_connect_ = (mode == MODE_SERVER);
   processing_incoming_ = false;
+  closed_ = false;
 }
 
 HANDLE Channel::ChannelImpl::GetServerPipeHandle() const {
@@ -104,6 +106,8 @@ void Channel::ChannelImpl::Close() {
 
   if (thread_check_.get())
     thread_check_.reset();
+
+  closed_ = true;
 }
 
 bool Channel::ChannelImpl::Send(Message* message) {
@@ -121,6 +125,15 @@ bool Channel::ChannelImpl::Send(Message* message) {
   Logging::current()->OnSendMessage(message, L"");
 #endif
 
+  if (closed_) {
+    if (mozilla::ipc::LoggingEnabled()) {
+      fprintf(stderr, "Can't send message %s, because this channel is closed.\n",
+              message->name());
+    }
+    delete message;
+    return false;
+  }
+
   output_queue_.push(message);
   // ensure waiting to write
   if (!waiting_connect_) {

ipc/chromium/src/chrome/common/ipc_channel_win.h

@@ -86,6 +86,9 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler {
   // problems.  TODO(darin): make this unnecessary
   bool processing_incoming_;
 
+  // This flag is set after Close() is run on the channel.
+  bool closed_;
+
   ScopedRunnableMethodFactory<ChannelImpl> factory_;
 
   scoped_ptr<NonThreadSafe> thread_check_;