From add074a3bd7507c8205ef9248fe2a81505287c9e Mon Sep 17 00:00:00 2001 From: Jan Sebechlebsky Date: Mon, 8 May 2023 15:15:28 +0200 Subject: [PATCH] Do not share VirtualDeviceImpl lock with SensorController There's no reason to share those locks and separating the locks breaks lock inversion cycle. Also, return copy of mSensorDescriptors in getSensorDescriptors instead of original map. Bug: 278900649 Test: atest CtsVirtualDevicesTestCases Change-Id: I993ae4f0c2a5590529bcabaa15463956b3aa5b1d --- .../server/companion/virtual/SensorController.java | 9 ++++----- .../server/companion/virtual/VirtualDeviceImpl.java | 3 +-- .../server/companion/virtual/SensorControllerTest.java | 3 +-- .../virtual/VirtualDeviceManagerServiceTest.java | 3 +-- 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/services/companion/java/com/android/server/companion/virtual/SensorController.java b/services/companion/java/com/android/server/companion/virtual/SensorController.java index 6d198de984907..2d591b614d18d 100644 --- a/services/companion/java/com/android/server/companion/virtual/SensorController.java +++ b/services/companion/java/com/android/server/companion/virtual/SensorController.java @@ -53,19 +53,18 @@ public class SensorController { private static AtomicInteger sNextDirectChannelHandle = new AtomicInteger(1); - private final Object mLock; + private final Object mLock = new Object(); private final int mVirtualDeviceId; @GuardedBy("mLock") - private final Map mSensorDescriptors = new ArrayMap<>(); + private final ArrayMap mSensorDescriptors = new ArrayMap<>(); @NonNull private final SensorManagerInternal.RuntimeSensorCallback mRuntimeSensorCallback; private final SensorManagerInternal mSensorManagerInternal; private final VirtualDeviceManagerInternal mVdmInternal; - public SensorController(@NonNull Object lock, int virtualDeviceId, + public SensorController(int virtualDeviceId, @Nullable IVirtualSensorCallback virtualSensorCallback) { - mLock = lock; mVirtualDeviceId = virtualDeviceId; mRuntimeSensorCallback = new RuntimeSensorCallbackWrapper(virtualSensorCallback); mSensorManagerInternal = LocalServices.getService(SensorManagerInternal.class); @@ -185,7 +184,7 @@ public class SensorController { @VisibleForTesting Map getSensorDescriptors() { synchronized (mLock) { - return mSensorDescriptors; + return new ArrayMap<>(mSensorDescriptors); } } 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 de0f68ccd6659..6b55d7ed4d56f 100644 --- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java +++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java @@ -266,8 +266,7 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub mInputController = inputController; } if (sensorController == null) { - mSensorController = new SensorController( - mVirtualDeviceLock, mDeviceId, mParams.getVirtualSensorCallback()); + mSensorController = new SensorController(mDeviceId, mParams.getVirtualSensorCallback()); } else { mSensorController = sensorController; } diff --git a/services/tests/servicestests/src/com/android/server/companion/virtual/SensorControllerTest.java b/services/tests/servicestests/src/com/android/server/companion/virtual/SensorControllerTest.java index aea8b86589849..6a45d5f7823de 100644 --- a/services/tests/servicestests/src/com/android/server/companion/virtual/SensorControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/companion/virtual/SensorControllerTest.java @@ -70,8 +70,7 @@ public class SensorControllerTest { LocalServices.removeServiceForTest(SensorManagerInternal.class); LocalServices.addService(SensorManagerInternal.class, mSensorManagerInternalMock); - mSensorController = - new SensorController(new Object(), VIRTUAL_DEVICE_ID, mVirtualSensorCallback); + mSensorController = new SensorController(VIRTUAL_DEVICE_ID, mVirtualSensorCallback); mSensorEvent = new VirtualSensorEvent.Builder(new float[] { 1f, 2f, 3f}).build(); mVirtualSensorConfig = new VirtualSensorConfig.Builder(Sensor.TYPE_ACCELEROMETER, VIRTUAL_SENSOR_NAME) 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 c8c1d6f0f7ed2..8884dba217ed7 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 @@ -359,8 +359,7 @@ public class VirtualDeviceManagerServiceTest { mInputController = new InputController(mNativeWrapperMock, new Handler(TestableLooper.get(this).getLooper()), mContext.getSystemService(WindowManager.class), threadVerifier); - mSensorController = - new SensorController(new Object(), VIRTUAL_DEVICE_ID_1, mVirtualSensorCallback); + mSensorController = new SensorController(VIRTUAL_DEVICE_ID_1, mVirtualSensorCallback); mCameraAccessController = new CameraAccessController(mContext, mLocalService, mCameraAccessBlockedCallback);