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

Commit fc811dce authored by Riddle Hsu's avatar Riddle Hsu
Browse files

Cancel pending launch only if its removed process wasn't attached 

There could be a corner race:
Low-memory-killer kills a process which is the target process of
a launching activity. Assume binder thread T1, T2:
 T1 receives binder died.
 T1 sets thread null.
    From AMS: handleAppDiedLocked > handleAppDiedLocked
               > onCleanupApplicationRecordLSP > makeInactive
               > WindowProcessController#setThread)
 T2 previous activity paused triggers to resume next top activity,
    then requests to start process because thread is null.
    From ActivityTaskSupervisor#startSpecificActivity
 T1 removes died process record.
    From AMS: handleAppDiedLocked > cleanUpApplicationRecordLocked
               > removeProcessNameLocked
               > ATMI:onProcessRemoved
               > remove launching activities

The activity removal in onProcessRemoved only focuses on handling
process which hasn't completed attach (i.e. no binder died).

Bug: 360797479
Flag: EXEMPT bugfix
Test: atest RootWindowContainerTests#testAttachApplication
Change-Id: I36484c634fa3e97194d3e60a9d33d0490051570f
parent 5268d622

services/core/java/com/android/server/wm/ActivityTaskManagerService.java

+2 −1
Original line number Diff line number Diff line
@@ -6218,7 +6218,8 @@ public class ActivityTaskManagerService extends IActivityTaskManager.Stub { 
        public void onProcessRemoved(String name, int uid) {

            synchronized (mGlobalLockWithoutBoost) {

                final WindowProcessController proc = mProcessNames.remove(name, uid);

                if (proc != null && !mStartingProcessActivities.isEmpty()) {

                if (proc != null && !proc.mHasEverAttached

                        && !mStartingProcessActivities.isEmpty()) {

                    // Use a copy in case finishIfPossible changes the list indirectly.

                    final ArrayList<ActivityRecord> activities =

                            new ArrayList<>(mStartingProcessActivities);

services/core/java/com/android/server/wm/RootWindowContainer.java

+1 −0
Original line number Diff line number Diff line
@@ -1841,6 +1841,7 @@ class RootWindowContainer extends WindowContainer<DisplayContent> 
    }



    boolean attachApplication(WindowProcessController app) throws RemoteException {

        app.mHasEverAttached = true;

        final ArrayList<ActivityRecord> activities = mService.mStartingProcessActivities;

        RemoteException remoteException = null;

        boolean hasActivityStarted = false;

services/core/java/com/android/server/wm/WindowProcessController.java

+3 −0
Original line number Diff line number Diff line
@@ -204,6 +204,9 @@ public class WindowProcessController extends ConfigurationContainer<Configuratio 
    // Set to true when process was launched with a wrapper attached

    private volatile boolean mUsingWrapper;



    /** Whether this process has ever completed ActivityThread#handleBindApplication. */

    boolean mHasEverAttached;



    /** Non-null if this process may have a window. */

    @Nullable

    Session mWindowSession;

services/tests/wmtests/src/com/android/server/wm/RootWindowContainerTests.java

+10 −0
Original line number Diff line number Diff line
@@ -331,6 +331,7 @@ public class RootWindowContainerTests extends WindowTestsBase { 
        final WindowProcessController proc = mSystemServicesTestRule.addProcess(

                activity.packageName, activity.processName,

                6789 /* pid */, activity.info.applicationInfo.uid);

        assertFalse(proc.mHasEverAttached);

        try {

            mRootWindowContainer.attachApplication(proc);

            verify(mSupervisor).realStartActivityLocked(eq(topActivity), eq(proc),
@@ -338,6 +339,15 @@ public class RootWindowContainerTests extends WindowTestsBase { 
        } catch (RemoteException e) {

            e.rethrowAsRuntimeException();

        }



        // Verify that onProcessRemoved won't clear the launching activities if an attached process

        // is died. Because in real case, it should be handled from WindowProcessController's

        // and ActivityRecord's handleAppDied to decide whether to remove the activities.

        assertTrue(proc.mHasEverAttached);

        assertTrue(mAtm.mStartingProcessActivities.isEmpty());

        mAtm.mStartingProcessActivities.add(activity);

        mAtm.mInternal.onProcessRemoved(proc.mName, proc.mUid);

        assertFalse(mAtm.mStartingProcessActivities.isEmpty());

    }



    /**