Implement command line switch to allow using a FIFO or a socketpair() for IPC on POSIX.

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

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

6 files changed, 169 insertions, 33 deletions

diff --git a/chrome/common/ipc_channel.h b/chrome/common/ipc_channel.h
index bd7bf05..9b7f8b2 100644
--- a/chrome/common/ipc_channel.h
+++ b/chrome/common/ipc_channel.h
@@ -85,6 +85,22 @@ class Channel : public Message::Sender {
   //
   virtual bool Send(Message* message);
 
+#if defined(OS_POSIX)
+  // On POSIX an IPC::Channel wraps a socketpair(), this method returns the
+  // FD # for the client end of the socket and the equivalent FD# to use for
+  // mapping it into the Child process.
+  // This method may only be called on the server side of a channel.
+  //
+  // If the kTestingChannelID flag is specified on the command line then
+  // a named FIFO is used as the channel transport mechanism rather than a
+  // socketpair() in which case this method returns -1 for both parameters.
+  void GetClientFileDescriptorMapping(int *src_fd, int *dest_fd);
+
+  // Call this method on the server side of the IPC Channel once a client is
+  // connected in order to close the client side of the socketpair().
+  void OnClientConnected();
+#endif  // defined(OS_POSIX)
+
  private:
   // PIMPL to which all channel calls are delegated.
   class ChannelImpl;
diff --git a/chrome/common/ipc_channel_posix.cc b/chrome/common/ipc_channel_posix.cc
index 2cdbca4..dad5ff8 100644
--- a/chrome/common/ipc_channel_posix.cc
+++ b/chrome/common/ipc_channel_posix.cc
@@ -16,21 +16,29 @@
 #include <sys/un.h>
 #endif
 
-
+#include "base/command_line.h"
 #include "base/logging.h"
 #include "base/process_util.h"
 #include "base/scoped_ptr.h"
 #include "base/string_util.h"
 #include "chrome/common/chrome_counters.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/ipc_message_utils.h"
 
 namespace IPC {
 
 //------------------------------------------------------------------------------
-// TODO(playmobil): Only use FIFOs for debugging, for real work, use a
-// socketpair.
 namespace {
 
+// Used to map a channel name to the equivalent FD # in the client process.
+int ChannelNameToClientFD(const std::string& channel_id) {
+  // TODO(playmobil): Support a smarter mapping that allows for a process to
+  // be a client of multiple channels.
+  const int kClientChannelID = 50;
+  return kClientChannelID;
+}
+
+//------------------------------------------------------------------------------
 // The -1 is to take the NULL terminator into account.
 #if defined(OS_LINUX)
 const size_t kMaxPipeNameLength = UNIX_PATH_MAX - 1;
@@ -156,8 +164,10 @@ Channel::ChannelImpl::ChannelImpl(const std::wstring& channel_id, Mode mode,
     : mode_(mode),
       is_blocked_on_write_(false),
       message_send_bytes_written_(0),
+      uses_fifo_(CommandLine().HasSwitch(switches::kTestingChannelID)),
       server_listen_pipe_(-1),
       pipe_(-1),
+      client_pipe_(-1),
       listener_(listener),
       waiting_connect_(true),
       processing_incoming_(false),
@@ -183,19 +193,42 @@ bool Channel::ChannelImpl::CreatePipe(const std::wstring& channel_id,
                                       Mode mode) {
   DCHECK(server_listen_pipe_ == -1 && pipe_ == -1);
 
-  // TODO(playmobil): Should we just change pipe_name to be a normal string
-  // everywhere?
-  pipe_name_ = WideToUTF8(PipeName(channel_id));
+  if (uses_fifo_) {
+    // TODO(playmobil): Should we just change pipe_name to be a normal string
+    // everywhere?
+    pipe_name_ = WideToUTF8(PipeName(channel_id));
 
-  if (mode == MODE_SERVER) {
-    if (!CreateServerFifo(pipe_name_, &server_listen_pipe_)) {
-      return false;
+    if (mode == MODE_SERVER) {
+      if (!CreateServerFifo(pipe_name_, &server_listen_pipe_)) {
+        return false;
+      }
+    } else {
+      if (!ClientConnectToFifo(pipe_name_, &pipe_)) {
+        return false;
+      }
+      waiting_connect_ = false;
     }
   } else {
-    if (!ClientConnectToFifo(pipe_name_, &pipe_)) {
-      return false;
+    // socketpair()
+    if (mode == MODE_SERVER) {
+      int pipe_fds[2];
+      if (socketpair(AF_UNIX, SOCK_STREAM, 0, pipe_fds) != 0) {
+        return false;
+      }
+      // Set both ends to be non-blocking.
+      if (fcntl(pipe_fds[0], F_SETFL, O_NONBLOCK) == -1 ||
+          fcntl(pipe_fds[1], F_SETFL, O_NONBLOCK) == -1) {
+        close(pipe_fds[0]);
+        close(pipe_fds[1]);
+        return false;
+      }
+      pipe_ = pipe_fds[0];
+      client_pipe_ = pipe_fds[1];
+    } else {
+      pipe_ = ChannelNameToClientFD(pipe_name_);
+      DCHECK(pipe_ > 0);
+      waiting_connect_ = false;
     }
-    waiting_connect_ = false;
   }
 
   // Create the Hello message to be sent when Connect is called
@@ -212,7 +245,7 @@ bool Channel::ChannelImpl::CreatePipe(const std::wstring& channel_id,
 }
 
 bool Channel::ChannelImpl::Connect() {
-  if (mode_ == MODE_SERVER) {
+  if (mode_ == MODE_SERVER && uses_fifo_) {
     if (server_listen_pipe_ == -1) {
       return false;
     }
@@ -396,10 +429,27 @@ bool Channel::ChannelImpl::Send(Message* message) {
   return true;
 }
 
+void Channel::ChannelImpl::GetClientFileDescriptorMapping(int *src_fd,
+                                                          int *dest_fd) {
+  DCHECK(mode_ == MODE_SERVER);
+  *src_fd = client_pipe_;
+  *dest_fd = ChannelNameToClientFD(pipe_name_);
+}
+
+void Channel::ChannelImpl::OnClientConnected() {
+  DCHECK(mode_ == MODE_SERVER);
+  close(client_pipe_);
+  client_pipe_ = -1;
+}
+
 // Called by libevent when we can read from th pipe without blocking.
 void Channel::ChannelImpl::OnFileCanReadWithoutBlocking(int fd) {
   bool send_server_hello_msg = false;
   if (waiting_connect_ && mode_ == MODE_SERVER) {
+    // In the case of a socketpair() the server starts listening on its end
+    // of the pipe in Connect().
+    DCHECK(uses_fifo_);
+
     if (!ServerAcceptFifoConnection(server_listen_pipe_, &pipe_)) {
       Close();
     }
@@ -465,6 +515,10 @@ void Channel::ChannelImpl::Close() {
     close(pipe_);
     pipe_ = -1;
   }
+  if (client_pipe_ != -1) {
+    close(client_pipe_);
+    client_pipe_ = -1;
+  }
 
   // Unlink the FIFO
   unlink(pipe_name_.c_str());
@@ -503,4 +557,13 @@ bool Channel::Send(Message* message) {
   return channel_impl_->Send(message);
 }
 
+void Channel::GetClientFileDescriptorMapping(int *src_fd, int *dest_fd) {
+  return channel_impl_->GetClientFileDescriptorMapping(src_fd, dest_fd);
+}
+
+void Channel::OnClientConnected() {
+  return channel_impl_->OnClientConnected();
+}
+
+
 }  // namespace IPC
diff --git a/chrome/common/ipc_channel_posix.h b/chrome/common/ipc_channel_posix.h
index e788963..cdc0db2 100644
--- a/chrome/common/ipc_channel_posix.h
+++ b/chrome/common/ipc_channel_posix.h
@@ -23,6 +23,9 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
   void Close();
   void set_listener(Listener* listener) { listener_ = listener; }
   bool Send(Message* message);
+  void GetClientFileDescriptorMapping(int *src_fd, int *dest_fd);
+  void OnClientConnected();
+
  private:
   const std::wstring PipeName(const std::wstring& channel_id) const;
   bool CreatePipe(const std::wstring& channel_id, Mode mode);
@@ -48,8 +51,13 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher {
   // to keep track of where we are.
   size_t message_send_bytes_written_;
 
+  // If the kTestingChannelID flag is specified, we use a FIFO instead of
+  // a socketpair().
+  bool uses_fifo_;
+
   int server_listen_pipe_;
   int pipe_;
+  int client_pipe_;  // The client end of our socketpair().
   std::string pipe_name_;
 
   Listener* listener_;
diff --git a/chrome/common/ipc_fuzzing_tests.cc b/chrome/common/ipc_fuzzing_tests.cc
index 0c729f8..cbd2bd3 100644
--- a/chrome/common/ipc_fuzzing_tests.cc
+++ b/chrome/common/ipc_fuzzing_tests.cc
@@ -270,7 +270,7 @@ class FuzzerClientListener : public SimpleListener {
 MULTIPROCESS_TEST_MAIN(RunFuzzServer) {
   MessageLoopForIO main_message_loop;
   FuzzerServerListener listener;
-  IPC::Channel chan(kFuzzerChannel, IPC::Channel::MODE_SERVER, &listener);
+  IPC::Channel chan(kFuzzerChannel, IPC::Channel::MODE_CLIENT, &listener);
   chan.Connect();
   listener.Init(&chan);
   MessageLoop::current()->Run();
@@ -283,12 +283,12 @@ class IPCFuzzingTest : public IPCChannelTest {
 // This test makes sure that the FuzzerClientListener and FuzzerServerListener
 // are working properly by generating two well formed IPC calls.
 TEST_F(IPCFuzzingTest, SanityTest) {
-  base::ProcessHandle server_process = SpawnChild(FUZZER_SERVER);
-  ASSERT_TRUE(server_process);
-  PlatformThread::Sleep(1000);
   FuzzerClientListener listener;
-  IPC::Channel chan(kFuzzerChannel, IPC::Channel::MODE_CLIENT,
+  IPC::Channel chan(kFuzzerChannel, IPC::Channel::MODE_SERVER,
                     &listener);
+  base::ProcessHandle server_process = SpawnChild(FUZZER_SERVER, &chan);
+  ASSERT_TRUE(server_process);
+  PlatformThread::Sleep(1000);
   ASSERT_TRUE(chan.Connect());
   listener.Init(&chan);
 
@@ -312,12 +312,12 @@ TEST_F(IPCFuzzingTest, SanityTest) {
 // properly.
 #ifdef NDEBUG
 TEST_F(IPCFuzzingTest, MsgBadPayloadShort) {
-  base::ProcessHandle server_process = SpawnChild(FUZZER_SERVER);
-  ASSERT_TRUE(server_process);
-  PlatformThread::Sleep(1000);
   FuzzerClientListener listener;
-  IPC::Channel chan(kFuzzerChannel, IPC::Channel::MODE_CLIENT,
+  IPC::Channel chan(kFuzzerChannel, IPC::Channel::MODE_SERVER,
                     &listener);
+  base::ProcessHandle server_process = SpawnChild(FUZZER_SERVER, &chan);
+  ASSERT_TRUE(server_process);
+  PlatformThread::Sleep(1000);
   ASSERT_TRUE(chan.Connect());
   listener.Init(&chan);
 
@@ -341,12 +341,12 @@ TEST_F(IPCFuzzingTest, MsgBadPayloadShort) {
 // This test does not pinpoint a flaw (per se) as by design we don't carry
 // type information on the IPC message.
 TEST_F(IPCFuzzingTest, MsgBadPayloadArgs) {
-  base::ProcessHandle server_process = SpawnChild(FUZZER_SERVER);
-  ASSERT_TRUE(server_process);
-  PlatformThread::Sleep(1000);
   FuzzerClientListener listener;
-  IPC::Channel chan(kFuzzerChannel, IPC::Channel::MODE_CLIENT,
+  IPC::Channel chan(kFuzzerChannel, IPC::Channel::MODE_SERVER,
                     &listener);
+  base::ProcessHandle server_process = SpawnChild(FUZZER_SERVER, &chan);
+  ASSERT_TRUE(server_process);
+  PlatformThread::Sleep(1000);
   ASSERT_TRUE(chan.Connect());
   listener.Init(&chan);
 
diff --git a/chrome/common/ipc_tests.cc b/chrome/common/ipc_tests.cc
index 444824e..971f16b 100644
--- a/chrome/common/ipc_tests.cc
+++ b/chrome/common/ipc_tests.cc
@@ -49,7 +49,9 @@ void IPCChannelTest::TearDown() {
   MultiProcessTest::TearDown();
 }
 
-base::ProcessHandle IPCChannelTest::SpawnChild(ChildType child_type) {
+#if defined(OS_WIN)
+base::ProcessHandle IPCChannelTest::SpawnChild(ChildType child_type,
+                                               IPC::Channel *channel) {
   // kDebugChildren support.
   bool debug_on_start = CommandLine().HasSwitch(switches::kDebugChildren);
 
@@ -68,6 +70,47 @@ base::ProcessHandle IPCChannelTest::SpawnChild(ChildType child_type) {
     break;
   }
 }
+#elif defined(OS_POSIX)
+base::ProcessHandle IPCChannelTest::SpawnChild(ChildType child_type,
+                                               IPC::Channel *channel) {
+  // kDebugChildren support.
+  bool debug_on_start = CommandLine().HasSwitch(switches::kDebugChildren);
+
+  base::file_handle_mapping_vector fds_to_map;
+  int src_fd;
+  int dest_fd;
+  channel->GetClientFileDescriptorMapping(&src_fd, &dest_fd);
+  if (src_fd > -1) {
+    fds_to_map.push_back(std::pair<int,int>(src_fd, dest_fd));
+  }
+
+  base::ProcessHandle ret = NULL;
+  switch (child_type) {
+  case TEST_CLIENT:
+    ret = MultiProcessTest::SpawnChild(L"RunTestClient",
+                                       fds_to_map,
+                                       debug_on_start);
+    channel->OnClientConnected();
+    break;
+  case TEST_REFLECTOR:
+    ret = MultiProcessTest::SpawnChild(L"RunReflector",
+                                       fds_to_map,
+                                       debug_on_start);
+    channel->OnClientConnected();
+    break;
+  case FUZZER_SERVER:
+    ret = MultiProcessTest::SpawnChild(L"RunFuzzServer",
+                                       fds_to_map,
+                                       debug_on_start);
+    channel->OnClientConnected();
+    break;
+  default:
+    return NULL;
+    break;
+  }
+  return ret;
+}
+#endif  // defined(OS_POSIX)
 
 TEST_F(IPCChannelTest, BasicMessageTest) {
   int v1 = 10;
@@ -157,7 +200,7 @@ TEST_F(IPCChannelTest, ChannelTest) {
 
   channel_listener.Init(&chan);
 
-  base::ProcessHandle process_handle = SpawnChild(TEST_CLIENT);
+  base::ProcessHandle process_handle = SpawnChild(TEST_CLIENT, &chan);
   ASSERT_TRUE(process_handle);
 
   Send(&chan, "hello from parent");
@@ -184,7 +227,10 @@ TEST_F(IPCChannelTest, ChannelProxyTest) {
 
     channel_listener.Init(&chan);
 
-    HANDLE process_handle = SpawnChild(TEST_CLIENT);
+    bool debug_on_start = CommandLine().HasSwitch(switches::kDebugChildren);
+    base::ProcessHandle process_handle = MultiProcessTest::SpawnChild(
+        L"RunTestClient",
+        debug_on_start);
     ASSERT_TRUE(process_handle);
 
     Send(&chan, "hello from parent");
@@ -193,8 +239,7 @@ TEST_F(IPCChannelTest, ChannelProxyTest) {
     MessageLoop::current()->Run();
 
     // cleanup child process
-    WaitForSingleObject(process_handle, 5000);
-    CloseHandle(process_handle);
+    EXPECT_TRUE(base::WaitForSingleProcess(process_handle, 5000));
   }
   thread.Stop();
 }
@@ -340,7 +385,7 @@ TEST_F(IPCChannelTest, Performance) {
   chan.set_listener(&perf_listener);
   chan.Connect();
 
-  HANDLE process = SpawnChild(TEST_REFLECTOR);
+  HANDLE process = SpawnChild(TEST_REFLECTOR, &chan);
   ASSERT_TRUE(process);
 
   Sleep(1000);
diff --git a/chrome/common/ipc_tests.h b/chrome/common/ipc_tests.h
index 30c043f..09f3d12 100644
--- a/chrome/common/ipc_tests.h
+++ b/chrome/common/ipc_tests.h
@@ -22,6 +22,9 @@ extern const wchar_t kReflectorChannel[];
 extern const wchar_t kFuzzerChannel[];
 
 class MessageLoopForIO;
+namespace IPC {
+class Channel;
+}  // namespace IPC
 
 //Base class to facilitate Spawning IPC Client processes.
 class IPCChannelTest : public MultiProcessTest {
@@ -32,7 +35,8 @@ class IPCChannelTest : public MultiProcessTest {
   virtual void TearDown();
 
   // Spawns a child process of the specified type
-  base::ProcessHandle SpawnChild(ChildType child_type);
+  base::ProcessHandle SpawnChild(ChildType child_type,
+                                 IPC::Channel *channel);
 
   // Created around each test instantiation.
   MessageLoopForIO *message_loop_;