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

Commit bed37ac5 authored by Chris Li's avatar Chris Li
Browse files

Fix potential crash when TaskFragmentOrganizer died

Before, when TaskFragmentOrganizerController#onTransactionHandled()
is called, we will always call #validateAndGetState(), which call throw
an exception for racing condition like organizer process died.

Bug: 207070762
Test: atest WmTests:TaskFragmentOrganizerControllerTest
Change-Id: I04d37ecab4a37a616167ce4e6ca98c668f1254da
parent 63cd60ee
Loading
Loading
Loading
Loading
+30 −8
Original line number Original line Diff line number Diff line
@@ -425,7 +425,7 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr
            ProtoLog.v(WM_DEBUG_WINDOW_ORGANIZER,
            ProtoLog.v(WM_DEBUG_WINDOW_ORGANIZER,
                    "Register task fragment organizer=%s uid=%d pid=%d",
                    "Register task fragment organizer=%s uid=%d pid=%d",
                    organizer.asBinder(), uid, pid);
                    organizer.asBinder(), uid, pid);
            if (mTaskFragmentOrganizerState.containsKey(organizer.asBinder())) {
            if (isOrganizerRegistered(organizer)) {
                throw new IllegalStateException(
                throw new IllegalStateException(
                        "Replacing existing organizer currently unsupported");
                        "Replacing existing organizer currently unsupported");
            }
            }
@@ -503,19 +503,27 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr
            @WindowManager.TransitionType int transitionType, boolean shouldApplyIndependently) {
            @WindowManager.TransitionType int transitionType, boolean shouldApplyIndependently) {
        // Keep the calling identity to avoid unsecure change.
        // Keep the calling identity to avoid unsecure change.
        synchronized (mGlobalLock) {
        synchronized (mGlobalLock) {
            if (isValidTransaction(wct)) {
                applyTransaction(wct, transitionType, shouldApplyIndependently);
                applyTransaction(wct, transitionType, shouldApplyIndependently);
            final TaskFragmentOrganizerState state = validateAndGetState(
            }
                    wct.getTaskFragmentOrganizer());
            // Even if the transaction is empty, we still need to invoke #onTransactionFinished
            // unless the organizer has been unregistered.
            final ITaskFragmentOrganizer organizer = wct.getTaskFragmentOrganizer();
            final TaskFragmentOrganizerState state = organizer != null
                    ? mTaskFragmentOrganizerState.get(organizer.asBinder())
                    : null;
            if (state != null) {
                state.onTransactionFinished(transactionToken);
                state.onTransactionFinished(transactionToken);
            }
            }
        }
        }
    }


    @Override
    @Override
    public void applyTransaction(@NonNull WindowContainerTransaction wct,
    public void applyTransaction(@NonNull WindowContainerTransaction wct,
            @WindowManager.TransitionType int transitionType, boolean shouldApplyIndependently) {
            @WindowManager.TransitionType int transitionType, boolean shouldApplyIndependently) {
        // Keep the calling identity to avoid unsecure change.
        // Keep the calling identity to avoid unsecure change.
        synchronized (mGlobalLock) {
        synchronized (mGlobalLock) {
            if (wct.isEmpty()) {
            if (!isValidTransaction(wct)) {
                return;
                return;
            }
            }
            mWindowOrganizerController.applyTaskFragmentTransactionLocked(wct, transitionType,
            mWindowOrganizerController.applyTaskFragmentTransactionLocked(wct, transitionType,
@@ -656,7 +664,7 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr
            }
            }
            organizer = organizedTf[0].getTaskFragmentOrganizer();
            organizer = organizedTf[0].getTaskFragmentOrganizer();
        }
        }
        if (!mTaskFragmentOrganizerState.containsKey(organizer.asBinder())) {
        if (!isOrganizerRegistered(organizer)) {
            Slog.w(TAG, "The last TaskFragmentOrganizer no longer exists");
            Slog.w(TAG, "The last TaskFragmentOrganizer no longer exists");
            return;
            return;
        }
        }
@@ -702,7 +710,7 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr
        mPendingTaskFragmentEvents.get(event.mTaskFragmentOrg.asBinder()).remove(event);
        mPendingTaskFragmentEvents.get(event.mTaskFragmentOrg.asBinder()).remove(event);
    }
    }


    boolean isOrganizerRegistered(@NonNull ITaskFragmentOrganizer organizer) {
    private boolean isOrganizerRegistered(@NonNull ITaskFragmentOrganizer organizer) {
        return mTaskFragmentOrganizerState.containsKey(organizer.asBinder());
        return mTaskFragmentOrganizerState.containsKey(organizer.asBinder());
    }
    }


