From 773ec04592322d4aea0a4526213443bd1e1ee513 Mon Sep 17 00:00:00 2001
From: Antony Sargent <asargent@google.com>
Date: Thu, 19 Jan 2023 15:46:07 -0800
Subject: [PATCH] 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
---
 .../virtual/VirtualDeviceManagerService.java       |  5 +++++
 .../virtual/VirtualDeviceManagerServiceTest.java   | 14 +++++++++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java
index 47ec80e5de5ea..2395814cc4bf7 100644
--- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java
+++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java
@@ -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();
diff --git a/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java b/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java
index 6a4435f480f52..dad7977a8bb3d 100644
--- a/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/companion/virtual/VirtualDeviceManagerServiceTest.java
@@ -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(