Mojo: Abstract out write/writev vs send/sendmsg (for Unix domain sockets).

I thought I had fixed our SIGPIPE suppression, but I was wrong (missed the
write() case). So I guess I should do the right thing.

R=darin@chromium.org
BUG=356195

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

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

6 files changed, 122 insertions, 29 deletions

diff --git a/mojo/embedder/platform_channel_pair.h b/mojo/embedder/platform_channel_pair.h
index f0629d6..126bd3d 100644
--- a/mojo/embedder/platform_channel_pair.h
+++ b/mojo/embedder/platform_channel_pair.h
@@ -45,6 +45,11 @@ typedef base::FileHandleMappingVector HandlePassingInformation;
 // Note: |PlatformChannelPair()|, |PassClientHandleFromParentProcess()| and
 // |PrepareToPassClientHandleToChildProcess()| have platform-specific
 // implementations.
+//
+// Note: On POSIX platforms, to write to the "pipe", use
+// |PlatformChannel{Write,Writev}()| (from platform_channel_utils_posix.h)
+// instead of |write()|, |writev()|, etc. Otherwise, you have to worry about
+// platform differences in suppressing |SIGPIPE|.
 class MOJO_SYSTEM_IMPL_EXPORT PlatformChannelPair {
  public:
   PlatformChannelPair();
diff --git a/mojo/embedder/platform_channel_pair_posix_unittest.cc b/mojo/embedder/platform_channel_pair_posix_unittest.cc
index e03c053..7b09e31 100644
--- a/mojo/embedder/platform_channel_pair_posix_unittest.cc
+++ b/mojo/embedder/platform_channel_pair_posix_unittest.cc
@@ -8,11 +8,12 @@
 #include <signal.h>
 #include <sys/socket.h>
 #include <sys/types.h>
+#include <sys/uio.h>
 #include <unistd.h>
 
 #include "base/logging.h"
 #include "base/macros.h"
-#include "build/build_config.h"
+#include "mojo/embedder/platform_channel_utils_posix.h"
 #include "mojo/embedder/scoped_platform_handle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -70,15 +71,18 @@ TEST_F(PlatformChannelPairPosixTest, NoSigPipe) {
   if (result == -1)
     PLOG(WARNING) << "read (expected 0 for EOF)";
 
-  // However, |write()|/|send()| should fail outright.
-  // On Mac, |SIGPIPE| needs to be suppressed on the socket itself and we can
-  // use |write()|/|writev()|. On Linux, we have to suppress it by using
-  // |send()|/|sendmsg()| with |MSG_NOSIGNAL|.
-#if defined(OS_MACOSX)
-  result = write(server_handle.get().fd, kHello, sizeof(kHello));
-#else
-  result = send(server_handle.get().fd, kHello, sizeof(kHello), MSG_NOSIGNAL);
-#endif
+  // Test our replacement for |write()|/|send()|.
+  result = PlatformChannelWrite(server_handle.get(), kHello, sizeof(kHello));
+  EXPECT_EQ(-1, result);
+  if (errno != EPIPE)
+    PLOG(WARNING) << "write (expected EPIPE)";
+
+  // Test our replacement for |writev()|/|sendv()|.
+  struct iovec iov[2] = {
+    { const_cast<char*>(kHello), sizeof(kHello) },
+    { const_cast<char*>(kHello), sizeof(kHello) }
+  };
+  result = PlatformChannelWritev(server_handle.get(), iov, 2);
   EXPECT_EQ(-1, result);
   if (errno != EPIPE)
     PLOG(WARNING) << "write (expected EPIPE)";
diff --git a/mojo/embedder/platform_channel_utils_posix.cc b/mojo/embedder/platform_channel_utils_posix.cc
new file mode 100644
index 0000000..f0e0538
--- /dev/null
+++ b/mojo/embedder/platform_channel_utils_posix.cc
@@ -0,0 +1,63 @@
+// Copyright 2014 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 "mojo/embedder/platform_channel_utils_posix.h"
+
+#include <sys/socket.h>
+#include <sys/uio.h>
+#include <unistd.h>
+
+#include "base/posix/eintr_wrapper.h"
+#include "build/build_config.h"
+
+namespace mojo {
+namespace embedder {
+
+// On Linux, |SIGPIPE| is suppressed by passing |MSG_NOSIGNAL| to
+// |send()|/|sendmsg()|. (There is no way of suppressing |SIGPIPE| on
+// |write()|/|writev().) On Mac, |SIGPIPE| is suppressed by setting the
+// |SO_NOSIGPIPE| option on the socket.
+//
+// Performance notes:
+//  - On Linux, we have to use |send()|/|sendmsg()| rather than
+//    |write()|/|writev()| in order to suppress |SIGPIPE|. This is okay, since
+//    |send()| is (slightly) faster than |write()| (!), while |sendmsg()| is
+//    quite comparable to |writev()|.
+//  - On Mac, we may use |write()|/|writev()|. Here, |write()| is considerably
+//    faster than |send()|, whereas |sendmsg()| is quite comparable to
+//    |writev()|.
+//  - On both platforms, an appropriate |sendmsg()|/|writev()| is considerably
+//    faster than two |send()|s/|write()|s.
+//  - Relative numbers (minimum real times from 10 runs) for one |write()| of
+//    1032 bytes, one |send()| of 1032 bytes, one |writev()| of 32+1000 bytes,
+//    one |sendmsg()| of 32+1000 bytes, two |write()|s of 32 and 1000 bytes, two
+//    |send()|s of 32 and 1000 bytes:
+//    - Linux: 0.81 s, 0.77 s, 0.87 s, 0.89 s, 1.31 s, 1.22 s
+//    - Mac: 2.21 s, 2.91 s, 2.98 s, 3.08 s, 3.59 s, 4.74 s
+
+ssize_t PlatformChannelWrite(PlatformHandle h,
+                             const void* bytes,
+                             size_t num_bytes) {
+#if defined(OS_MACOSX)
+  return HANDLE_EINTR(write(h.fd, bytes, num_bytes));
+#else
+  return send(h.fd, bytes, num_bytes, MSG_NOSIGNAL);
+#endif
+}
+
+ssize_t PlatformChannelWritev(PlatformHandle h,
+                              struct iovec* iov,
+                              size_t num_iov) {
+#if defined(OS_MACOSX)
+  return HANDLE_EINTR(writev(h.fd, iov, static_cast<int>(num_iov)));
+#else
+  struct msghdr msg = {};
+  msg.msg_iov = iov;
+  msg.msg_iovlen = num_iov;
+  return HANDLE_EINTR(sendmsg(h.fd, &msg, MSG_NOSIGNAL));
+#endif
+}
+
+}  // namespace embedder
+}  // namespace mojo
diff --git a/mojo/embedder/platform_channel_utils_posix.h b/mojo/embedder/platform_channel_utils_posix.h
new file mode 100644
index 0000000..7ec5fa5
--- /dev/null
+++ b/mojo/embedder/platform_channel_utils_posix.h
@@ -0,0 +1,33 @@
+// Copyright 2014 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.
+
+#ifndef MOJO_EMBEDDER_PLATFORM_CHANNEL_UTILS_POSIX_H_
+#define MOJO_EMBEDDER_PLATFORM_CHANNEL_UTILS_POSIX_H_
+
+#include <stddef.h>
+#include <sys/types.h>  // For |ssize_t|.
+
+#include "mojo/embedder/platform_handle.h"
+#include "mojo/system/system_impl_export.h"
+
+struct iovec;  // Declared in <sys/uio.h>.
+
+namespace mojo {
+namespace embedder {
+
+// Use these to write to a socket created using |PlatformChannelPair| (or
+// equivalent). These are like |write()| and |writev()|, but handle |EINTR| and
+// never raise |SIGPIPE|. (Note: On Mac, the suppression of |SIGPIPE| is set up
+// by |PlatformChannelPair|.)
+MOJO_SYSTEM_IMPL_EXPORT ssize_t PlatformChannelWrite(PlatformHandle h,
+                                                     const void* bytes,
+                                                     size_t num_bytes);
+MOJO_SYSTEM_IMPL_EXPORT ssize_t PlatformChannelWritev(PlatformHandle h,
+                                                      struct iovec* iov,
+                                                      size_t num_iov);
+
+}  // namespace embedder
+}  // namespace mojo
+
+#endif  // MOJO_EMBEDDER_PLATFORM_CHANNEL_UTILS_POSIX_H_
diff --git a/mojo/mojo.gyp b/mojo/mojo.gyp
index be72709..c3efca8 100644
--- a/mojo/mojo.gyp
+++ b/mojo/mojo.gyp
@@ -111,6 +111,8 @@
         'embedder/platform_channel_pair.h',
         'embedder/platform_channel_pair_posix.cc',
         'embedder/platform_channel_pair_win.cc',
+        'embedder/platform_channel_utils_posix.cc',
+        'embedder/platform_channel_utils_posix.h',
         'embedder/platform_handle.cc',
         'embedder/platform_handle.h',
         'embedder/scoped_platform_handle.h',
diff --git a/mojo/system/raw_channel_posix.cc b/mojo/system/raw_channel_posix.cc
index 754b4a4..2b1a358 100644
--- a/mojo/system/raw_channel_posix.cc
+++ b/mojo/system/raw_channel_posix.cc
@@ -5,8 +5,6 @@
 #include "mojo/system/raw_channel.h"
 
 #include <errno.h>
-#include <sys/socket.h>
-#include <sys/types.h>
 #include <sys/uio.h>
 #include <unistd.h>
 
@@ -22,7 +20,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/synchronization/lock.h"
-#include "build/build_config.h"
+#include "mojo/embedder/platform_channel_utils_posix.h"
 #include "mojo/embedder/platform_handle.h"
 
 namespace mojo {
@@ -145,8 +143,8 @@ RawChannel::IOResult RawChannelPosix::WriteNoLock(size_t* bytes_written) {
 
   ssize_t write_result = -1;
   if (buffers.size() == 1) {
-    write_result = HANDLE_EINTR(
-        write(fd_.get().fd, buffers[0].addr, buffers[0].size));
+    write_result = embedder::PlatformChannelWrite(fd_.get(), buffers[0].addr,
+                                                  buffers[0].size);
   } else {
     // Note that using |writev()|/|sendmsg()| is measurably slower than using
     // |write()| -- at least in a microbenchmark -- but much faster than using
@@ -166,20 +164,8 @@ RawChannel::IOResult RawChannelPosix::WriteNoLock(size_t* bytes_written) {
       iov[i].iov_len = buffers[i].size;
     }
 
-    // On Mac, we can use |writev()|, which is slightly faster, but on Linux we
-    // need to use |sendmsg()|. See comment above.
-    // TODO(vtl): We should have an actual test that |SIGPIPE| is suppressed for
-    // |RawChannelPosix|, since it has to be suppressed at "use" time on Linux.
-    // Or maybe I should abstract out |write()|/|send()| and
-    // |writev()|/|sendmsg()|. crbug.com/356195
-#if defined(OS_MACOSX)
-    write_result = HANDLE_EINTR(writev(fd_.get().fd, iov, buffer_count));
-#else
-    struct msghdr msg = {};
-    msg.msg_iov = iov;
-    msg.msg_iovlen = buffer_count;
-    write_result = HANDLE_EINTR(sendmsg(fd_.get().fd, &msg, MSG_NOSIGNAL));
-#endif
+    write_result = embedder::PlatformChannelWritev(fd_.get(), iov,
+                                                   buffer_count);
   }
 
   if (write_result >= 0) {