commit 2c5e7e102bc2059d22f8457db68c567b64cec963
author Dianne Hackborn <hackbod@google.com> Mon Oct 13 17:45:22 2014 -0700
committer Dianne Hackborn <hackbod@google.com> Mon Oct 13 17:52:32 2014 -0700
tree 520bb9860e196dfcb130fc99a4bc7958e68669ab
parent 8218b6aae9cd4a19fa074a8a8203fe9275b35447

Maybe fix issue #17969789: Shamu FR: Runtime restart while scrolling Instagram

It looks like there were a couple problems in the code:

- It would not 0-terminate the string it read, to make sure we didn't
  see garbage at the end.
- It didn't reduce buf_len as it processes data in the buffer, so if
  we need to read more we will increase the buffer length to be longer
  than the actual available data.

Also put in some logs about every thing we kill, so we can see what
is going on when debugging.  And add a special check for us trying
to kill pid 0 for any reason, since doing so seem to be terminal to
the caller.

Change-Id: I2fe29bfef08938b8a2eb182475c0705c14d8d84f

libprocessgroup/Android.mk

diff --git a/libprocessgroup/Android.mk b/libprocessgroup/Android.mk
index 501321f..051999a 100644
--- a/libprocessgroup/Android.mk
+++ b/libprocessgroup/Android.mk
@@ -3,7 +3,7 @@
 include $(CLEAR_VARS)
 LOCAL_SRC_FILES := processgroup.cpp
 LOCAL_MODULE := libprocessgroup
-LOCAL_SHARED_LIBRARIES := liblog
+LOCAL_SHARED_LIBRARIES := liblog libutils
 LOCAL_C_INCLUDES := $(LOCAL_PATH)/include
 LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include
 LOCAL_CFLAGS := -Wall -Werror

libprocessgroup/processgroup.cpp

diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp
index f7bc2cd..d5f3ad3 100644
--- a/libprocessgroup/processgroup.cpp
+++ b/libprocessgroup/processgroup.cpp
@@ -30,6 +30,8 @@
 #include <log/log.h>
 #include <private/android_filesystem_config.h>
 
+#include <utils/SystemClock.h>
+
 #include <processgroup/processgroup.h>
 #include "processgroup_priv.h"
 
@@ -69,7 +71,7 @@
     int fd = open(path, O_RDONLY);
     if (fd < 0) {
         ret = -errno;
-        SLOGV("failed to open %s: %s", path, strerror(errno));
+        SLOGW("failed to open %s: %s", path, strerror(errno));
         return ret;
     }
 
@@ -78,6 +80,8 @@
     ctx->buf_len = 0;
     ctx->initialized = true;
 
+    SLOGV("Initialized context for %s", path);
+
     return 0;
 }
 
@@ -87,7 +91,7 @@
     ctx->buf_ptr = ctx->buf;
 
     ssize_t ret = read(ctx->fd, ctx->buf_ptr + ctx->buf_len,
-                sizeof(ctx->buf) - ctx->buf_len);
+                sizeof(ctx->buf) - ctx->buf_len - 1);
     if (ret < 0) {
         return -errno;
     } else if (ret == 0) {
@@ -95,6 +99,9 @@
     }
 
     ctx->buf_len += ret;
+    ctx->buf[ctx->buf_len-1] = 0;
+    SLOGV("Read %d to buffer: %s", ret, ctx->buf);
+
     assert(ctx->buf_len <= sizeof(ctx->buf));
 
     return ret;
@@ -131,6 +138,7 @@
         return -EINVAL;
     }
 
+    ctx->buf_len -= (eptr - ctx->buf_ptr) + 1;
     ctx->buf_ptr = eptr + 1;
 
     return (pid_t)pid;
@@ -213,10 +221,22 @@
 
     while ((pid = getOneAppProcess(uid, initialPid, &ctx)) >= 0) {
         processes++;
-        SLOGV("sending processgroup kill to pid %d\n", pid);
+        if (pid == 0) {
+            // Should never happen...  but if it does, trying to kill this
+            // will boomerang right back and kill us!  Let's not let that happen.
+            SLOGW("Yikes, we've been told to kill pid 0!  How about we don't do that.");
+            continue;
+        }
+        if (pid != initialPid) {
+            // We want to be noisy about killing processes so we can understand
+            // what is going on in the log; however, don't be noisy about the base
+            // process, since that it something we always kill, and we have already
+            // logged elsewhere about killing it.
+            SLOGI("Killing pid %d in uid %d as part of process group %d", pid, uid, initialPid);
+        }
         int ret = kill(pid, signal);
         if (ret == -1) {
-            SLOGV("failed to kill pid %d: %s", pid, strerror(errno));
+            SLOGW("failed to kill pid %d: %s", pid, strerror(errno));
         }
     }
 
@@ -231,6 +251,7 @@
 {
     int processes;
     int sleep_us = 100;
+    long startTime = android::uptimeMillis();
 
     while ((processes = killProcessGroupOnce(uid, initialPid, signal)) > 0) {
         SLOGV("killed %d processes for processgroup %d\n", processes, initialPid);
@@ -244,6 +265,9 @@
         }
     }
 
+    SLOGV("Killed process group uid %d pid %d in %ldms, %d procs remain", uid, initialPid,
+            android::uptimeMillis()-startTime, processes);
+
     if (processes == 0) {
         return removeProcessGroup(uid, initialPid);
     } else {