Fix possible race conditions during channel unregistration.

Previously, the input dispatcher assumed that the input channel's
receive pipe file descriptor was a sufficiently unique identifier for
looking up input channels in its various tables.  However, it can happen
that an input channel is disposed and then a new input channel is
immediately created that reuses the same file descriptor.  Ordinarily
this is not a problem, however there is a small opportunity for a race
to arise in InputQueue.

When InputQueue receives an input event from the dispatcher, it
generates a finishedToken that encodes the channel's receive pipe fd,
and a sequence number.  The finishedToken is used by the ViewRoot
as a handle for the event so that it can tell the InputQueue when
the event has finished being processed.

Here is the race:

1. InputQueue receives an input event, assigns a new finishedToken.
2. ViewRoot begins processing the input event.
3. During processing, ViewRoot unregisters the InputChannel.
4. A new InputChannel is created and is registered with the Input Queue.
   This InputChannel happens to have the same receive pipe fd as
   the one previously registered.
5. ViewRoot tells the InputQueue that it has finished processing the
   input event, passing along the original finishedToken.
6. InputQueue throws an exception because the finishedToken's receive
   pipe fd is registered but the sequence number is incorrect so it
   assumes that the client has called finish spuriously.

The fix is to include a unique connection id within the finishedToken so
that the InputQueue can accurately confirm that the token belongs to
the currently registered InputChannel rather than to an old one that
happened to have the same receive pipe fd.  When it notices this, it
ignores the spurious finish.

I've also made a couple of other small changes to avoid similar races
elsewhere.

This patch set also includes a fix to synthesize a finished signal
when the input channel is unregistered on the client side to
help keep the server and client in sync.

Bug: 2834068
Change-Id: I1de34a36249ab74c359c2c67a57e333543400f7b

1 files changed, 10 insertions, 14 deletions

diff --git a/services/jni/com_android_server_InputManager.cpp b/services/jni/com_android_server_InputManager.cpp
index ebe71ab..ba58b43 100644
--- a/services/jni/com_android_server_InputManager.cpp
+++ b/services/jni/com_android_server_InputManager.cpp
@@ -319,9 +319,9 @@ private:
     bool isScreenOn();
     bool isScreenBright();
 
-    // Weak references to all currently registered input channels by receive fd.
+    // Weak references to all currently registered input channels by connection pointer.
     Mutex mInputChannelRegistryLock;
-    KeyedVector<int, jweak> mInputChannelObjWeakByReceiveFd;
+    KeyedVector<InputChannel*, jweak> mInputChannelObjWeakTable;
 
     jobject getInputChannelObjLocal(JNIEnv* env, const sp<InputChannel>& inputChannel);
 
@@ -509,8 +509,7 @@ status_t NativeInputManager::registerInputChannel(JNIEnv* env,
     {
         AutoMutex _l(mInputChannelRegistryLock);
 
-        ssize_t index = mInputChannelObjWeakByReceiveFd.indexOfKey(
-                inputChannel->getReceivePipeFd());
+        ssize_t index = mInputChannelObjWeakTable.indexOfKey(inputChannel.get());
         if (index >= 0) {
             LOGE("Input channel object '%s' has already been registered",
                     inputChannel->getName().string());
@@ -518,8 +517,7 @@ status_t NativeInputManager::registerInputChannel(JNIEnv* env,
             goto DeleteWeakRef;
         }
 
-        mInputChannelObjWeakByReceiveFd.add(inputChannel->getReceivePipeFd(),
-                inputChannelObjWeak);
+        mInputChannelObjWeakTable.add(inputChannel.get(), inputChannelObjWeak);
     }
 
     status = mInputManager->registerInputChannel(inputChannel);
@@ -534,7 +532,7 @@ status_t NativeInputManager::registerInputChannel(JNIEnv* env,
     // Failed!
     {
         AutoMutex _l(mInputChannelRegistryLock);
-        mInputChannelObjWeakByReceiveFd.removeItem(inputChannel->getReceivePipeFd());
+        mInputChannelObjWeakTable.removeItem(inputChannel.get());
     }
 
 DeleteWeakRef:
@@ -548,16 +546,15 @@ status_t NativeInputManager::unregisterInputChannel(JNIEnv* env,
     {
         AutoMutex _l(mInputChannelRegistryLock);
 
-        ssize_t index = mInputChannelObjWeakByReceiveFd.indexOfKey(
-                inputChannel->getReceivePipeFd());
+        ssize_t index = mInputChannelObjWeakTable.indexOfKey(inputChannel.get());
         if (index < 0) {
             LOGE("Input channel object '%s' is not currently registered",
                     inputChannel->getName().string());
             return INVALID_OPERATION;
         }
 
-        inputChannelObjWeak = mInputChannelObjWeakByReceiveFd.valueAt(index);
-        mInputChannelObjWeakByReceiveFd.removeItemsAt(index);
+        inputChannelObjWeak = mInputChannelObjWeakTable.valueAt(index);
+        mInputChannelObjWeakTable.removeItemsAt(index);
     }
 
     env->DeleteWeakGlobalRef(inputChannelObjWeak);
@@ -572,13 +569,12 @@ jobject NativeInputManager::getInputChannelObjLocal(JNIEnv* env,
     {
         AutoMutex _l(mInputChannelRegistryLock);
 
-        ssize_t index = mInputChannelObjWeakByReceiveFd.indexOfKey(
-                inputChannel->getReceivePipeFd());
+        ssize_t index = mInputChannelObjWeakTable.indexOfKey(inputChannel.get());
         if (index < 0) {
             return NULL;
         }
 
-        jweak inputChannelObjWeak = mInputChannelObjWeakByReceiveFd.valueAt(index);
+        jweak inputChannelObjWeak = mInputChannelObjWeakTable.valueAt(index);
         return env->NewLocalRef(inputChannelObjWeak);
     }
 }