Ensure objects remain valid after calling policy

The function
afterKeyEventLockedInterruptable releases the lock and calls into
policy. During this time, the call to "removeInputChannel" might come
in. This call would cause the waitQueue to be drained. Therefore, the
dispatchEntry that's stored in this queue would be deleted.

Before this CL, we obtained a reference to the EventEntry object before
calling policy. If there aren't any more strong pointers remaining to
the EventEntry, the object would become deleted, and the reference would
end up pointing to freed memory.

Previous flow of events:
- KeyEntry is allocated during setFocusedWindow call, as part of
  "synthesizeCancelationEvents".
- App calls "finish" on an event, and dispatcher notifies policy about
  the unhandled key event. But dispatcher must release lock before
  calling policy.
- After dispatcher has released the lock, but before it called policy,
  there is a binder call to "removeInputChannel" that comes in. That
  causes the waitQueue to be drained, and deletes the DispatchEntry. If
  the dispatch entry is the last remaining reference to the KeyEntry,
  then the KeyEntry gets deleted, as well.
- The dispatcher calls policy, and uses the reference to the KeyEntry
  that it was provided. But that reference points to freed memory. This
  causes a crash.

To deal with this, make a few changes in this CL:
- Since the "doDispatchCycleFinishedCommand" is stored in queue, it
  should have a strong pointer to the connection object, and not just
  a reference. That means the Connection object will be valid when the
  command actually runs (otherwise, someone might delete it)
- Inside afterKeyEventLockedInterruptable, assume that the dispatchEntry
  will be deleted after the lock is released. Make copies of the data
  that we need after the lock is regained:
  1) Add refcount for EventEntry
  2) Store the "hasForegroundTarget" into a separate variable
     (technically, it's not necessary, but it allows us to remove all
     usages of "dispatchEntry" in the rest of the function.

As an alternative, we could re-look up the DispatchEntry in the
waitQueue after we regain the lock, but that seems more complex in terms
of implementation / readability.

Bug: 343129193
Test: atest --host inputflinger_tests
Merged-In: Ibea7117e4c85cd1e98bbd01872ce249cbb2d54bd
Change-Id: Ibea7117e4c85cd1e98bbd01872ce249cbb2d54bd

Author: vishniakou

services/inputflinger/dispatcher/InputDispatcher.cpp

@@ -5730,9 +5730,7 @@ void InputDispatcher::doDispatchCycleFinishedCommand(nsecs_t finishTime,
 
     bool restartEvent;
     if (dispatchEntry->eventEntry->type == EventEntry::Type::KEY) {
-        KeyEntry& keyEntry = static_cast<KeyEntry&>(*(dispatchEntry->eventEntry));
-        restartEvent =
-                afterKeyEventLockedInterruptable(connection, dispatchEntry, keyEntry, handled);
+        restartEvent = afterKeyEventLockedInterruptable(connection, dispatchEntry, handled);
     } else if (dispatchEntry->eventEntry->type == EventEntry::Type::MOTION) {
         MotionEntry& motionEntry = static_cast<MotionEntry&>(*(dispatchEntry->eventEntry));
         restartEvent = afterMotionEventLockedInterruptable(connection, dispatchEntry, motionEntry,
@@ -5960,7 +5958,17 @@ void InputDispatcher::processConnectionResponsiveLocked(const Connection& connec
 
 bool InputDispatcher::afterKeyEventLockedInterruptable(const sp<Connection>& connection,
                                                        DispatchEntry* dispatchEntry,
-                                                       KeyEntry& keyEntry, bool handled) {
+                                                       bool handled) {
+    // The dispatchEntry is currently valid, but it might point to a deleted object after we release
+    // the lock. For simplicity, make copies of the data of interest here and assume that
+    // 'dispatchEntry' is not valid after this section.
+    // Hold a strong reference to the EventEntry to ensure it's valid for the duration of this
+    // function, even if the DispatchEntry gets destroyed and releases its share of the ownership.
+    std::shared_ptr<EventEntry> eventEntry = dispatchEntry->eventEntry;
+    const bool hasForegroundTarget = dispatchEntry->hasForegroundTarget();
+    KeyEntry& keyEntry = static_cast<KeyEntry&>(*(eventEntry));
+    // To prevent misuse, ensure dispatchEntry is no longer valid.
+    dispatchEntry = nullptr;
     if (keyEntry.flags & AKEY_EVENT_FLAG_FALLBACK) {
         if (!handled) {
             // Report the key as unhandled, since the fallback was not handled.
@@ -5977,7 +5985,7 @@ bool InputDispatcher::afterKeyEventLockedInterruptable(const sp<Connection>& con
         connection->inputState.removeFallbackKey(originalKeyCode);
     }
 
-    if (handled || !dispatchEntry->hasForegroundTarget()) {
+    if (handled || !hasForegroundTarget) {
         // If the application handles the original key for which we previously
         // generated a fallback or if the window is not a foreground window,
         // then cancel the associated fallback key, if any.

services/inputflinger/dispatcher/InputDispatcher.h

@@ -662,7 +662,7 @@ class InputDispatcher : public android::InputDispatcherInterface {
     void updateLastAnrStateLocked(const std::string& windowLabel, const std::string& reason)
             REQUIRES(mLock);
     bool afterKeyEventLockedInterruptable(const sp<Connection>& connection,
-                                          DispatchEntry* dispatchEntry, KeyEntry& keyEntry,
+                                          DispatchEntry* dispatchEntry,
                                           bool handled) REQUIRES(mLock);
     bool afterMotionEventLockedInterruptable(const sp<Connection>& connection,
                                              DispatchEntry* dispatchEntry, MotionEntry& motionEntry,