Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6dabda8d authored by Hui Yu's avatar Hui Yu
Browse files

Clean up ProcessRecord when reuse a pid. 

When Zygote starts a process with a pid, system_server may have this
pid associate with a old process which is killed by the OS but
system_server has not finished cleanup. In this case, clean up the old
ProcessRecord so the new process can use the pid.

This problem is exposed because the asynchronous process start change.
attachApplicationLocked() may happen before handleProcessStartedLocked() and
the mPidsSelfLocked may still have the old ProcessRecord associate with
the new process's pid.

Pid alone can not uniquely identify the process in mPidsSelfLocked.
In addition to pid, use the startSeq to uniquely identify the ProcessRecord. Refactor
PidMap.put() and PidMap.remove() to take a ProcessRecord as parameter.
In PidMap.remove(), check startSeq before removing ProcessRecord from
the map.

Bug: 131105245
Test: Using the POC test steps in b/131105245. Without the fix, the issue
can be reproduced in few attempts. With the fix, the issue can not be reproduced
any more.

Change-Id: I5d421f6c68f6b3437d51c94f4aef77e08a7bf002
parent 9d89fd9f

services/core/java/com/android/server/am/ActivityManagerService.java

+43 −15
Original line number Diff line number Diff line
@@ -734,11 +734,11 @@ public class ActivityManagerService extends IActivityManager.Stub 
         * <p>NOTE: Callers should avoid acquiring the mPidsSelfLocked lock before calling this

         * method.

         */

        void put(int key, ProcessRecord value) {

        void put(ProcessRecord app) {

            synchronized (this) {

                mPidMap.put(key, value);

                mPidMap.put(app.pid, app);

            }

            mAtmInternal.onProcessMapped(key, value.getWindowProcessController());

            mAtmInternal.onProcessMapped(app.pid, app.getWindowProcessController());

        }















        /**









@@ -746,11 +746,18 @@ public class ActivityManagerService extends IActivityManager.Stub













         * <p>NOTE: Callers should avoid acquiring the mPidsSelfLocked lock before calling this















         * method.















         */













        void remove(int pid) {













        void remove(ProcessRecord app) {













            boolean removed = false;















            synchronized (this) {













                mPidMap.remove(pid);













                final ProcessRecord existingApp = mPidMap.get(app.pid);













                if (existingApp != null && existingApp.startSeq == app.startSeq) {













                    mPidMap.remove(app.pid);













                    removed = true;













                }













            }













            if (removed) {













                mAtmInternal.onProcessUnMapped(app.pid);















            }













            mAtmInternal.onProcessUnMapped(pid);















        }





























        /**









@@ -758,17 +765,18 @@ public class ActivityManagerService extends IActivityManager.Stub













         * <p>NOTE: Callers should avoid acquiring the mPidsSelfLocked lock before calling this















         * method.















         */













        boolean removeIfNoThread(int pid) {













        boolean removeIfNoThread(ProcessRecord app) {















            boolean removed = false;















            synchronized (this) {













                final ProcessRecord app = get(pid);













                if (app != null && app.thread == null) {













                    mPidMap.remove(pid);













                final ProcessRecord existingApp = get(app.pid);













                if (existingApp != null && existingApp.startSeq == app.startSeq













                        && app.thread == null) {













                    mPidMap.remove(app.pid);















                    removed = true;















                }















            }















            if (removed) {













                mAtmInternal.onProcessUnMapped(pid);













                mAtmInternal.onProcessUnMapped(app.pid);















            }















            return removed;















        }










@@ -1969,7 +1977,7 @@ public class ActivityManagerService extends IActivityManager.Stub













                app.getWindowProcessController().setPid(MY_PID);















                app.maxAdj = ProcessList.SYSTEM_ADJ;















                app.makeActive(mSystemThread.getApplicationThread(), mProcessStats);













                mPidsSelfLocked.put(app.pid, app);













                mPidsSelfLocked.put(app);















                mProcessList.updateLruProcessLocked(app, false, null);















                updateOomAdjLocked(OomAdjuster.OOM_ADJ_REASON_NONE);















            }










@@ -4600,7 +4608,7 @@ public class ActivityManagerService extends IActivityManager.Stub













    @GuardedBy("this")















    private final void processStartTimedOutLocked(ProcessRecord app) {















        final int pid = app.pid;













        boolean gone = mPidsSelfLocked.removeIfNoThread(pid);













        boolean gone = mPidsSelfLocked.removeIfNoThread(app);





























        if (gone) {















            Slog.w(TAG, "Process " + app + " failed to attach");










@@ -4657,6 +4665,26 @@ public class ActivityManagerService extends IActivityManager.Stub













            synchronized (mPidsSelfLocked) {















                app = mPidsSelfLocked.get(pid);















            }













            if (app != null && (app.startUid != callingUid || app.startSeq != startSeq)) {













                String processName = null;













                final ProcessRecord pending = mProcessList.mPendingStarts.get(startSeq);













                if (pending != null) {













                    processName = pending.processName;













                }













                final String msg = "attachApplicationLocked process:" + processName













                        + " startSeq:" + startSeq













                        + " pid:" + pid













                        + " belongs to another existing app:" + app.processName













                        + " startSeq:" + app.startSeq;













                Slog.wtf(TAG, msg);













                // SafetyNet logging for b/131105245.













                EventLog.writeEvent(0x534e4554, "131105245", app.startUid, msg);













                // If there is already an app occupying that pid that hasn't been cleaned up













                cleanUpApplicationRecordLocked(app, false, false, -1,













                            true /*replacingPid*/);













                mPidsSelfLocked.remove(app);













                app = null;













            }















        } else {















            app = null;















        }









@@ -4665,7 +4693,7 @@ public class ActivityManagerService extends IActivityManager.Stub













        // update the internal state.















        if (app == null && startSeq > 0) {















            final ProcessRecord pending = mProcessList.mPendingStarts.get(startSeq);













            if (pending != null && pending.startUid == callingUid













            if (pending != null && pending.startUid == callingUid && pending.startSeq == startSeq















                    && mProcessList.handleProcessStartedLocked(pending, pid, pending















                            .isUsingWrapper(),















                            startSeq, true)) {










@@ -13639,7 +13667,7 @@ public class ActivityManagerService extends IActivityManager.Stub













            return true;















        } else if (app.pid > 0 && app.pid != MY_PID) {















            // Goodbye!













            mPidsSelfLocked.remove(app.pid);













            mPidsSelfLocked.remove(app);















            mHandler.removeMessages(PROC_START_TIMEOUT_MSG, app);















            mBatteryStatsService.noteProcessFinish(app.processName, app.info.uid);















            if (app.isolated) {

























services/core/java/com/android/server/am/ProcessList.java



+17
−5




























Original line number
Diff line number
Diff line








@@ -1445,10 +1445,11 @@ public final class ProcessList {













        long startTime = SystemClock.elapsedRealtime();















        if (app.pid > 0 && app.pid != ActivityManagerService.MY_PID) {















            checkSlow(startTime, "startProcess: removing from pids map");













            mService.mPidsSelfLocked.remove(app.pid);













            mService.mPidsSelfLocked.remove(app);















            mService.mHandler.removeMessages(PROC_START_TIMEOUT_MSG, app);















            checkSlow(startTime, "startProcess: done removing from pids map");















            app.setPid(0);













            app.startSeq = 0;















        }































        if (DEBUG_PROCESSES && mService.mProcessesOnHold.contains(app)) Slog.v(










@@ -1656,6 +1657,14 @@ public final class ProcessList {













        app.killedByAm = false;















        app.removed = false;















        app.killed = false;













        if (app.startSeq != 0) {













            Slog.wtf(TAG, "startProcessLocked processName:" + app.processName













                    + " with non-zero startSeq:" + app.startSeq);













        }













        if (app.pid != 0) {













            Slog.wtf(TAG, "startProcessLocked processName:" + app.processName













                    + " with non-zero pid:" + app.pid);













        }















        final long startSeq = app.startSeq = ++mProcStartSeqCounter;















        app.setStartParams(uid, hostingRecord, seInfo, startTime);















        if (mService.mConstants.FLAG_PROCESS_START_ASYNC) {










@@ -2079,12 +2088,15 @@ public final class ProcessList {













        // If there is already an app occupying that pid that hasn't been cleaned up















        if (oldApp != null && !app.isolated) {















            // Clean up anything relating to this pid first













            Slog.w(TAG, "Reusing pid " + pid













                    + " while app is still mapped to it");













            Slog.wtf(TAG, "handleProcessStartedLocked process:" + app.processName













                    + " startSeq:" + app.startSeq













                    + " pid:" + pid













                    + " belongs to another existing app:" + oldApp.processName













                    + " startSeq:" + oldApp.startSeq);















            mService.cleanUpApplicationRecordLocked(oldApp, false, false, -1,















                    true /*replacingPid*/);















        }













        mService.mPidsSelfLocked.put(pid, app);













        mService.mPidsSelfLocked.put(app);















        synchronized (mService.mPidsSelfLocked) {















            if (!procAttached) {















                Message msg = mService.mHandler.obtainMessage(PROC_START_TIMEOUT_MSG);










@@ -2257,7 +2269,7 @@ public final class ProcessList {













                .pendingStart)) {















            int pid = app.pid;















            if (pid > 0) {













                mService.mPidsSelfLocked.remove(pid);













                mService.mPidsSelfLocked.remove(app);















                mService.mHandler.removeMessages(PROC_START_TIMEOUT_MSG, app);















                mService.mBatteryStatsService.noteProcessFinish(app.processName, app.info.uid);















                if (app.isolated) {