Bug 1582297 - Suppress IPC "pipe error" messages if the cause was probably the other process exiting. r=froydnj
authorJed Davis <jld@mozilla.com>
Tue, 12 Nov 2019 21:04:40 +0000
changeset 502002 512d489d83e128358c960823d75b22bd59cb7163
parent 502001 d395b70f3ff8af258f4ccb2e78cef02c27cf9ff4
child 502003 9ba180fee075f942198a5e6cc9c40a48e0b1072c
push id114172
push userdluca@mozilla.com
push dateTue, 19 Nov 2019 11:31:10 +0000
treeherdermozilla-inbound@b5c5ba07d3db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1582297
milestone72.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1582297 - Suppress IPC "pipe error" messages if the cause was probably the other process exiting. r=froydnj There are two issues here: 1. These error messages occur even during normal channel shutdown, because that's tracked in the mozilla::ipc::MessageChannel layer, which the ipc/chromium code can't access. 2. If we get this kind of error when the channel wasn't intentionally closed, it almost certainly means that the other process crashed. In that case, having error messages from a different process and a likely unrelated subsystem just leads to confusion and misfiled bugs. (Also complicating things: on Unix a closed channel often, but not always, results in an end-of-file indication, which already isn't logged; on Windows it's always a broken pipe error, which causes a much larger amount of log spam.) Bonus fix: the error that contains a fd number is clarified to avoid having it mistaken for an error code. Differential Revision: https://phabricator.services.mozilla.com/D52727
ipc/chromium/src/chrome/common/ipc_channel_posix.cc
ipc/chromium/src/chrome/common/ipc_channel_win.cc
--- a/ipc/chromium/src/chrome/common/ipc_channel_posix.cc
+++ b/ipc/chromium/src/chrome/common/ipc_channel_posix.cc
@@ -189,16 +189,18 @@ bool SetCloseOnExec(int fd) {
   if (flags == -1) return false;
 
   flags |= FD_CLOEXEC;
   if (fcntl(fd, F_SETFD, flags) == -1) return false;
 
   return true;
 }
 
+bool ErrorIsBrokenPipe(int err) { return err == EPIPE || err == ECONNRESET; }
+
 }  // namespace
 //------------------------------------------------------------------------------
 
 #if defined(MOZ_WIDGET_ANDROID)
 void Channel::SetClientChannelFd(int fd) { gClientChannelFd = fd; }
 #endif  // defined(MOZ_WIDGET_ANDROID)
 
 Channel::ChannelImpl::ChannelImpl(const std::wstring& channel_id, Mode mode,
@@ -350,18 +352,20 @@ bool Channel::ChannelImpl::ProcessIncomi
     // recvmsg() returns 0 if the connection has closed or EAGAIN if no data
     // is waiting on the pipe.
     ssize_t bytes_read = HANDLE_EINTR(recvmsg(pipe_, &msg, MSG_DONTWAIT));
 
     if (bytes_read < 0) {
       if (errno == EAGAIN) {
         return true;
       } else {
-        CHROMIUM_LOG(ERROR)
-            << "pipe error (" << pipe_ << "): " << strerror(errno);
+        if (!ErrorIsBrokenPipe(errno)) {
+          CHROMIUM_LOG(ERROR)
+              << "pipe error (fd " << pipe_ << "): " << strerror(errno);
+        }
         return false;
       }
     } else if (bytes_read == 0) {
       // The pipe has closed...
       Close();
       return false;
     }
     DCHECK(bytes_read);
@@ -720,17 +724,19 @@ bool Channel::ChannelImpl::ProcessOutgoi
           // FD over atomically.
         case EMSGSIZE:
           // Because this is likely to result in a busy-wait, we'll try to make
           // it easier for the receiver to make progress.
           sched_yield();
           break;
 #endif
         default:
-          CHROMIUM_LOG(ERROR) << "pipe error: " << strerror(errno);
+          if (!ErrorIsBrokenPipe(errno)) {
+            CHROMIUM_LOG(ERROR) << "pipe error: " << strerror(errno);
+          }
           return false;
       }
     }
 
     if (static_cast<size_t>(bytes_written) != amt_to_write) {
       // If write() fails with EAGAIN then bytes_written will be -1.
       if (bytes_written > 0) {
         partial_write_iter_.ref().AdvanceAcrossSegments(msg->Buffers(),
--- a/ipc/chromium/src/chrome/common/ipc_channel_win.cc
+++ b/ipc/chromium/src/chrome/common/ipc_channel_win.cc
@@ -333,17 +333,19 @@ bool Channel::ChannelImpl::ProcessIncomi
                          Channel::kReadBufferSize - input_buf_offset_,
                          &bytes_read, &input_state_.context.overlapped);
       if (!ok) {
         DWORD err = GetLastError();
         if (err == ERROR_IO_PENDING) {
           input_state_.is_pending = true;
           return true;
         }
-        CHROMIUM_LOG(ERROR) << "pipe error: " << err;
+        if (err != ERROR_BROKEN_PIPE) {
+          CHROMIUM_LOG(ERROR) << "pipe error: " << err;
+        }
         return false;
       }
       input_state_.is_pending = true;
       return true;
     }
     DCHECK(bytes_read);
 
     // Process messages from input buffer.
@@ -450,17 +452,19 @@ bool Channel::ChannelImpl::ProcessOutgoi
                               // no connection?
   ASSERT_OWNINGTHREAD(ChannelImpl);
 
   if (output_state_.is_pending) {
     DCHECK(context);
     output_state_.is_pending = false;
     if (!context || bytes_written == 0) {
       DWORD err = GetLastError();
-      CHROMIUM_LOG(ERROR) << "pipe error: " << err;
+      if (err != ERROR_BROKEN_PIPE) {
+        CHROMIUM_LOG(ERROR) << "pipe error: " << err;
+      }
       return false;
     }
     // Message was sent.
     DCHECK(!output_queue_.empty());
     Message* m = output_queue_.front();
 
     MOZ_RELEASE_ASSERT(partial_write_iter_.isSome());
     Pickle::BufferList::IterImpl& iter = partial_write_iter_.ref();
@@ -494,17 +498,19 @@ bool Channel::ChannelImpl::ProcessOutgoi
 
 #ifdef IPC_MESSAGE_DEBUG_EXTRA
       DLOG(INFO) << "sent pending message @" << m << " on channel @" << this
                  << " with type " << m->type();
 #endif
 
       return true;
     }
-    CHROMIUM_LOG(ERROR) << "pipe error: " << err;
+    if (err != ERROR_BROKEN_PIPE) {
+      CHROMIUM_LOG(ERROR) << "pipe error: " << err;
+    }
     return false;
   }
 
 #ifdef IPC_MESSAGE_DEBUG_EXTRA
   DLOG(INFO) << "sent message @" << m << " on channel @" << this
              << " with type " << m->type();
 #endif