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

Commit 5d718893 authored by Robin Lee's avatar Robin Lee
Browse files

Crash shell if callers use the wrong thread 

This prevents data races from custom transition handlers by making the
race very noticeable before anyone gets a chance to ship it.

Test: atest ShellTransitionTests
Flag: com.android.window.flags.enforce_shell_thread_model
Bug: 351189446
Change-Id: I3e9381e7a65a293a35b3e17ea37335f2e52d5e70
parent 002dd68f

core/java/android/window/flags/windowing_frontend.aconfig

+10 −0
Original line number Diff line number Diff line
@@ -210,3 +210,13 @@ flag { 
  }

}



flag {

  name: "enforce_shell_thread_model"

  namespace: "windowing_frentend"

  description: "Crash the shell process if someone calls in from the wrong thread"

  bug: "351189446"

  is_fixed_read_only: true

  metadata {

    purpose: PURPOSE_BUGFIX

  }

}

libs/WindowManager/Shell/src/com/android/wm/shell/common/HandlerExecutor.java

+7 −0
Original line number Diff line number Diff line
@@ -54,4 +54,11 @@ public class HandlerExecutor implements ShellExecutor { 
    public boolean hasCallback(Runnable r) {

        return mHandler.hasCallbacks(r);

    }



    @Override

    public void assertCurrentThread() {

        if (!mHandler.getLooper().isCurrentThread()) {

            throw new IllegalStateException("must be called on " + mHandler);

        }

    }

}

libs/WindowManager/Shell/src/com/android/wm/shell/common/ShellExecutor.java

+7 −0
Original line number Diff line number Diff line
@@ -96,4 +96,11 @@ public interface ShellExecutor extends Executor { 
     * See {@link android.os.Handler#hasCallbacks(Runnable)}.

     */

    boolean hasCallback(Runnable runnable);



    /**

     * May throw if the caller is not on the same thread as the executor.

     */

    default void assertCurrentThread() {

       return;

    }

}

libs/WindowManager/Shell/src/com/android/wm/shell/transition/Transitions.java

+30 −5
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ import static android.window.TransitionInfo.FLAG_IS_WALLPAPER; 
import static android.window.TransitionInfo.FLAG_NO_ANIMATION;

import static android.window.TransitionInfo.FLAG_STARTING_WINDOW_TRANSFER_RECIPIENT;



import static com.android.window.flags.Flags.enforceShellThreadModel;

import static com.android.window.flags.Flags.ensureWallpaperInTransitions;

import static com.android.systemui.shared.Flags.returnAnimationFrameworkLibrary;

import static com.android.wm.shell.shared.TransitionUtil.isClosingType;
@@ -921,9 +922,12 @@ public class Transitions implements RemoteCallable<Transitions>, 
        }

        // An existing animation is playing, so see if we can merge.

        final ActiveTransition playing = track.mActiveTransition;

        final IBinder playingToken = playing.mToken;

        final IBinder readyToken = ready.mToken;



        if (ready.mAborted) {

            // record as merged since it is no-op. Calls back into processReadyQueue

            onMerged(playing, ready);

            onMerged(playingToken, readyToken);

            return;

        }

        ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, "Transition %s ready while"
@@ -931,14 +935,31 @@ public class Transitions implements RemoteCallable<Transitions>, 
                + " in case they can be merged", ready, playing);

        mTransitionTracer.logMergeRequested(ready.mInfo.getDebugId(), playing.mInfo.getDebugId());

        playing.mHandler.mergeAnimation(ready.mToken, ready.mInfo, ready.mStartT,

                playing.mToken, (wct) -> onMerged(playing, ready));

                playing.mToken, (wct) -> onMerged(playingToken, readyToken));

    }



    private void onMerged(@NonNull IBinder playingToken, @NonNull IBinder mergedToken) {

        if (enforceShellThreadModel()) {

            mMainExecutor.assertCurrentThread();

        }



        ActiveTransition playing = mKnownTransitions.get(playingToken);

        if (playing == null) {

            Log.e(TAG, "Merging into a non-existent transition: " + playingToken);

            return;

        }



        ActiveTransition merged = mKnownTransitions.get(mergedToken);

        if (merged == null) {

            Log.e(TAG, "Merging a non-existent transition: " + mergedToken);

            return;

        }



    private void onMerged(@NonNull ActiveTransition playing, @NonNull ActiveTransition merged) {

        if (playing.getTrack() != merged.getTrack()) {

            throw new IllegalStateException("Can't merge across tracks: " + merged + " into "

                    + playing);

        }



        final Track track = mTracks.get(playing.getTrack());

        ProtoLog.v(ShellProtoLogGroup.WM_SHELL_TRANSITIONS, "Transition was merged: %s into %s",

                merged, playing);
@@ -1068,13 +1089,17 @@ public class Transitions implements RemoteCallable<Transitions>, 
        info.releaseAnimSurfaces();

    }



    private void onFinish(IBinder token,

            @Nullable WindowContainerTransaction wct) {

    private void onFinish(IBinder token, @Nullable WindowContainerTransaction wct) {

        if (enforceShellThreadModel()) {

            mMainExecutor.assertCurrentThread();

        }



        final ActiveTransition active = mKnownTransitions.get(token);

        if (active == null) {

            Log.e(TAG, "Trying to finish a non-existent transition: " + token);

            return;

        }



        final Track track = mTracks.get(active.getTrack());

        if (track == null || track.mActiveTransition != active) {

            Log.e(TAG, "Trying to finish a non-running transition. Either remote crashed or "