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

Commit 773ec045 authored by Antony Sargent's avatar Antony Sargent
Browse files

Fix occasional crash in VirtualDeviceManagerService 

If a VirtualDevice is closed very soon after an Activity is displayed
on one of its VirtualDisplays, we can get a crash in
VirtualDeviceManagerService because a late call to
notifyRunningAppsChanged just after the close both leaves
mVirtualDevices and mAppsOnVirtualDevices out of sync about whether
the device still exists, and also dispatches the
onAppsOnVirtualDeviceChanged to listeners via the LocalService where
listeners may try and get the device that's already been removed.

This problem happens occasionally when running CTS tests where we are
often tearing down a VirtualDevice at the end of a test just after
starting an Activity and verifying something, but is likely rare in
regular device usage.

Bug: 265825399
Test: atest VirtualDeviceManagerServiceTest
Change-Id: I11f3d85a9ed81a57cebd561bd0ba3c28d75f520b
parent aa8a8dff

services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java

+5 −0
Original line number Diff line number Diff line
@@ -176,6 +176,11 @@ public class VirtualDeviceManagerService extends SystemService { 
    @VisibleForTesting

    void notifyRunningAppsChanged(int deviceId, ArraySet<Integer> uids) {

        synchronized (mVirtualDeviceManagerLock) {

            if (!mVirtualDevices.contains(deviceId)) {

                Slog.e(TAG, "notifyRunningAppsChanged called for unknown deviceId:" + deviceId

                        + " (maybe it was recently closed?)");

                return;

            }

            mAppsOnVirtualDevices.put(deviceId, uids);

        }

        mLocalService.onAppsOnVirtualDeviceChanged();

services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java

+13 −1
Original line number Diff line number Diff line
@@ -634,6 +634,8 @@ public class VirtualDeviceManagerServiceTest { 


    @Test

    public void onAppsOnVirtualDeviceChanged_multipleVirtualDevices_listenersNotified() {

        createVirtualDevice(VIRTUAL_DEVICE_ID_2, DEVICE_OWNER_UID_2);



        ArraySet<Integer> uidsOnDevice1 = new ArraySet<>(Arrays.asList(UID_1, UID_2));

        ArraySet<Integer> uidsOnDevice2 = new ArraySet<>(Arrays.asList(UID_3, UID_4));

        mLocalService.registerAppsOnVirtualDeviceListener(mAppsOnVirtualDeviceListener);
@@ -645,7 +647,7 @@ public class VirtualDeviceManagerServiceTest { 
                new ArraySet<>(Arrays.asList(UID_1, UID_2)));



        // Notifies that the running apps on the second virtual device has changed.

        mVdms.notifyRunningAppsChanged(mDeviceImpl.getDeviceId() + 1, uidsOnDevice2);

        mVdms.notifyRunningAppsChanged(VIRTUAL_DEVICE_ID_2, uidsOnDevice2);

        TestableLooper.get(this).processAllMessages();

        // The union of the apps running on both virtual devices are sent to the listeners.

        verify(mAppsOnVirtualDeviceListener).onAppsOnAnyVirtualDeviceChanged(
@@ -1058,6 +1060,16 @@ public class VirtualDeviceManagerServiceTest { 
        verify(mSensorManagerInternalMock).removeRuntimeSensor(SENSOR_HANDLE);

    }



    @Test

    public void closedDevice_lateCallToRunningAppsChanged_isIgnored() {

        mLocalService.registerAppsOnVirtualDeviceListener(mAppsOnVirtualDeviceListener);

        int deviceId = mDeviceImpl.getDeviceId();

        mDeviceImpl.close();

        mVdms.notifyRunningAppsChanged(deviceId, Sets.newArraySet(UID_1));

        TestableLooper.get(this).processAllMessages();

        verify(mAppsOnVirtualDeviceListener, never()).onAppsOnAnyVirtualDeviceChanged(any());

    }



    @Test

    public void sendKeyEvent_noFd() {

        assertThrows(