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

Commit 725dad7d authored by Antony Sargent's avatar Antony Sargent
Browse files

Prevent deadlocks during VirtualDisplay creation 

When creating a VirtualDisplay for a VirtualDevice, we need to do some
extra work like setting up for input injection, creating a
DisplayWindowPolicyController, and grabbing a wakelock to keep the
display awake. Some parts of this extra work end up calling back into
the DisplayManagerService from another thread, but since we're already
holding the mSyncRoot lock we sometimes get into situations where
those threads get blocked trying to acquire mSyncRoot, while holding a
lock that another thread ends up needing like the global window
manager one, so we get deadlock.

The fix in this CL is to break up the extra work the
VirtualDeviceManagerService needs to do into two parts: one part that
happens before the VirtualDisplay is created, and one part
after. Neither of these needs mSyncRoot to be held.

Fixes: 230544802
Test: atest CreateVirtualDisplayTest
Change-Id: I6a6e03f816567579510b3f1ef0705375549a3c88
parent 0bdc2763

core/java/android/companion/virtual/IVirtualDeviceManager.aidl

+14 −0
Original line number Diff line number Diff line
@@ -19,6 +19,8 @@ package android.companion.virtual; 
import android.companion.virtual.IVirtualDevice;

import android.companion.virtual.IVirtualDeviceActivityListener;

import android.companion.virtual.VirtualDeviceParams;

import android.hardware.display.IVirtualDisplayCallback;

import android.hardware.display.VirtualDisplayConfig;



/**

 * Interface for communication between VirtualDeviceManager and VirtualDeviceManagerService.
@@ -42,4 +44,16 @@ interface IVirtualDeviceManager { 
    IVirtualDevice createVirtualDevice(

            in IBinder token, String packageName, int associationId,

            in VirtualDeviceParams params, in IVirtualDeviceActivityListener activityListener);



    /**

     * Creates a virtual display owned by a particular virtual device.

     *

     * @param virtualDisplayConfig The configuration used in creating the display

     * @param callback A callback that receives display lifecycle events

     * @param virtualDevice The device that will own this display

     * @param packageName The package name of the calling app

     */

    int createVirtualDisplay(in VirtualDisplayConfig virtualDisplayConfig,

            in IVirtualDisplayCallback callback, in IVirtualDevice virtualDevice,

            String packageName);

}

core/java/android/companion/virtual/VirtualDeviceManager.java

+22 −13
Original line number Diff line number Diff line
@@ -33,6 +33,8 @@ import android.content.Context; 
import android.graphics.Point;

import android.hardware.display.DisplayManager;

import android.hardware.display.DisplayManager.VirtualDisplayFlag;

import android.hardware.display.DisplayManagerGlobal;

import android.hardware.display.IVirtualDisplayCallback;

import android.hardware.display.VirtualDisplay;

import android.hardware.display.VirtualDisplayConfig;

import android.hardware.input.VirtualKeyboard;
@@ -65,7 +67,7 @@ import java.util.function.IntConsumer; 
public final class VirtualDeviceManager {



    private static final boolean DEBUG = false;

    private static final String LOG_TAG = "VirtualDeviceManager";

    private static final String TAG = "VirtualDeviceManager";



    private static final int DEFAULT_VIRTUAL_DISPLAY_FLAGS =

