commit 4d4a23306a0ae1d5cb9ff2bd796bc0f2716547e0
author Kweku Adams <kwekua@google.com> Thu Sep 13 15:44:58 2018 -0700
committer Kweku Adams <kwekua@google.com> Tue Sep 18 18:55:19 2018 -0700
tree d430444ab654896f805db8be1dfdbd509421c948
parent 6d66e03371e02f6cddc02b2377dacd3bfac604ea

Handling potential SIGPIPE signals.

There are cases where dumping Java traces times out and then later tries
to write to a closed pipe, causing incidentd to crash. This introduces a
handler for the SIGPIPE signal so it doesn't crash due to a closed pipe.

Bug: 111729299
Test: lower section timeout to 5 seconds and make sure incident service
doesn't crash

Change-Id: I868aba1ae21a50ae76977e0e09e782cf94198473
(cherry picked from commit 5b763c13472effa7df25cc53a44d5bdae9392410)

cmds/incidentd/src/Section.cpp

diff --git a/cmds/incidentd/src/Section.cpp b/cmds/incidentd/src/Section.cpp
index 4bbe042..72a4103 100644
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -425,7 +425,19 @@
 
 WorkerThreadSection::~WorkerThreadSection() {}
 
+void sigpipe_handler(int signum) {
+    if (signum == SIGPIPE) {
+        ALOGE("Wrote to a broken pipe\n");
+    } else {
+        ALOGE("Received unexpected signal: %d\n", signum);
+    }
+}
+
 static void* worker_thread_func(void* cookie) {
+    // Don't crash the service if we write to a closed pipe (which can happen if
+    // dumping times out).
+    signal(SIGPIPE, sigpipe_handler);
+
     WorkerThreadData* data = (WorkerThreadData*)cookie;
     status_t err = data->section->BlockingCall(data->pipe.writeFd().get());
 
@@ -506,6 +518,7 @@
             }
         }
     }
+
     write_section_stats(requests->sectionStats(this->id), buffer);
     if (timedOut || buffer.timedOut()) {
         ALOGW("WorkerThreadSection '%s' timed out", this->name.string());
@@ -813,7 +826,10 @@
         }
     }
     gLastLogsRetrieved[mLogID] = lastTimestamp;
-    proto.flush(pipeWriteFd);
+    if (!proto.flush(pipeWriteFd) && errno == EPIPE) {
+        ALOGE("[%s] wrote to a broken pipe\n", this->name.string());
+        return EPIPE;
+    }
     return NO_ERROR;
 }
 
@@ -915,7 +931,7 @@
             break;
         }
         if (cStatus != NO_ERROR) {
-            ALOGE("TombstoneSection '%s' child had an issue: %s\n", this->name.string(), strerror(-cStatus));
+            ALOGE("[%s] child had an issue: %s\n", this->name.string(), strerror(-cStatus));
         }
 
         auto dump = std::make_unique<char[]>(buffer.size());
@@ -934,7 +950,13 @@
         dumpPipe.readFd().reset();
     }
 
-    proto.flush(pipeWriteFd);
+    if (!proto.flush(pipeWriteFd) && errno == EPIPE) {
+        ALOGE("[%s] wrote to a broken pipe\n", this->name.string());
+        if (err != NO_ERROR) {
+            return EPIPE;
+        }
+    }
+
     return err;
 }