From 2c5e7e102bc2059d22f8457db68c567b64cec963 Mon Sep 17 00:00:00 2001
From: Dianne Hackborn <hackbod@google.com>
Date: Mon, 13 Oct 2014 17:45:22 -0700
Subject: [PATCH] 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       |  2 +-
 libprocessgroup/processgroup.cpp | 32 ++++++++++++++++++++++++++++----
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/libprocessgroup/Android.mk b/libprocessgroup/Android.mk
index 501321f30..051999adc 100644
--- a/libprocessgroup/Android.mk
+++ b/libprocessgroup/Android.mk
@@ -3,7 +3,7 @@ LOCAL_PATH := $(call my-dir)
 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
diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp
index f7bc2cd7d..d5f3ad3c2 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 @@ static int initCtx(uid_t uid, int pid, struct ctx *ctx)
     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 @@ static int initCtx(uid_t uid, int pid, struct ctx *ctx)
     ctx->buf_len = 0;
     ctx->initialized = true;
 
+    SLOGV("Initialized context for %s", path);
+
     return 0;
 }
 
@@ -87,7 +91,7 @@ static int refillBuffer(struct ctx *ctx)
     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 @@ static int refillBuffer(struct ctx *ctx)
     }
 
     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 @@ static pid_t getOneAppProcess(uid_t uid, int appProcessPid, struct ctx *ctx)
         return -EINVAL;
     }
 
+    ctx->buf_len -= (eptr - ctx->buf_ptr) + 1;
     ctx->buf_ptr = eptr + 1;
 
     return (pid_t)pid;
@@ -213,10 +221,22 @@ static int killProcessGroupOnce(uid_t uid, int initialPid, int signal)
 
     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 killProcessGroup(uid_t uid, int initialPid, int signal)
 {
     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 @@ int killProcessGroup(uid_t uid, int initialPid, int signal)
         }
     }
 
+    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 {