From 2d4ad028228abd28fcd83e7996ddd7a7828d91df Mon Sep 17 00:00:00 2001
From: Jan Sebechlebsky <jsebechlebsky@google.com>
Date: Tue, 28 Feb 2023 17:25:48 +0100
Subject: [PATCH] Use dedicated lock in InputController

Sharing the single lock with VirtualDeviceImpl doesn't make sense
since they don't really share resources which cannot be accessed
concurrently.

The shared lock lead to deadlock described in b/270703290, using
dedicated lock in InputController removes resource allocation cycle
which caused that deadlock instance.

Bug: 270703290
Test: atest VirtualMouseTest --iterations 20
Test: atest CtsHardwareTestCases
Test: atest CtsVirtualDevicesTestCases
Test: atest VirtualDeviceManagerServiceTest
Change-Id: Id12a945e182a5516ee06193a4fa475b863f11846
---
 .../server/companion/virtual/InputController.java        | 9 ++++-----
 .../server/companion/virtual/VirtualDeviceImpl.java      | 1 -
 .../server/companion/virtual/InputControllerTest.java    | 2 +-
 .../virtual/VirtualDeviceManagerServiceTest.java         | 2 +-
 4 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/services/companion/java/com/android/server/companion/virtual/InputController.java b/services/companion/java/com/android/server/companion/virtual/InputController.java
index 21b51b1acef02..607439b79d91d 100644
--- a/services/companion/java/com/android/server/companion/virtual/InputController.java
+++ b/services/companion/java/com/android/server/companion/virtual/InputController.java
@@ -88,7 +88,7 @@ class InputController {
      */
     private static final int DEVICE_NAME_MAX_LENGTH = 80;
 
-    final Object mLock;
+    final Object mLock = new Object();
 
     /* Token -> file descriptor associations. */
     @GuardedBy("mLock")
@@ -101,18 +101,17 @@ class InputController {
     private final WindowManager mWindowManager;
     private final DeviceCreationThreadVerifier mThreadVerifier;
 
-    InputController(@NonNull Object lock, @NonNull Handler handler,
+    InputController(@NonNull Handler handler,
             @NonNull WindowManager windowManager) {
-        this(lock, new NativeWrapper(), handler, windowManager,
+        this(new NativeWrapper(), handler, windowManager,
                 // Verify that virtual devices are not created on the handler thread.
                 () -> !handler.getLooper().isCurrentThread());
     }
 
     @VisibleForTesting
-    InputController(@NonNull Object lock, @NonNull NativeWrapper nativeWrapper,
+    InputController(@NonNull NativeWrapper nativeWrapper,
             @NonNull Handler handler, @NonNull WindowManager windowManager,
             @NonNull DeviceCreationThreadVerifier threadVerifier) {
-        mLock = lock;
         mHandler = handler;
         mNativeWrapper = nativeWrapper;
         mDisplayManagerInternal = LocalServices.getService(DisplayManagerInternal.class);
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 f650560e6b22d..0e7fecf1284dd 100644
--- a/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java
+++ b/services/companion/java/com/android/server/companion/virtual/VirtualDeviceImpl.java
@@ -251,7 +251,6 @@ final class VirtualDeviceImpl extends IVirtualDevice.Stub
         mDisplayManager = displayManager;
         if (inputController == null) {
             mInputController = new InputController(
-                    mVirtualDeviceLock,
                     context.getMainThreadHandler(),
                     context.getSystemService(WindowManager.class));
         } else {
diff --git a/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java b/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java
index 760ed9be6234f..7642e7bc3b912 100644
--- a/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java
+++ b/services/tests/servicestests/src/com/android/server/companion/virtual/InputControllerTest.java
@@ -87,7 +87,7 @@ public class InputControllerTest {
 
         // Allow virtual devices to be created on the looper thread for testing.
         final InputController.DeviceCreationThreadVerifier threadVerifier = () -> true;
-        mInputController = new InputController(new Object(), mNativeWrapperMock,
+        mInputController = new InputController(mNativeWrapperMock,
                 new Handler(TestableLooper.get(this).getLooper()),
                 InstrumentationRegistry.getTargetContext().getSystemService(WindowManager.class),
                 threadVerifier);
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 cc6f7c27b01d4..50567b9c9f7b1 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
@@ -355,7 +355,7 @@ public class VirtualDeviceManagerServiceTest {
                 TestableLooper.get(this), mNativeWrapperMock, mIInputManagerMock);
         // Allow virtual devices to be created on the looper thread for testing.
         final InputController.DeviceCreationThreadVerifier threadVerifier = () -> true;
-        mInputController = new InputController(new Object(), mNativeWrapperMock,
+        mInputController = new InputController(mNativeWrapperMock,
                 new Handler(TestableLooper.get(this).getLooper()),
                 mContext.getSystemService(WindowManager.class), threadVerifier);
         mSensorController =