diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index dbc77d81c2514..9afda8c0fba4f 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -12127,6 +12127,7 @@ public class ActivityManagerService extends IActivityManager.Stub app.setHasClientActivities(false); mServices.killServicesLocked(app, allowRestart); + mPhantomProcessList.onAppDied(app.pid); boolean restart = false; diff --git a/services/core/java/com/android/server/am/PhantomProcessList.java b/services/core/java/com/android/server/am/PhantomProcessList.java index e2fcf08ce82b7..5167c57199551 100644 --- a/services/core/java/com/android/server/am/PhantomProcessList.java +++ b/services/core/java/com/android/server/am/PhantomProcessList.java @@ -27,15 +27,22 @@ import android.app.ApplicationExitInfo.Reason; import android.app.ApplicationExitInfo.SubReason; import android.os.Handler; import android.os.Process; +import android.os.StrictMode; import android.util.Slog; import android.util.SparseArray; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.os.ProcStatsUtil; import com.android.internal.os.ProcessCpuTracker; import libcore.io.IoUtils; import java.io.FileDescriptor; +import java.io.FileInputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; import java.io.PrintWriter; import java.util.ArrayList; import java.util.Collections; @@ -78,18 +85,178 @@ public final class PhantomProcessList { @GuardedBy("mLock") private final ArrayList mTempPhantomProcesses = new ArrayList<>(); + /** + * The mapping between a phantom process ID to its parent process (an app process) + */ + @GuardedBy("mLock") + private final SparseArray mPhantomToAppProcessMap = new SparseArray<>(); + + @GuardedBy("mLock") + private final SparseArray mCgroupProcsFds = new SparseArray<>(); + + @GuardedBy("mLock") + private final byte[] mDataBuffer = new byte[4096]; + @GuardedBy("mLock") private boolean mTrimPhantomProcessScheduled = false; @GuardedBy("mLock") int mUpdateSeq; + @VisibleForTesting + Injector mInjector; + private final ActivityManagerService mService; private final Handler mKillHandler; PhantomProcessList(final ActivityManagerService service) { mService = service; mKillHandler = service.mProcessList.sKillHandler; + mInjector = new Injector(); + } + + @VisibleForTesting + @GuardedBy("mLock") + void lookForPhantomProcessesLocked() { + mPhantomToAppProcessMap.clear(); + StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskReads(); + try { + synchronized (mService.mPidsSelfLocked) { + for (int i = mService.mPidsSelfLocked.size() - 1; i >= 0; i--) { + final ProcessRecord app = mService.mPidsSelfLocked.valueAt(i); + lookForPhantomProcessesLocked(app); + } + } + } finally { + StrictMode.setThreadPolicy(oldPolicy); + } + } + + @GuardedBy({"mLock", "mService.mPidsSelfLocked"}) + private void lookForPhantomProcessesLocked(ProcessRecord app) { + if (app.appZygote || app.killed || app.killedByAm) { + // process forked from app zygote doesn't have its own acct entry + return; + } + InputStream input = mCgroupProcsFds.get(app.pid); + if (input == null) { + final String path = getCgroupFilePath(app.info.uid, app.pid); + try { + input = mInjector.openCgroupProcs(path); + } catch (FileNotFoundException | SecurityException e) { + if (DEBUG_PROCESSES) { + Slog.w(TAG, "Unable to open " + path, e); + } + return; + } + // Keep the FD open for better performance + mCgroupProcsFds.put(app.pid, input); + } + final byte[] buf = mDataBuffer; + try { + int read = 0; + int pid = 0; + long totalRead = 0; + do { + read = mInjector.readCgroupProcs(input, buf, 0, buf.length); + if (read == -1) { + break; + } + totalRead += read; + for (int i = 0; i < read; i++) { + final byte b = buf[i]; + if (b == '\n') { + addChildPidLocked(app, pid); + pid = 0; + } else { + pid = pid * 10 + (b - '0'); + } + } + if (read < buf.length) { + // we may break from here safely as sysfs reading should return the whole page + // if the remaining data is larger than a page + break; + } + } while (true); + if (pid != 0) { + addChildPidLocked(app, pid); + } + // rewind the fd for the next read + input.skip(-totalRead); + } catch (IOException e) { + Slog.e(TAG, "Error in reading cgroup procs from " + app, e); + IoUtils.closeQuietly(input); + mCgroupProcsFds.delete(app.pid); + } + } + + @VisibleForTesting + static String getCgroupFilePath(int uid, int pid) { + return "/acct/uid_" + uid + "/pid_" + pid + "/cgroup.procs"; + } + + static String getProcessName(int pid) { + String procName = ProcStatsUtil.readTerminatedProcFile( + "/proc/" + pid + "/cmdline", (byte) '\0'); + if (procName == null) { + return null; + } + int l = procName.lastIndexOf('/'); + if (l > 0 && l < procName.length() - 1) { + procName = procName.substring(l + 1); + } + return procName; + } + + @GuardedBy({"mLock", "mService.mPidsSelfLocked"}) + private void addChildPidLocked(final ProcessRecord app, final int pid) { + if (app.pid != pid) { + // That's something else... + final ProcessRecord r = mService.mPidsSelfLocked.get(pid); + if (r != null) { + // Is this a process forked via app zygote? + if (!r.appZygote) { + // Unexpected... + if (DEBUG_PROCESSES) { + Slog.w(TAG, "Unexpected: " + r + " appears in the cgroup.procs of " + app); + } + } else { + // Just a child process of app zygote, no worries + } + } else { + final int index = mPhantomToAppProcessMap.indexOfKey(pid); + if (index >= 0) { // unlikely since we cleared the map at the beginning + final ProcessRecord current = mPhantomToAppProcessMap.valueAt(index); + if (app == current) { + // Okay it's unchanged + return; + } + mPhantomToAppProcessMap.setValueAt(index, app); + } else { + mPhantomToAppProcessMap.put(pid, app); + } + // Its UID isn't necessarily to be the same as the app.info.uid, since it could be + // forked from child processes of app zygote + final int uid = Process.getUidForPid(pid); + String procName = mInjector.getProcessName(pid); + if (procName == null || uid < 0) { + mPhantomToAppProcessMap.delete(pid); + return; + } + getOrCreatePhantomProcessIfNeededLocked(procName, uid, pid, true); + } + } + } + + void onAppDied(final int pid) { + synchronized (mLock) { + final int index = mCgroupProcsFds.indexOfKey(pid); + if (index >= 0) { + final InputStream inputStream = mCgroupProcsFds.valueAt(index); + mCgroupProcsFds.removeAt(index); + IoUtils.closeQuietly(inputStream); + } + } } /** @@ -99,7 +266,7 @@ public final class PhantomProcessList { */ @GuardedBy("mLock") PhantomProcessRecord getOrCreatePhantomProcessIfNeededLocked(final String processName, - final int uid, final int pid) { + final int uid, final int pid, boolean createIfNeeded) { // First check if it's actually an app process we know if (isAppProcess(pid)) { return null; @@ -123,56 +290,47 @@ public final class PhantomProcessList { if (proc.equals(processName, uid, pid)) { return proc; } - // Our zombie process information is outdated, let's remove this one, it shoud + // Our zombie process information is outdated, let's remove this one, it should // have been gone. mZombiePhantomProcesses.removeAt(idx); } } - int ppid = getParentPid(pid); + if (!createIfNeeded) { + return null; + } - // Walk through its parents and see if it could be traced back to an app process. - while (ppid > 1) { - if (isAppProcess(ppid)) { - // It's a phantom process, bookkeep it - try { - final PhantomProcessRecord proc = new PhantomProcessRecord( - processName, uid, pid, ppid, mService, - this::onPhantomProcessKilledLocked); - proc.mUpdateSeq = mUpdateSeq; - mPhantomProcesses.put(pid, proc); - SparseArray array = mAppPhantomProcessMap.get(ppid); - if (array == null) { - array = new SparseArray<>(); - mAppPhantomProcessMap.put(ppid, array); - } - array.put(pid, proc); - if (proc.mPidFd != null) { - mKillHandler.getLooper().getQueue().addOnFileDescriptorEventListener( - proc.mPidFd, EVENT_INPUT | EVENT_ERROR, - this::onPhantomProcessFdEvent); - mPhantomProcessesPidFds.put(proc.mPidFd.getInt$(), proc); - } - scheduleTrimPhantomProcessesLocked(); - return proc; - } catch (IllegalStateException e) { - return null; + final ProcessRecord r = mPhantomToAppProcessMap.get(pid); + + if (r != null) { + // It's a phantom process, bookkeep it + try { + final PhantomProcessRecord proc = new PhantomProcessRecord( + processName, uid, pid, r.pid, mService, + this::onPhantomProcessKilledLocked); + proc.mUpdateSeq = mUpdateSeq; + mPhantomProcesses.put(pid, proc); + SparseArray array = mAppPhantomProcessMap.get(r.pid); + if (array == null) { + array = new SparseArray<>(); + mAppPhantomProcessMap.put(r.pid, array); } + array.put(pid, proc); + if (proc.mPidFd != null) { + mKillHandler.getLooper().getQueue().addOnFileDescriptorEventListener( + proc.mPidFd, EVENT_INPUT | EVENT_ERROR, + this::onPhantomProcessFdEvent); + mPhantomProcessesPidFds.put(proc.mPidFd.getInt$(), proc); + } + scheduleTrimPhantomProcessesLocked(); + return proc; + } catch (IllegalStateException e) { + return null; } - - ppid = getParentPid(ppid); } return null; } - private static int getParentPid(int pid) { - try { - return Process.getParentPid(pid); - } catch (Exception e) { - } - return -1; - } - private boolean isAppProcess(int pid) { synchronized (mService.mPidsSelfLocked) { return mService.mPidsSelfLocked.get(pid) != null; @@ -346,10 +504,14 @@ public final class PhantomProcessList { synchronized (mLock) { // refresh the phantom process list with the latest cpu stats results. mUpdateSeq++; + + // Scan app process's accounting procs + lookForPhantomProcessesLocked(); + for (int i = tracker.countStats() - 1; i >= 0; i--) { final ProcessCpuTracker.Stats st = tracker.getStats(i); final PhantomProcessRecord r = - getOrCreatePhantomProcessIfNeededLocked(st.name, st.uid, st.pid); + getOrCreatePhantomProcessIfNeededLocked(st.name, st.uid, st.pid, false); if (r != null) { r.mUpdateSeq = mUpdateSeq; r.mCurrentCputime += st.rel_utime + st.rel_stime; @@ -392,4 +554,19 @@ public final class PhantomProcessList { proc.dump(pw, prefix + " "); } } + + @VisibleForTesting + static class Injector { + InputStream openCgroupProcs(String path) throws FileNotFoundException, SecurityException { + return new FileInputStream(path); + } + + int readCgroupProcs(InputStream input, byte[] buf, int offset, int len) throws IOException { + return input.read(buf, offset, len); + } + + String getProcessName(final int pid) { + return PhantomProcessList.getProcessName(pid); + } + } } diff --git a/services/tests/mockingservicestests/src/com/android/server/am/AppChildProcessTest.java b/services/tests/mockingservicestests/src/com/android/server/am/AppChildProcessTest.java index 04e8b634e72e0..b85da94604760 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/AppChildProcessTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/AppChildProcessTest.java @@ -34,12 +34,12 @@ import android.os.Handler; import android.os.HandlerThread; import android.os.Process; import android.platform.test.annotations.Presubmit; +import android.util.ArrayMap; import android.util.ArraySet; import com.android.dx.mockito.inline.extended.StaticMockitoSession; import com.android.server.LocalServices; import com.android.server.ServiceThread; -import com.android.server.am.ActivityManagerService.Injector; import com.android.server.appop.AppOpsService; import com.android.server.wm.ActivityTaskManagerService; @@ -55,7 +55,11 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import org.mockito.quality.Strictness; +import java.io.ByteArrayInputStream; import java.io.File; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; @Presubmit public class AppChildProcessTest { @@ -68,6 +72,7 @@ public class AppChildProcessTest { private Context mContext = getInstrumentation().getTargetContext(); private TestInjector mInjector; + private PhantomTestInjector mPhantomInjector; private ActivityManagerService mAms; private ProcessList mProcessList; private PhantomProcessList mPhantomProcessList; @@ -94,6 +99,7 @@ public class AppChildProcessTest { mProcessList = spy(pList); mInjector = new TestInjector(mContext); + mPhantomInjector = new PhantomTestInjector(); mAms = new ActivityManagerService(mInjector, mServiceThreadRule.getThread()); mAms.mActivityTaskManager = new ActivityTaskManagerService(mContext); mAms.mActivityTaskManager.initialize(null, null, mContext.getMainLooper()); @@ -101,6 +107,7 @@ public class AppChildProcessTest { mAms.mPackageManagerInt = mPackageManagerInt; pList.mService = mAms; mPhantomProcessList = mAms.mPhantomProcessList; + mPhantomProcessList.mInjector = mPhantomInjector; doReturn(new ComponentName("", "")).when(mPackageManagerInt).getSystemUiServiceComponent(); doReturn(false).when(() -> Process.supportsPidFd()); // Remove stale instance of PackageManagerInternal if there is any @@ -136,47 +143,60 @@ public class AppChildProcessTest { final String child2ProcessName = "test1_child1_child2"; final String nativeProcessName = "test_native"; - makeParent(zygote64Pid, initPid); - makeParent(zygote32Pid, initPid); + makeProcess(rootUid, zygote64Pid, zygote64ProcessName); + makeParent(rootUid, zygote64Pid, initPid); + makeProcess(rootUid, zygote32Pid, zygote32ProcessName); + makeParent(rootUid, zygote32Pid, initPid); makeAppProcess(app1Pid, app1Uid, app1ProcessName, app1ProcessName); - makeParent(app1Pid, zygote64Pid); makeAppProcess(app2Pid, app2Uid, app2ProcessName, app2ProcessName); - makeParent(app2Pid, zygote64Pid); + + mPhantomProcessList.lookForPhantomProcessesLocked(); assertEquals(0, mPhantomProcessList.mPhantomProcesses.size()); // Verify zygote itself isn't a phantom process assertEquals(null, mPhantomProcessList.getOrCreatePhantomProcessIfNeededLocked( - zygote64ProcessName, rootUid, zygote64Pid)); + zygote64ProcessName, rootUid, zygote64Pid, false)); assertEquals(null, mPhantomProcessList.getOrCreatePhantomProcessIfNeededLocked( - zygote32ProcessName, rootUid, zygote32Pid)); + zygote32ProcessName, rootUid, zygote32Pid, false)); // Verify none of the app isn't a phantom process assertEquals(null, mPhantomProcessList.getOrCreatePhantomProcessIfNeededLocked( - app1ProcessName, app1Uid, app1Pid)); + app1ProcessName, app1Uid, app1Pid, false)); assertEquals(null, mPhantomProcessList.getOrCreatePhantomProcessIfNeededLocked( - app2ProcessName, app2Uid, app2Pid)); + app2ProcessName, app2Uid, app2Pid, false)); // "Fork" an app child process - makeParent(child1Pid, app1Pid); + makeProcess(app1Uid, child1Pid, child1ProcessName); + makeParent(app1Uid, child1Pid, app1Pid); + mPhantomProcessList.lookForPhantomProcessesLocked(); + PhantomProcessRecord pr = mPhantomProcessList - .getOrCreatePhantomProcessIfNeededLocked(child1ProcessName, app1Uid, child1Pid); + .getOrCreatePhantomProcessIfNeededLocked( + child1ProcessName, app1Uid, child1Pid, true); assertTrue(pr != null); assertEquals(1, mPhantomProcessList.mPhantomProcesses.size()); assertEquals(pr, mPhantomProcessList.mPhantomProcesses.valueAt(0)); verifyPhantomProcessRecord(pr, child1ProcessName, app1Uid, child1Pid); // Create another native process from init - makeParent(nativePid, initPid); + makeProcess(rootUid, nativePid, nativeProcessName); + makeParent(rootUid, nativePid, initPid); + mPhantomProcessList.lookForPhantomProcessesLocked(); + assertEquals(null, mPhantomProcessList.getOrCreatePhantomProcessIfNeededLocked( - nativeProcessName, rootUid, nativePid)); + nativeProcessName, rootUid, nativePid, false)); assertEquals(1, mPhantomProcessList.mPhantomProcesses.size()); assertEquals(pr, mPhantomProcessList.mPhantomProcesses.valueAt(0)); // "Fork" another app child process - makeParent(child2Pid, child1Pid); + makeProcess(app1Uid, child2Pid, child2ProcessName); + makeParent(app1Uid, child2Pid, app1Pid); + mPhantomProcessList.lookForPhantomProcessesLocked(); + PhantomProcessRecord pr2 = mPhantomProcessList - .getOrCreatePhantomProcessIfNeededLocked(child2ProcessName, app1Uid, child2Pid); + .getOrCreatePhantomProcessIfNeededLocked( + child2ProcessName, app1Uid, child2Pid, false); assertTrue(pr2 != null); assertEquals(2, mPhantomProcessList.mPhantomProcesses.size()); verifyPhantomProcessRecord(pr2, child2ProcessName, app1Uid, child2Pid); @@ -197,19 +217,27 @@ public class AppChildProcessTest { assertEquals(pid, pr.mPid); } + private void makeProcess(int uid, int pid, String processName) { + doReturn(uid).when(() -> Process.getUidForPid(eq(pid))); + mPhantomInjector.mPidToName.put(pid, processName); + } + private void makeAppProcess(int pid, int uid, String packageName, String processName) { + makeProcess(uid, pid, processName); ApplicationInfo ai = new ApplicationInfo(); ai.packageName = packageName; + ai.uid = uid; ProcessRecord app = new ProcessRecord(mAms, ai, processName, uid); app.pid = pid; mAms.mPidsSelfLocked.doAddInternal(app); + mPhantomInjector.addToProcess(uid, pid, pid); } - private void makeParent(int pid, int ppid) { - doReturn(ppid).when(() -> Process.getParentPid(eq(pid))); + private void makeParent(int uid, int pid, int ppid) { + mPhantomInjector.addToProcess(uid, ppid, pid); } - private class TestInjector extends Injector { + private class TestInjector extends ActivityManagerService.Injector { TestInjector(Context context) { super(context); } @@ -230,6 +258,56 @@ public class AppChildProcessTest { } } + private class PhantomTestInjector extends PhantomProcessList.Injector { + ArrayMap mPathToInput = new ArrayMap<>(); + ArrayMap mPathToData = new ArrayMap<>(); + ArrayMap mInputToData = new ArrayMap<>(); + ArrayMap mPidToName = new ArrayMap<>(); + + @Override + InputStream openCgroupProcs(String path) throws FileNotFoundException, SecurityException { + InputStream input = mPathToInput.get(path); + if (input != null) { + return input; + } + input = new ByteArrayInputStream(new byte[8]); // buf size doesn't matter here + mPathToInput.put(path, input); + StringBuffer sb = mPathToData.get(path); + if (sb == null) { + sb = new StringBuffer(); + mPathToData.put(path, sb); + } + mInputToData.put(input, sb); + return input; + } + + @Override + int readCgroupProcs(InputStream input, byte[] buf, int offset, int len) throws IOException { + StringBuffer sb = mInputToData.get(input); + if (sb == null) { + return -1; + } + byte[] avail = sb.toString().getBytes(); + System.arraycopy(avail, 0, buf, offset, Math.min(len, avail.length)); + return Math.min(len, avail.length); + } + + @Override + String getProcessName(final int pid) { + return mPidToName.get(pid); + } + + void addToProcess(int uid, int pid, int newPid) { + final String path = PhantomProcessList.getCgroupFilePath(uid, pid); + StringBuffer sb = mPathToData.get(path); + if (sb == null) { + sb = new StringBuffer(); + mPathToData.put(path, sb); + } + sb.append(newPid).append('\n'); + } + } + static class ServiceThreadRule implements TestRule { private ServiceThread mThread;