            DisplayManager.VIRTUAL_DISPLAY_FLAG_PUBLIC
@@ -150,6 +152,7 @@ public final class VirtualDeviceManager { 
    public static class VirtualDevice implements AutoCloseable {



        private final Context mContext;

        private final IVirtualDeviceManager mService;

        private final IVirtualDevice mVirtualDevice;

        private final ArrayMap<ActivityListener, ActivityListenerDelegate> mActivityListeners =

                new ArrayMap<>();
@@ -189,6 +192,7 @@ public final class VirtualDeviceManager { 
                Context context,

                int associationId,

                VirtualDeviceParams params) throws RemoteException {

            mService = service;

            mContext = context.getApplicationContext();

            mVirtualDevice = service.createVirtualDevice(

                    new Binder(),
@@ -274,18 +278,23 @@ public final class VirtualDeviceManager { 
            // TODO(b/205343547): Handle display groups properly instead of creating a new display

            //  group for every new virtual display created using this API.

            // belongs to the same display group.

            DisplayManager displayManager = mContext.getSystemService(DisplayManager.class);

            // DisplayManager will call into VirtualDeviceManagerInternal to register the

            // created displays.

            return displayManager.createVirtualDisplay(

                    mVirtualDevice,

                    new VirtualDisplayConfig.Builder(

            VirtualDisplayConfig config = new VirtualDisplayConfig.Builder(

                    getVirtualDisplayName(), width, height, densityDpi)

                    .setSurface(surface)

                    .setFlags(getVirtualDisplayFlags(flags))

                            .build(),

                    callback,

                    executor);

                    .build();

            IVirtualDisplayCallback callbackWrapper =

                    new DisplayManagerGlobal.VirtualDisplayCallback(callback, executor);

            final int displayId;

            try {

                displayId = mService.createVirtualDisplay(config, callbackWrapper, mVirtualDevice,

                        mContext.getPackageName());

            } catch (RemoteException ex) {

                throw ex.rethrowFromSystemServer();

            }

            DisplayManagerGlobal displayManager = DisplayManagerGlobal.getInstance();

            return displayManager.createVirtualDisplayWrapper(config, mContext, callbackWrapper,

                    displayId);

        }



        /**

core/java/android/hardware/display/DisplayManager.java

+2 −12
Original line number Diff line number Diff line
@@ -31,7 +31,6 @@ import android.annotation.SystemApi; 
import android.annotation.SystemService;

import android.annotation.TestApi;

import android.app.KeyguardManager;

import android.companion.virtual.IVirtualDevice;

import android.compat.annotation.UnsupportedAppUsage;

import android.content.Context;

import android.content.res.Resources;
@@ -971,17 +970,8 @@ public final class DisplayManager { 
            executor = new HandlerExecutor(

                    Handler.createAsync(handler != null ? handler.getLooper() : Looper.myLooper()));

        }

        return mGlobal.createVirtualDisplay(mContext, projection, null /* virtualDevice */,

                virtualDisplayConfig, callback, executor, windowContext);

    }



    /** @hide */

    public VirtualDisplay createVirtualDisplay(@Nullable IVirtualDevice virtualDevice,

            @NonNull VirtualDisplayConfig virtualDisplayConfig,

            @Nullable VirtualDisplay.Callback callback,

            @Nullable Executor executor) {

        return mGlobal.createVirtualDisplay(mContext, null /* projection */, virtualDevice,

                virtualDisplayConfig, callback, executor, null);

        return mGlobal.createVirtualDisplay(mContext, projection, virtualDisplayConfig, callback,

                executor, windowContext);

    }



    /**

core/java/android/hardware/display/DisplayManagerGlobal.java

+19 −7
Original line number Diff line number Diff line
@@ -24,7 +24,6 @@ import android.annotation.IntDef; 
import android.annotation.NonNull;

import android.annotation.Nullable;

import android.app.PropertyInvalidatedCache;

import android.companion.virtual.IVirtualDevice;

import android.compat.annotation.UnsupportedAppUsage;

import android.content.Context;

import android.content.pm.ParceledListSlice;
@@ -586,18 +585,28 @@ public final class DisplayManagerGlobal { 
    }



    public VirtualDisplay createVirtualDisplay(@NonNull Context context, MediaProjection projection,

            IVirtualDevice virtualDevice, @NonNull VirtualDisplayConfig virtualDisplayConfig,

            VirtualDisplay.Callback callback, @Nullable Executor executor,

            @Nullable Context windowContext) {

            @NonNull VirtualDisplayConfig virtualDisplayConfig, VirtualDisplay.Callback callback,

            @Nullable Executor executor, @Nullable Context windowContext) {

        VirtualDisplayCallback callbackWrapper = new VirtualDisplayCallback(callback, executor);

        IMediaProjection projectionToken = projection != null ? projection.getProjection() : null;

        int displayId;

        try {

            displayId = mDm.createVirtualDisplay(virtualDisplayConfig, callbackWrapper,

                    projectionToken, virtualDevice, context.getPackageName());

                    projectionToken, context.getPackageName());

        } catch (RemoteException ex) {

            throw ex.rethrowFromSystemServer();

        }

        return createVirtualDisplayWrapper(virtualDisplayConfig, windowContext, callbackWrapper,

                displayId);

    }



    /**

     * Create a VirtualDisplay wrapper object for a newly created virtual display ; to be called

     * once the display has been created in system_server.

     */

    @Nullable

    public VirtualDisplay createVirtualDisplayWrapper(VirtualDisplayConfig virtualDisplayConfig,

            Context windowContext, IVirtualDisplayCallback callbackWrapper, int displayId) {

        if (displayId < 0) {

            Log.e(TAG, "Could not create virtual display: " + virtualDisplayConfig.getName());

            return null;
@@ -1050,7 +1059,10 @@ public final class DisplayManagerGlobal { 
        }

    }



    private final static class VirtualDisplayCallback extends IVirtualDisplayCallback.Stub {

    /**

     * Assists in dispatching VirtualDisplay lifecycle event callbacks on a given Executor.

     */

    public static final class VirtualDisplayCallback extends IVirtualDisplayCallback.Stub {

        @Nullable private final VirtualDisplay.Callback mCallback;

        @Nullable private final Executor mExecutor;
@@ -1062,7 +1074,7 @@ public final class DisplayManagerGlobal { 
         * @param executor The executor to call the {@code callback} on. Must not be {@code null} if

         * the callback is not {@code null}.

         */

        VirtualDisplayCallback(VirtualDisplay.Callback callback, Executor executor) {

        public VirtualDisplayCallback(VirtualDisplay.Callback callback, Executor executor) {

            mCallback = callback;

            mExecutor = mCallback != null ? Objects.requireNonNull(executor) : null;

        }

core/java/android/hardware/display/DisplayManagerInternal.java

+9 −0
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package android.hardware.display; 


import android.annotation.IntDef;

import android.annotation.Nullable;

import android.companion.virtual.IVirtualDevice;

import android.graphics.Point;

import android.hardware.SensorManager;

import android.os.Handler;
@@ -59,6 +60,14 @@ public abstract class DisplayManagerInternal { 
    public abstract void initPowerManagement(DisplayPowerCallbacks callbacks,

            Handler handler, SensorManager sensorManager);



    /**

     * Called by the VirtualDeviceManagerService to create a VirtualDisplay owned by a

     * VirtualDevice.

     */

    public abstract int createVirtualDisplay(VirtualDisplayConfig config,

            IVirtualDisplayCallback callback, IVirtualDevice virtualDevice,

            DisplayWindowPolicyController dwpc, String packageName);



    /**

     * Called by the power manager to request a new power state.

     * <p>