Improvements for MachMultiprocess.

This adds a pipe going in the other direction (parent to child).
Initially, I didn’t think this was necessary, but it turned out to be
needed for ProcessReader. Having the child wait on a pipe read is the
easiest way to keep it alive until the parent is done with it.

This also tests MachMultiprocess::ChildPID() in
mach_multiprocess_test.cc.

Both of these fell out of https://codereview.chromium.org/491963002/.

TEST=util_test MachMultiprocess.MachMultiprocess
R=rsesek@chromium.org

Review URL: https://codereview.chromium.org/491363002

util/test/mac/mach_multiprocess.cc

@@ -67,19 +67,25 @@ namespace internal {
 struct MachMultiprocessInfo {
   MachMultiprocessInfo()
       : service_name(),
-        read_pipe(-1),
+        pipe_c2p_read(-1),
-        write_pipe(-1),
+        pipe_c2p_write(-1),
+        pipe_p2c_read(-1),
+        pipe_p2c_write(-1),
         child_pid(0),
-        pipe_fd(-1),
+        read_pipe_fd(-1),
+        write_pipe_fd(-1),
         local_port(MACH_PORT_NULL),
         remote_port(MACH_PORT_NULL),
         child_task(MACH_PORT_NULL) {}
 
   std::string service_name;
-  base::ScopedFD read_pipe;
+  base::ScopedFD pipe_c2p_read;  // child to parent
-  base::ScopedFD write_pipe;
+  base::ScopedFD pipe_c2p_write;  // child to parent
+  base::ScopedFD pipe_p2c_read;  // parent to child
+  base::ScopedFD pipe_p2c_write;  // parent to child
   pid_t child_pid;  // valid only in parent
-  int pipe_fd;  // read_pipe in parent, write_pipe in child
+  int read_pipe_fd;  // pipe_c2p_read in parent, pipe_p2c_read in child
+  int write_pipe_fd;  // pipe_p2c_write in parent, pipe_c2p_write in child
   base::mac::ScopedMachReceiveRight local_port;
   base::mac::ScopedMachSendRight remote_port;
   base::mac::ScopedMachSendRight child_task;  // valid only in parent
@@ -97,12 +103,19 @@ void MachMultiprocess::Run() {
   base::AutoReset<internal::MachMultiprocessInfo*> reset_info(&info_,
                                                               info.get());
 
-  int pipe_fds[2];
+  int pipe_fds_c2p[2];
-  int rv = pipe(pipe_fds);
+  int rv = pipe(pipe_fds_c2p);
   ASSERT_EQ(0, rv) << ErrnoMessage("pipe");
 
-  info_->read_pipe.reset(pipe_fds[0]);
+  info_->pipe_c2p_read.reset(pipe_fds_c2p[0]);
-  info_->write_pipe.reset(pipe_fds[1]);
+  info_->pipe_c2p_write.reset(pipe_fds_c2p[1]);
+
+  int pipe_fds_p2c[2];
+  rv = pipe(pipe_fds_p2c);
+  ASSERT_EQ(0, rv) << ErrnoMessage("pipe");
+
+  info_->pipe_p2c_read.reset(pipe_fds_p2c[0]);
+  info_->pipe_p2c_write.reset(pipe_fds_p2c[1]);
 
   // Set up the parent port and register it with the bootstrap server before
   // forking, so that it’s guaranteed to be there when the child attempts to
@@ -169,9 +182,14 @@ pid_t MachMultiprocess::ChildPID() const {
   return info_->child_pid;
 }
 
-int MachMultiprocess::PipeFD() const {
+int MachMultiprocess::ReadPipeFD() const {
-  EXPECT_NE(-1, info_->pipe_fd);
+  EXPECT_NE(-1, info_->read_pipe_fd);
-  return info_->pipe_fd;
+  return info_->read_pipe_fd;
+}
+
+int MachMultiprocess::WritePipeFD() const {
+  EXPECT_NE(-1, info_->write_pipe_fd);
+  return info_->write_pipe_fd;
 }
 
 mach_port_t MachMultiprocess::LocalPort() const {
@@ -190,9 +208,11 @@ mach_port_t MachMultiprocess::ChildTask() const {
 }
 
 void MachMultiprocess::RunParent() {
-  // The parent uses the read end of the pipe.
+  // The parent uses the read end of c2p and the write end of p2c.
-  info_->write_pipe.reset();
+  info_->pipe_c2p_write.reset();
-  info_->pipe_fd = info_->read_pipe.get();
+  info_->read_pipe_fd = info_->pipe_c2p_read.get();
+  info_->pipe_p2c_read.reset();
+  info_->write_pipe_fd = info_->pipe_p2c_write.get();
 
   ReceiveHelloMessage message = {};
 
@@ -284,8 +304,10 @@ void MachMultiprocess::RunParent() {
   info_->remote_port.reset();
   info_->local_port.reset();
 
-  info_->pipe_fd = -1;
+  info_->read_pipe_fd = -1;
-  info_->read_pipe.reset();
+  info_->pipe_c2p_read.reset();
+  info_->write_pipe_fd = -1;
+  info_->pipe_p2c_write.reset();
 }
 
 void MachMultiprocess::RunChild() {
@@ -294,9 +316,11 @@ void MachMultiprocess::RunChild() {
   // local_port is not valid in the forked child process.
   ignore_result(info_->local_port.release());
 
-  // The child uses the write end of the pipe.
+  // The child uses the write end of c2p and the read end of p2c.
-  info_->read_pipe.reset();
+  info_->pipe_c2p_read.reset();
-  info_->pipe_fd = info_->write_pipe.get();
+  info_->write_pipe_fd = info_->pipe_c2p_write.get();
+  info_->pipe_p2c_write.reset();
+  info_->read_pipe_fd = info_->pipe_p2c_read.get();
 
   mach_port_t local_port;
   kern_return_t kr = mach_port_allocate(
@@ -341,8 +365,10 @@ void MachMultiprocess::RunChild() {
   info_->remote_port.reset();
   info_->local_port.reset();
 
-  info_->pipe_fd = -1;
+  info_->write_pipe_fd = -1;
-  info_->write_pipe.reset();
+  info_->pipe_c2p_write.reset();
+  info_->read_pipe_fd = -1;
+  info_->pipe_p2c_read.reset();
 
   if (Test::HasFailure()) {
     // Trigger the ScopedNotReached destructor.

util/test/mac/mach_multiprocess.h

@@ -31,8 +31,7 @@ struct MachMultiprocessInfo;
 //!
 //! These tests are `fork()`-based. The parent process has access to the child
 //! process’ task port. The parent and child processes are able to communicate
-//! via Mach IPC, and the child process can also send messages to the parent
+//! via Mach IPC, and via a pair of POSIX pipes.
-//! process via a POSIX pipe.
 //!
 //! Subclasses are expected to implement the parent and child by overriding the
 //! appropriate methods.
@@ -79,12 +78,19 @@ class MachMultiprocess {
   //! This method may only be called by the parent process.
   pid_t ChildPID() const;
 
-  //! \brief Returns the pipe’s file descriptor.
+  //! \brief Returns the read pipe’s file descriptor.
   //!
-  //! This method may be called by either the parent or the child process. In
+  //! This method may be called by either the parent or the child process.
-  //! the parent process, the pipe is read-only, and in the child process, it is
+  //! Anything written to the write pipe in the partner process will appear
-  //! write-only.
+  //! on the this file descriptor in this process.
-  int PipeFD() const;
+  int ReadPipeFD() const;
+
+  //! \brief Returns the write pipe’s file descriptor.
+  //!
+  //! This method may be called by either the parent or the child process.
+  //! Anything written to this file descriptor in this process will appear on
+  //! the read pipe in the partner process.
+  int WritePipeFD() const;
 
   //! \brief Returns a receive right for the local port.
   //!

util/test/mac/mach_multiprocess_test.cc

@@ -14,6 +14,8 @@
 
 #include "util/test/mac/mach_multiprocess.h"
 
+#include <unistd.h>
+
 #include "base/basictypes.h"
 #include "gtest/gtest.h"
 #include "util/file/fd_io.h"
@@ -32,27 +34,46 @@ class TestMachMultiprocess final : public MachMultiprocess {
 
  protected:
   // The base class will have already exercised the Mach ports for IPC and the
-  // child task port. Just make sure that the pipe is set up correctly.
+  // child task port. Just make sure that the pipe is set up correctly and that
+  // ChildPID() works as expected.
   virtual void Parent() override {
-    int fd = PipeFD();
+    int read_fd = ReadPipeFD();
-
     char c;
-    ssize_t rv = ReadFD(fd, &c, 1);
+    ssize_t rv = ReadFD(read_fd, &c, 1);
     ASSERT_EQ(1, rv) << ErrnoMessage("read");
     EXPECT_EQ('M', c);
 
+    pid_t pid;
+    rv = ReadFD(read_fd, &pid, sizeof(pid));
+    ASSERT_EQ(static_cast<ssize_t>(sizeof(pid)), rv) << ErrnoMessage("read");
+    EXPECT_EQ(pid, ChildPID());
+
+    int write_fd = WritePipeFD();
+    c = 'm';
+    rv = WriteFD(write_fd, &c, 1);
+    ASSERT_EQ(1, rv) << ErrnoMessage("write");
+
     // The child will close its end of the pipe and exit. Make sure that the
     // parent sees EOF.
-    rv = ReadFD(fd, &c, 1);
+    rv = ReadFD(read_fd, &c, 1);
     ASSERT_EQ(0, rv) << ErrnoMessage("read");
   }
 
   virtual void Child() override {
-    int fd = PipeFD();
+    int write_fd = WritePipeFD();
 
     char c = 'M';
-    ssize_t rv = WriteFD(fd, &c, 1);
+    ssize_t rv = WriteFD(write_fd, &c, 1);
     ASSERT_EQ(1, rv) << ErrnoMessage("write");
+
+    pid_t pid = getpid();
+    rv = WriteFD(write_fd, &pid, sizeof(pid));
+    ASSERT_EQ(static_cast<ssize_t>(sizeof(pid)), rv) << ErrnoMessage("write");
+
+    int read_fd = ReadPipeFD();
+    rv = ReadFD(read_fd, &c, 1);
+    ASSERT_EQ(1, rv) << ErrnoMessage("read");
+    EXPECT_EQ('m', c);
   }
 
  private: