Prevent double-closure of VirtualDeviceImpl
In certain conditions VirtualDeviceImpl#close() can execute twice
causing system server to crash.
Bug: 278934835
Test: atest VirtualDeviceManagerBasicTest
Change-Id: I48269b105d5019fcd12fd5f949be507ba8bc0eb5
diff --git a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java
index ae88f24..de0f68c 100644
--- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java
+++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java
@@ -404,39 +404,44 @@
public void close() {
super.close_enforcePermission();
// Remove about-to-be-closed virtual device from the service before butchering it.
- mService.removeVirtualDevice(mDeviceId);
+ boolean removed = mService.removeVirtualDevice(mDeviceId);
mDeviceId = Context.DEVICE_ID_INVALID;
- VirtualDisplayWrapper[] virtualDisplaysToBeReleased;
- synchronized (mVirtualDeviceLock) {
- if (mVirtualAudioController != null) {
- mVirtualAudioController.stopListening();
- mVirtualAudioController = null;
- }
- mLocaleList = null;
- virtualDisplaysToBeReleased = new VirtualDisplayWrapper[mVirtualDisplays.size()];
- for (int i = 0; i < mVirtualDisplays.size(); i++) {
- virtualDisplaysToBeReleased[i] = mVirtualDisplays.valueAt(i);
- }
- mVirtualDisplays.clear();
- mVirtualSensorList = null;
- mVirtualSensors.clear();
+ // Device is already closed.
+ if (!removed) {
+ return;
}
- // Destroy the display outside locked section.
- for (VirtualDisplayWrapper virtualDisplayWrapper : virtualDisplaysToBeReleased) {
- mDisplayManager.releaseVirtualDisplay(virtualDisplayWrapper.getToken());
- // The releaseVirtualDisplay call above won't trigger
- // VirtualDeviceImpl.onVirtualDisplayRemoved callback because we already removed the
- // virtual device from the service - we release the other display-tied resources here
- // with the guarantee it will be done exactly once.
- releaseOwnedVirtualDisplayResources(virtualDisplayWrapper);
- }
-
- mAppToken.unlinkToDeath(this, 0);
- mCameraAccessController.stopObservingIfNeeded();
final long ident = Binder.clearCallingIdentity();
try {
+ VirtualDisplayWrapper[] virtualDisplaysToBeReleased;
+ synchronized (mVirtualDeviceLock) {
+ if (mVirtualAudioController != null) {
+ mVirtualAudioController.stopListening();
+ mVirtualAudioController = null;
+ }
+ mLocaleList = null;
+ virtualDisplaysToBeReleased = new VirtualDisplayWrapper[mVirtualDisplays.size()];
+ for (int i = 0; i < mVirtualDisplays.size(); i++) {
+ virtualDisplaysToBeReleased[i] = mVirtualDisplays.valueAt(i);
+ }
+ mVirtualDisplays.clear();
+ mVirtualSensorList = null;
+ mVirtualSensors.clear();
+ }
+ // Destroy the display outside locked section.
+ for (VirtualDisplayWrapper virtualDisplayWrapper : virtualDisplaysToBeReleased) {
+ mDisplayManager.releaseVirtualDisplay(virtualDisplayWrapper.getToken());
+ // The releaseVirtualDisplay call above won't trigger
+ // VirtualDeviceImpl.onVirtualDisplayRemoved callback because we already removed the
+ // virtual device from the service - we release the other display-tied resources
+ // here with the guarantee it will be done exactly once.
+ releaseOwnedVirtualDisplayResources(virtualDisplayWrapper);
+ }
+
+ mAppToken.unlinkToDeath(this, 0);
+ mCameraAccessController.stopObservingIfNeeded();
+
mInputController.close();
mSensorController.close();
} finally {
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 9644642..ad4c0bf 100644
--- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java
+++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceManagerService.java
@@ -202,8 +202,19 @@
}
}
- void removeVirtualDevice(int deviceId) {
+ /**
+ * Remove the virtual device. Sends the
+ * {@link VirtualDeviceManager#ACTION_VIRTUAL_DEVICE_REMOVED} broadcast as a result.
+ *
+ * @param deviceId deviceId to be removed
+ * @return {@code true} if the device was removed, {@code false} if the operation was a no-op
+ */
+ boolean removeVirtualDevice(int deviceId) {
synchronized (mVirtualDeviceManagerLock) {
+ if (!mVirtualDevices.contains(deviceId)) {
+ return false;
+ }
+
mAppsOnVirtualDevices.remove(deviceId);
mVirtualDevices.remove(deviceId);
}
@@ -223,6 +234,7 @@
} finally {
Binder.restoreCallingIdentity(identity);
}
+ return true;
}
private void syncVirtualDevicesToCdmAssociations(List<AssociationInfo> associations) {
@@ -248,7 +260,6 @@
for (VirtualDeviceImpl virtualDevice : virtualDevicesToRemove) {
virtualDevice.close();
}
-
}
private void registerCdmAssociationListener() {