@@ -739,6 +747,20 @@ public class TaskFragmentOrganizerController extends ITaskFragmentOrganizerContr
        return state;
        return state;
    }
    }


    boolean isValidTransaction(@NonNull WindowContainerTransaction t) {
        if (t.isEmpty()) {
            return false;
        }
        final ITaskFragmentOrganizer organizer = t.getTaskFragmentOrganizer();
        if (t.getTaskFragmentOrganizer() == null || !isOrganizerRegistered(organizer)) {
            // Transaction from an unregistered organizer should not be applied. This can happen
            // when the organizer process died before the transaction is applied.
            Slog.e(TAG, "Caller organizer=" + organizer + " is no longer registered");
            return false;
        }
        return true;
    }

    /**
    /**
     * A class to store {@link ITaskFragmentOrganizer} and its organized
     * A class to store {@link ITaskFragmentOrganizer} and its organized
     * {@link TaskFragment TaskFragments} with different pending event request.
     * {@link TaskFragment TaskFragments} with different pending event request.
+1 −16
Original line number Original line Diff line number Diff line
@@ -403,9 +403,6 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
     */
     */
    void applyTaskFragmentTransactionLocked(@NonNull WindowContainerTransaction wct,
    void applyTaskFragmentTransactionLocked(@NonNull WindowContainerTransaction wct,
            @WindowManager.TransitionType int type, boolean shouldApplyIndependently) {
            @WindowManager.TransitionType int type, boolean shouldApplyIndependently) {
        if (!isValidTransaction(wct)) {
            return;
        }
        enforceTaskFragmentOrganizerPermission("applyTaskFragmentTransaction()",
        enforceTaskFragmentOrganizerPermission("applyTaskFragmentTransaction()",
                Objects.requireNonNull(wct.getTaskFragmentOrganizer()),
                Objects.requireNonNull(wct.getTaskFragmentOrganizer()),
                Objects.requireNonNull(wct));
                Objects.requireNonNull(wct));
@@ -459,7 +456,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                    // calls startSyncSet.
                    // calls startSyncSet.
                    () -> mTransitionController.moveToCollecting(nextTransition),
                    () -> mTransitionController.moveToCollecting(nextTransition),
                    () -> {
                    () -> {
                        if (isValidTransaction(wct)) {
                        if (mTaskFragmentOrganizerController.isValidTransaction(wct)) {
                            applyTransaction(wct, -1 /*syncId*/, nextTransition, caller);
                            applyTransaction(wct, -1 /*syncId*/, nextTransition, caller);
                            mTransitionController.requestStartTransition(nextTransition,
                            mTransitionController.requestStartTransition(nextTransition,
                                    null /* startTask */, null /* remoteTransition */,
                                    null /* startTask */, null /* remoteTransition */,
@@ -1620,18 +1617,6 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
        return (cfgChanges & CONTROLLABLE_CONFIGS) == 0;
        return (cfgChanges & CONTROLLABLE_CONFIGS) == 0;
    }
    }


    private boolean isValidTransaction(@NonNull WindowContainerTransaction t) {
        if (t.getTaskFragmentOrganizer() != null && !mTaskFragmentOrganizerController
                .isOrganizerRegistered(t.getTaskFragmentOrganizer())) {
            // Transaction from an unregistered organizer should not be applied. This can happen
            // when the organizer process died before the transaction is applied.
            Slog.e(TAG, "Caller organizer=" + t.getTaskFragmentOrganizer()
                    + " is no longer registered");
            return false;
        }
        return true;
    }

    /**
    /**
     * Makes sure that the transaction only contains operations that are allowed for the
     * Makes sure that the transaction only contains operations that are allowed for the
     * {@link WindowContainerTransaction#getTaskFragmentOrganizer()}.
     * {@link WindowContainerTransaction#getTaskFragmentOrganizer()}.
+15 −0
Original line number Original line Diff line number Diff line
@@ -763,6 +763,21 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {
        assertNotNull(mWindowOrganizerController.getTaskFragment(fragmentToken));
        assertNotNull(mWindowOrganizerController.getTaskFragment(fragmentToken));
    }
    }


    @Test
    public void testOnTransactionHandled_skipTransactionForUnregisterOrganizer() {
        mController.unregisterOrganizer(mIOrganizer);
        final ActivityRecord ownerActivity = createActivityRecord(mDisplayContent);
        final IBinder fragmentToken = new Binder();

        // Allow organizer to create TaskFragment and start/reparent activity to TaskFragment.
        createTaskFragmentFromOrganizer(mTransaction, ownerActivity, fragmentToken);
        mController.onTransactionHandled(new Binder(), mTransaction,
                getTransitionType(mTransaction), false /* shouldApplyIndependently */);

        // Nothing should happen as the organizer is not registered.
        assertNull(mWindowOrganizerController.getTaskFragment(fragmentToken));
    }

    @Test
    @Test
    public void testOrganizerRemovedWithPendingEvents() {
    public void testOrganizerRemovedWithPendingEvents() {
        final TaskFragment tf0 = new TaskFragmentBuilder(mAtm)
        final TaskFragment tf0 = new TaskFragmentBuilder(mAtm)