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
This commit is contained in:
Dianne Hackborn 2014-10-13 17:45:22 -07:00
parent 8218b6aae9
commit 2c5e7e102b
2 changed files with 29 additions and 5 deletions

View file

@ -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

View file

@ -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 {